Re: 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