Re: Fix duplicate detection for null-treatment window functions - Mailing list pgsql-hackers

From Chao Li
Subject Re: Fix duplicate detection for null-treatment window functions
Date
Msg-id B1F56C92-9B12-41A0-A6C1-F4BDA3949237@gmail.com
Whole thread
In response to Fix duplicate detection for null-treatment window functions  (Chao Li <li.evan.chao@gmail.com>)
Responses Re: Fix duplicate detection for null-treatment window functions
List pgsql-hackers

> On Jul 7, 2026, at 08:14, Chao Li <li.evan.chao@gmail.com> wrote:
>
> Hi,
>
> I just spotted an oversight from “[25a30bbd4] Add IGNORE NULLS/RESPECT NULLS option to Window functions”.
>
> In ExecInitWindowAgg(), there is logic to detect duplicate functions:
> ```
> if (i <= wfuncno && wfunc->ignore_nulls == perfunc[i].ignore_nulls)
> {
> /* Found a match to an existing entry, so just mark it */
> wfuncstate->wfuncno = i;
> continue;
> }
> ```
>
> However, when appending function info to perfunc, ignore_nulls is not copied:
> ```
> /* Fill in the perfuncstate data */
> perfuncstate->wfuncstate = wfuncstate;
> perfuncstate->wfunc = wfunc;
> perfuncstate->numArguments = list_length(wfuncstate->args);
> perfuncstate->winCollation = wfunc->inputcollid;
> ```
>
> As a result, wfunc->ignore_nulls == perfunc[i].ignore_nulls can never be true for duplicate IGNORE NULLS or explicit
RESPECTNULLS calls. This means duplicate detection doesn't work for those calls. This bug is easy to prove by adding
temporarylogs, and the fix is straightforward: copy ignore_nulls when filling in the perfuncstate data. 
>
> This is a simple repro:
>
> Without the fix:
> ```
> evantest=# WITH t(x) AS (VALUES (NULL::int), (1), (2))
> evantest-# SELECT first_value(x) IGNORE NULLS OVER w AS a,
> evantest-#        first_value(x) IGNORE NULLS OVER w AS b
> evantest-# FROM t
> evantest-# WINDOW w AS (ORDER BY x NULLS FIRST
> evantest(#              ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING);
> INFO:  WindowAgg duplicate cache miss: no previous match, ignore_nulls 1
> INFO:  WindowAgg duplicate cache miss: matched wfuncno 0, ignore_nulls 1, cached ignore_nulls 0
> a | b
> ---+---
> 1 | 1
> 1 | 1
> 1 | 1
> (3 rows)
> ```
>
> With the fix:
> ```
> evantest=# WITH t(x) AS (VALUES (NULL::int), (1), (2))
> evantest-# SELECT first_value(x) IGNORE NULLS OVER w AS a,
> evantest-#        first_value(x) IGNORE NULLS OVER w AS b
> evantest-# FROM t
> evantest-# WINDOW w AS (ORDER BY x NULLS FIRST
> evantest(#              ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING);
> INFO:  WindowAgg duplicate cache miss: no previous match, ignore_nulls 1
> INFO:  WindowAgg duplicate cache hit: existing wfuncno 0, ignore_nulls 1
> a | b
> ---+---
> 1 | 1
> 1 | 1
> 1 | 1
> (3 rows)
> ```
>
> See the attached patch for details. The actual fix is only one line. The INFO logs above were produced with temporary
debuglogs added around the duplicate-function lookup, those logs are not part of the proposed fix. I left those logs in
thepatch with TODO comments only so reviewers can see the behavior before and after the fix. They should be removed
beforepushing. 
>
> Best regards,
> --
> Chao Li (Evan)
> HighGo Software Co., Ltd.
> https://www.highgo.com/
>
>
>
>
> <v1-0001-Fix-duplicate-detection-for-null-treatment-window.patch>

I realized that leaving the temp log code in the patch is not friendly to the CF test. So, splitting the temp log part
intoa diff file. 

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





Attachment

pgsql-hackers by date:

Previous
From: Xuneng Zhou
Date:
Subject: Re: Implement waiting for wal lsn replay: reloaded
Next
From: Heikki Linnakangas
Date:
Subject: Re: libpq: Process buffered SSL read bytes to support records >8kB on async API