Should IGNORE NULLS cache nullness for volatile arguments?

First seen: 2026-05-14 04:14:54+00:00 · Messages: 7 · Participants: 2

Latest Update

2026-05-20 · claude-opus-4-6

No Substantive Progress

The only new message is a brief thank-you from Evan Chao acknowledging that Tatsuo Ishii pushed the patch. There is no new technical content, no further discussion, and no additional changes.

History (1 prior analysis)
2026-05-18 · claude-opus-4-6

Incremental Update: Thread Resolved — Patch Reviewed, Refined, and Pushed

Summary

The thread reached resolution quickly. Tatsuo Ishii reviewed the original patch, suggested a clean encapsulation improvement, received a v2 patch addressing the feedback, and pushed it to the repository.

Key Developments

Ishii's Review & Endorsement of Approach #3 (2026-05-14)

Ishii framed the design space explicitly into three options:

  1. Cache nullness unconditionally (current, buggy for volatile)
  2. Prohibit volatile expressions in window function arguments (implementation limitation)
  3. Skip the cache for volatile expressions (the patch's approach)

He endorsed option #3 as the reasonable choice, noting that the SQL standard does not prohibit volatile expressions in window function arguments (except for offset arguments).

Code Review Feedback: Encapsulation Improvement (2026-05-14)

Ishii's sole substantive review comment was an architectural cleanliness suggestion: move the notnull_info_cacheable[argno] guard inside get_notnull_info() and put_notnull_info() rather than having the caller (ignorenulls_getfuncarginframe) check cacheability externally. This keeps the caching logic self-contained — callers don't need to know about volatility-based cache bypass decisions.

v2 Patch (2026-05-15)

Evan Chao produced a v2 patch incorporating the encapsulation feedback. The functional behavior is identical to v1; the change is purely about where the cacheability check lives (pushed down into the get/put helper functions).

Patch Pushed (2026-05-18)

Ishii pushed the v2 patch after a 3-day objection window with no further comments.

Open Questions from Prior Analysis — Now Resolved

  • Back-patching: Not explicitly discussed, but given this was pushed (not back-patched with specific mention), it likely targets the development branch where IGNORE NULLS was introduced.
  • Caching the value alongside nullness: Not pursued; the simpler approach of skipping the cache for volatile expressions was accepted.
  • Other similar volatility assumptions in window code: Not investigated further in this thread.