public inbox for [email protected]
help / color / mirror / Atom feedFrom: Tatsuo Ishii <[email protected]>
To: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: Add RESPECT/IGNORE NULLS and FROM FIRST/LAST options
Date: Sun, 12 Oct 2025 17:39:59 +0900 (JST)
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<[email protected]>
> Probably I misunderstood what you said. Now I realize what you are
> suggesting was, throwing an error *only* when a RESPECT/IGNORE NULLS
> option is given and the function did not call
> WinAllowNullTreatmentOption. If the option is not given, no error is
> thrown even if WinAllowNullTreatmentOption is not called. I am okay
> with this direction. I will post a patch for this.
While I was implementing this, I realized that in order to check if
WinAllowNullTreatmentOption had been called or not, it's necessary to
call the window function at least once in mainline execution of
nodeWindowAgg.c (I supposed in eval_windowfunction). I don't like this
because I expected to throw an error *before* calling the window
function.
So I studied your idea:
> Alternatively, you could just drop the entire concept of throwing an
> error for that.
Attached is a patch to implement this. Previously window functions
should call WinCheckAndInitializeNullTreatment with
allowNullTreatment==true if they accept a null treatment
clause. Otherwise, they are called as if null treatment clause is not
specified. With the patch, window functions accept a null treatment
clause as specified without calling
WinCheckAndInitializeNullTreatment.
There's one thing which might be different from what you suggested
is, I want to give window functions a method to stat that they do not
want accept a null treatment clause. For this purpose
WinCheckAndInitializeNullTreatment (with allowNullTreatment==false)
can be called.(Alternatively we could eliminate allowNullTreatment
argument and rename it something like WinDisallowNullTreatmentOption).
Some of built-in window functions that do not accept a null treatment
clause call this in the patch. This way, we do not need to test the
case when the functions are given a null treatment option except just
they throw an error. User defined functions would call the function
for the same purpose as built-in window functions.
> The implementation is entirely
> within nodeWindowAgg.c and does not depend in any way on the
> cooperation of the window function.
I am not sure. For example built-in lead function's behavior (with
IGNORE NULLS option) is defined by the standard. Unlike RESPECT NULLS
case, the expected behavior may not be obvious. According the
standard:
1. If lead's "offset" option is 0, the argument evaluated on current
row is returned regardless the value is NULL or NOT.
2. Otherwise, returns the value evaluated on a row which is nth NOT NULL.
For me, 2 is obvious but 1 was not so obvious because I thought that
lead() returns only non NULL value (except there's no non null values
or specified offset is out of partition).
Thus lead() calls WinGetFuncArgInPartition with
seektype==WINDOW_SEEK_CURRENT. Thus WinGetFuncArgInPartition with
WINDOW_SEEK_CURRENT is implemented in a way to satisfy the lead()
semantics above. This means if someone tries to implement a new window
function calling WinGetFuncArgInPartition with WINDOW_SEEK_CURRENT,
the function must has the same semantics as lead(). I think there's a
cooperation between nodeWindowAgg.c and window functions.
> + This option is only allowed for the following functions: <function>lag</function>,
> + <function>lead</function>, <function>first_value</function>, <function>last_value</function>,
> + <function>nth_value</function>.
>
> as this fails to account for the possibility of user-defined window
> functions. IMO we could drop the error check altogether and rewrite
> the docs along the lines of "Not all window functions pay attention
> to this option. Of the built-in window functions, only blah blah
> and blah do."
Fixing docs are not included in the patch (yet).
Best regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
Attachments:
[application/octet-stream] v1-0001-Allow-window-functions-to-accept-a-null-treatment.patch (4.3K, ../[email protected]/2-v1-0001-Allow-window-functions-to-accept-a-null-treatment.patch)
download | inline diff:
From 925c527b4e776d49292a0fd747dcb16357418703 Mon Sep 17 00:00:00 2001
From: Tatsuo Ishii <[email protected]>
Date: Sun, 12 Oct 2025 16:21:40 +0900
Subject: [PATCH v1] Allow window functions to accept a null treatment clause
by default.
Previously window functions (either built-in or user defined) should
call WinCheckAndInitializeNullTreatment with allowNullTreatment==true
if they accept a null treatment clause (RESPECT NULLS/IGNORE
NULLS). Otherwise, they are called as if null treatment clause is not
specified.
This commit changes the behavior so that window functions accept a
null treatment clause as specified without calling
WinCheckAndInitializeNullTreatment.
If window functions do not want accept a null treatment clause, call
WinCheckAndInitializeNullTreatment with allowNullTreatment==false.
---
src/backend/executor/nodeWindowAgg.c | 17 +++++++++++------
src/backend/utils/adt/windowfuncs.c | 4 ----
2 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c
index e6a53f95391..c2e2ca69347 100644
--- a/src/backend/executor/nodeWindowAgg.c
+++ b/src/backend/executor/nodeWindowAgg.c
@@ -1303,7 +1303,8 @@ begin_partition(WindowAggState *winstate)
winobj->seekpos = -1;
/* reset null map */
- if (winobj->ignore_nulls == IGNORE_NULLS)
+ if (winobj->ignore_nulls == IGNORE_NULLS ||
+ winobj->ignore_nulls == PARSER_IGNORE_NULLS)
memset(winobj->notnull_info, 0,
NN_POS_TO_BYTES(
perfuncstate->winobj->num_notnull_info));
@@ -3530,8 +3531,10 @@ put_notnull_info(WindowObject winobj, int64 pos, bool isnull)
* WinCheckAndInitializeNullTreatment
* Check null treatment clause and sets ignore_nulls
*
- * Window functions should call this to check if they are being called with
- * a null treatment clause when they don't allow it, or to set ignore_nulls.
+ * Window functions call this if they do not accept a null treatment clause
+ * with allowNullTreatment==false. It's not mandatory but they can call this
+ * with allowNullTreatment==true to explicitly stat that they accept a a null
+ * treatment clause.
*/
void
WinCheckAndInitializeNullTreatment(WindowObject winobj,
@@ -3555,7 +3558,6 @@ WinCheckAndInitializeNullTreatment(WindowObject winobj,
}
else if (winobj->ignore_nulls == PARSER_IGNORE_NULLS)
winobj->ignore_nulls = IGNORE_NULLS;
-
}
/*
@@ -3727,7 +3729,9 @@ WinGetFuncArgInPartition(WindowObject winobj, int argno,
Assert(WindowObjectIsValid(winobj));
winstate = winobj->winstate;
- null_treatment = (winobj->ignore_nulls == IGNORE_NULLS && relpos != 0);
+ null_treatment = ((winobj->ignore_nulls == IGNORE_NULLS ||
+ winobj->ignore_nulls == PARSER_IGNORE_NULLS) &&
+ relpos != 0);
switch (seektype)
{
@@ -3866,7 +3870,8 @@ WinGetFuncArgInFrame(WindowObject winobj, int argno,
econtext = winstate->ss.ps.ps_ExprContext;
slot = winstate->temp_slot_1;
- if (winobj->ignore_nulls == IGNORE_NULLS)
+ if (winobj->ignore_nulls == IGNORE_NULLS ||
+ winobj->ignore_nulls == PARSER_IGNORE_NULLS)
return ignorenulls_getfuncarginframe(winobj, argno, relpos, seektype,
set_mark, isnull, isout);
diff --git a/src/backend/utils/adt/windowfuncs.c b/src/backend/utils/adt/windowfuncs.c
index 969f02aa59b..7e936a8bfbc 100644
--- a/src/backend/utils/adt/windowfuncs.c
+++ b/src/backend/utils/adt/windowfuncs.c
@@ -541,7 +541,6 @@ leadlag_common(FunctionCallInfo fcinfo,
bool isnull;
bool isout;
- WinCheckAndInitializeNullTreatment(winobj, true, fcinfo);
if (withoffset)
{
offset = DatumGetInt32(WinGetFuncArgCurrent(winobj, 1, &isnull));
@@ -659,7 +658,6 @@ window_first_value(PG_FUNCTION_ARGS)
Datum result;
bool isnull;
- WinCheckAndInitializeNullTreatment(winobj, true, fcinfo);
result = WinGetFuncArgInFrame(winobj, 0,
0, WINDOW_SEEK_HEAD, true,
&isnull, NULL);
@@ -681,7 +679,6 @@ window_last_value(PG_FUNCTION_ARGS)
Datum result;
bool isnull;
- WinCheckAndInitializeNullTreatment(winobj, true, fcinfo);
result = WinGetFuncArgInFrame(winobj, 0,
0, WINDOW_SEEK_TAIL, true,
&isnull, NULL);
@@ -705,7 +702,6 @@ window_nth_value(PG_FUNCTION_ARGS)
bool isnull;
int32 nth;
- WinCheckAndInitializeNullTreatment(winobj, true, fcinfo);
nth = DatumGetInt32(WinGetFuncArgCurrent(winobj, 1, &isnull));
if (isnull)
PG_RETURN_NULL();
--
2.43.0
view thread (91+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Add RESPECT/IGNORE NULLS and FROM FIRST/LAST options
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox