From: Chao Li <li.evan.chao@gmail.com>
Message-Id: <B1F56C92-9B12-41A0-A6C1-F4BDA3949237@gmail.com>
Content-Type: multipart/mixed;
	boundary="Apple-Mail=_43A14EE2-A4A0-4228-AF56-389DAE965431"
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.600.51.1.1\))
Subject: Re: Fix duplicate detection for null-treatment window functions
Date: Thu, 9 Jul 2026 05:32:53 -0700
In-Reply-To: <5D2C9081-5DFE-4E27-AB14-7358238EA1BC@gmail.com>
Cc: Tatsuo Ishii <ishii@postgresql.org>
To: PostgreSQL-development <pgsql-hackers@lists.postgresql.org>
References: <5D2C9081-5DFE-4E27-AB14-7358238EA1BC@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/B1F56C92-9B12-41A0-A6C1-F4BDA3949237%40gmail.com>
Precedence: bulk


--Apple-Mail=_43A14EE2-A4A0-4228-AF56-389DAE965431
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=utf-8


> On Jul 7, 2026, at 08:14, Chao Li <li.evan.chao@gmail.com> wrote:
>=20
> Hi,
>=20
> I just spotted an oversight from =E2=80=9C[25a30bbd4] Add IGNORE =
NULLS/RESPECT NULLS option to Window functions=E2=80=9D.
>=20
> In ExecInitWindowAgg(), there is logic to detect duplicate functions:
> ```
> if (i <=3D wfuncno && wfunc->ignore_nulls =3D=3D =
perfunc[i].ignore_nulls)
> {
> /* Found a match to an existing entry, so just mark it */
> wfuncstate->wfuncno =3D i;
> continue;
> }
> ```
>=20
> However, when appending function info to perfunc, ignore_nulls is not =
copied:
> ```
> /* Fill in the perfuncstate data */
> perfuncstate->wfuncstate =3D wfuncstate;
> perfuncstate->wfunc =3D wfunc;
> perfuncstate->numArguments =3D list_length(wfuncstate->args);
> perfuncstate->winCollation =3D wfunc->inputcollid;
> ```
>=20
> As a result, wfunc->ignore_nulls =3D=3D 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.
>=20
> This is a simple repro:
>=20
> Without the fix:
> ```
> evantest=3D# 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)
> ```
>=20
> With the fix:
> ```
> evantest=3D# 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)
> ```
>=20
> 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.
>=20
> Best regards,
> --
> Chao Li (Evan)
> HighGo Software Co., Ltd.
> https://www.highgo.com/
>=20
>=20
>=20
>=20
> <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 into a diff file.

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


--Apple-Mail=_43A14EE2-A4A0-4228-AF56-389DAE965431
Content-Disposition: attachment;
	filename=nocfbot.windowagg-null-treatment-debug-logs.diff
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="nocfbot.windowagg-null-treatment-debug-logs.diff"
Content-Transfer-Encoding: 7bit

diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c
index 676fe024965..00000000000 100644
--- a/src/backend/executor/nodeWindowAgg.c
+++ b/src/backend/executor/nodeWindowAgg.c
@@ -2749,11 +2749,23 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags)
 		}
 		if (i <= wfuncno && wfunc->ignore_nulls == perfunc[i].ignore_nulls)
 		{
+			/* TODO: remove this log before pushing */
+			elog(INFO, "WindowAgg duplicate cache hit: existing wfuncno %d, ignore_nulls %d",
+				 i, wfunc->ignore_nulls);
+
 			/* Found a match to an existing entry, so just mark it */
 			wfuncstate->wfuncno = i;
 			continue;
 		}
 
+		/* TODO: remove this log before pushing */
+		if (i <= wfuncno)
+			elog(INFO, "WindowAgg duplicate cache miss: matched wfuncno %d, ignore_nulls %d, cached ignore_nulls %d",
+				 i, wfunc->ignore_nulls, perfunc[i].ignore_nulls);
+		else
+			elog(INFO, "WindowAgg duplicate cache miss: no previous match, ignore_nulls %d",
+				 wfunc->ignore_nulls);
+
 		/* Nope, so assign a new PerAgg record */
 		perfuncstate = &perfunc[++wfuncno];
 

