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 RESPECT NULLS calls. This means
duplicate detection doesn't work for those calls. This bug is easy to prove by
adding temporary logs, 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 debug logs added around the
duplicate-function lookup, those logs are not part of the proposed fix. I left
those logs in the patch with TODO comments only so reviewers can see the
behavior before and after the fix. They should be removed before pushing.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
v1-0001-Fix-duplicate-detection-for-null-treatment-window.patch
Description: Binary data
