Should IGNORE NULLS cache nullness for volatile arguments?

Hi Chao,

Thank you for the test and patches.

Hi,

I tested the new IGNORE NULLS support for window functions and noticed one behavior that looks strange to me.

To avoid repeated evaluation, the current code caches whether an argument value is NULL or NOT NULL. That is fine for stable expressions, but it looks unsafe for volatile arguments. For example, an argument may be evaluated as NOT NULL when its nullness is first checked, but when the value is needed later and the argument is evaluated again, the result may become NULL. That can lead to surprising results for volatile functions.

I do not have full confidence to call this a bug yet, but I think it is at least worth discussing. If the value of a NOT NULL argument were also cached, then I guess this behavior might be acceptable. But with the current implementation, the argument can be re-evaluated later and produce the opposite nullness result, which seems wrong to me.

As far as I know, the SQL standard does not prohibit to use a
(possible) volatile value expression for window function's arguments
(except offset argument of course). So there are a few choices:

1) Cache whether NULL or NOT NULL and reuse even for volatile
expressions (current implementation). This produces weird results
as you described.

2) Prohibit to use volatile expressions for window functions
arguments. This becomes a PostgreSQL's implementation limitation.

3) Give up to use the cache when volatile expressions are used (your
patches).

For me, #3 seems to be a reasonable choice.

The attached patch makes a small change in that direction. It only uses the IGNORE NULLS nullness cache when the argument is safe to reuse. For non-cacheable arguments, the nullness is treated as unknown and the argument is evaluated again.

See the attached patch for details.

I will look into the patches.

Regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp

Hi Chao,

The attached patch makes a small change in that direction. It only uses the IGNORE NULLS nullness cache when the argument is safe to reuse. For non-cacheable arguments, the nullness is treated as unknown and the argument is evaluated again.

See the attached patch for details.

I will look into the patches.

@@ -3454,7 +3455,10 @@ ignorenulls_getfuncarginframe(WindowObject winobj, int argno,
if (isout)
*isout = false;

-		v = get_notnull_info(winobj, abs_pos, argno);
+		if (winobj->notnull_info_cacheable[argno])

What about moving this if statement inside get_notnull_info() so that
the caller does not care about this argno is cacheable or not?

+			/* record the row status if it is safe to reuse */
+			if (winobj->notnull_info_cacheable[argno])
+				put_notnull_info(winobj, abs_pos, argno, *isnull);

Similary, we can move "if (winobj->notnull_info_cacheable[argno])" inside put_notnull_info().

Regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp

On May 14, 2026, at 20:56, Tatsuo Ishii <ishii@postgresql.org> wrote:

Hi Chao,

The attached patch makes a small change in that direction. It only uses the IGNORE NULLS nullness cache when the argument is safe to reuse. For non-cacheable arguments, the nullness is treated as unknown and the argument is evaluated again.

See the attached patch for details.

I will look into the patches.

@@ -3454,7 +3455,10 @@ ignorenulls_getfuncarginframe(WindowObject winobj, int argno,
if (isout)
*isout = false;

- v = get_notnull_info(winobj, abs_pos, argno);
+ if (winobj->notnull_info_cacheable[argno])

What about moving this if statement inside get_notnull_info() so that
the caller does not care about this argno is cacheable or not?

+ /* record the row status if it is safe to reuse */
+ if (winobj->notnull_info_cacheable[argno])
+ put_notnull_info(winobj, abs_pos, argno, *isnull);

Similary, we can move "if (winobj->notnull_info_cacheable[argno])" inside put_notnull_info().

Yep, good idea. Addressed in attached v2.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

@@ -3454,7 +3455,10 @@ ignorenulls_getfuncarginframe(WindowObject winobj, int argno,
if (isout)
*isout = false;

- v = get_notnull_info(winobj, abs_pos, argno);
+ if (winobj->notnull_info_cacheable[argno])

What about moving this if statement inside get_notnull_info() so that
the caller does not care about this argno is cacheable or not?

+ /* record the row status if it is safe to reuse */
+ if (winobj->notnull_info_cacheable[argno])
+ put_notnull_info(winobj, abs_pos, argno, *isnull);

Similary, we can move "if (winobj->notnull_info_cacheable[argno])" inside put_notnull_info().

Yep, good idea. Addressed in attached v2.

Thanks for the v2 patch. It looks good to me. I am going to push the
patch within a few days if there's no objection.

Regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp

Yep, good idea. Addressed in attached v2.

Thanks for the v2 patch. It looks good to me. I am going to push the
patch within a few days if there's no objection.

Patch pushed. Thanks!
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp

On May 18, 2026, at 11:45, Tatsuo Ishii <ishii@postgresql.org> wrote:

Yep, good idea. Addressed in attached v2.

Thanks for the v2 patch. It looks good to me. I am going to push the
patch within a few days if there's no objection.

Patch pushed. Thanks!
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp

Hi Tatsuo-san, thank you very much for pushing.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/