--Apple-Mail=_43A14EE2-A4A0-4228-AF56-389DAE965431
Content-Disposition: attachment;
	filename=v2-0001-Fix-duplicate-detection-for-null-treatment-window.patch
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="v2-0001-Fix-duplicate-detection-for-null-treatment-window.patch"
Content-Transfer-Encoding: quoted-printable

=46rom=20b87100e21c618107c1f17d64c9efcd6d2e32a428=20Mon=20Sep=2017=20=
00:00:00=202001=0AFrom:=20"Chao=20Li=20(Evan)"=20<lic@highgo.com>=0A=
Date:=20Tue,=207=20Jul=202026=2008:10:58=20-0700=0ASubject:=20[PATCH=20=
v2]=20Fix=20duplicate=20detection=20for=20null-treatment=20window=0A=20=
functions=0A=0A`ExecInitWindowAgg()`=20checks=20=
`perfunc[i].ignore_nulls`=20when=20deciding=0Awhether=20a=20window=20=
function=20call=20can=20reuse=20an=20existing=20`WindowStatePerFunc`=0A=
entry.=20However,=20that=20field=20was=20never=20initialized=20when=20=
filling=20the=0A`perfuncstate`=20data,=20so=20it=20remained=20zero=20=
from=20`palloc0_array()`.=0A=0AAs=20a=20result,=20duplicate=20calls=20=
using=20`IGNORE=20NULLS`=20or=20explicit=0A`RESPECT=20NULLS`=20failed=20=
to=20reuse=20the=20existing=20entry,=20because=20their=0A=
`wfunc->ignore_nulls`=20value=20did=20not=20match=20the=20zeroed=20=
per-function=20state.=0APlain=20calls=20were=20unaffected=20because=20=
their=20null-treatment=20value=20is=20also=20zero.=0A=0ACopy=20=
`wfunc->ignore_nulls`=20into=20`perfuncstate->ignore_nulls`=20during=0A=
initialization=20so=20duplicate=20null-treatment=20window=20function=20=
calls=20are=0Adeduplicated=20as=20intended.=0A=0AOversight=20in=20=
25a30bbd423.=0A=0AAuthor:=20Chao=20Li=20<lic@highgo.com>=0A---=0A=20=
src/backend/executor/nodeWindowAgg.c=20|=201=20+=0A=201=20file=20=
changed,=201=20insertion(+)=0A=0Adiff=20--git=20=
a/src/backend/executor/nodeWindowAgg.c=20=
b/src/backend/executor/nodeWindowAgg.c=0Aindex=20=
f1c524d00df..b8b0d853dec=20100644=0A---=20=
a/src/backend/executor/nodeWindowAgg.c=0A+++=20=
b/src/backend/executor/nodeWindowAgg.c=0A@@=20-2773,6=20+2773,7=20@@=20=
ExecInitWindowAgg(WindowAgg=20*node,=20EState=20*estate,=20int=20eflags)=0A=
=20=09=09perfuncstate->wfunc=20=3D=20wfunc;=0A=20=09=09=
perfuncstate->numArguments=20=3D=20list_length(wfuncstate->args);=0A=20=09=
=09perfuncstate->winCollation=20=3D=20wfunc->inputcollid;=0A+=09=09=
perfuncstate->ignore_nulls=20=3D=20wfunc->ignore_nulls;=0A=20=0A=20=09=09=
get_typlenbyval(wfunc->wintype,=0A=20=09=09=09=09=09=09=
&perfuncstate->resulttypeLen,=0A--=20=0A2.50.1=20(Apple=20Git-155)=0A=0A=

--Apple-Mail=_43A14EE2-A4A0-4228-AF56-389DAE965431--