I revisited the thread:
https://www.postgresql.org/message-id/flat/CAGMVOdsbtRwE_4%2Bv8zjH1d9xfovDeQAGLkP_B6k69_VoFEgX-A%40mail.gmail.com
and came up with attached POC patch (I used some varibale names
appearing in the Krasiyan Andreev's patch). I really love to have
RESPECT/IGNORE NULLS because I believe they are convenient for
users. For FIRST/LAST I am not so excited since there are alternatives
as our document stats, so FIRST/LAST are not included in the patch.
Currently in the patch only nth_value is allowed to use RESPECT/IGNORE
NULLS. I think it's not hard to implement it for others (lead, lag,
first_value and last_value). No document nor test patches are
included for now.
Note that RESPECT/IGNORE are not registered as reserved keywords in
this patch (but registered as unreserved keywords). I am not sure if
this is acceptable or not.
> The questions of how we interface to the individual window functions
> are really independent of how we handle the parsing problem. My
> first inclination is to just pass the flags down to the window functions
> (store them in WindowObject and provide some additional inquiry functions
> in windowapi.h) and let them deal with it.
I agree with this. Also I do not change the prototype of
nth_value. So I pass RESPECT/IGNORE NULLS information from the raw
parser to parse/analysis and finally to WindowObject.
> It's also worth wondering if we couldn't just implement the flags in
> some generic fashion and not need to involve the window functions at
> all. FROM LAST, for example, could and perhaps should be implemented
> by inverting the sort order. Possibly IGNORE NULLS could be implemented
> inside the WinGetFuncArgXXX functions? These behaviors might or might
> not make much sense with other window functions, but that doesn't seem
> like it's our problem.
Yes, probably we could make WinGetFuncArgXXX a little bit smarter in
this direction (not implemented in the patch at this point).
Best reagards,
--
Tatsuo Ishii
SRA OSS LLC
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
>From 07f01f8859e159c908ada72e8f53daf51e0b8bdf Mon Sep 17 00:00:00 2001
From: Tatsuo Ishii <is...@postgresql.org>
Date: Sat, 22 Apr 2023 16:52:50 +0900
Subject: [PATCH v1 1/3] Implement IGNORE or RESPECT NULLS parse/analysis part.
Implement SQL standard's IGNORE/RESPECT NULLS clause for window functions.
For now, only nth_value() can use this option.
---
src/backend/parser/gram.y | 22 ++++++++++++++++++----
src/backend/parser/parse_func.c | 13 +++++++++++++
src/include/nodes/parsenodes.h | 8 ++++++++
src/include/nodes/primnodes.h | 2 ++
src/include/parser/kwlist.h | 2 ++
5 files changed, 43 insertions(+), 4 deletions(-)
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index acf6cf4866..2980ecd666 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -276,6 +276,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
MergeWhenClause *mergewhen;
struct KeyActions *keyactions;
struct KeyAction *keyaction;
+ NullTreatment nulltreatment;
}
%type <node> stmt toplevel_stmt schema_stmt routine_body_stmt
@@ -661,6 +662,8 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
json_object_constructor_null_clause_opt
json_array_constructor_null_clause_opt
+%type <nulltreatment> opt_null_treatment
+
/*
* Non-keyword token types. These are hard-wired into the "flex" lexer.
* They must be listed first so that their numeric codes do not depend on
@@ -718,7 +721,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
HANDLER HAVING HEADER_P HOLD HOUR_P
- IDENTITY_P IF_P ILIKE IMMEDIATE IMMUTABLE IMPLICIT_P IMPORT_P IN_P INCLUDE
+ IDENTITY_P IF_P IGNORE_P ILIKE IMMEDIATE IMMUTABLE IMPLICIT_P IMPORT_P IN_P INCLUDE
INCLUDING INCREMENT INDENT INDEX INDEXES INHERIT INHERITS INITIALLY INLINE_P
INNER_P INOUT INPUT_P INSENSITIVE INSERT INSTEAD INT_P INTEGER
INTERSECT INTERVAL INTO INVOKER IS ISNULL ISOLATION
@@ -752,7 +755,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
RANGE READ REAL REASSIGN RECHECK RECURSIVE REF_P REFERENCES REFERENCING
REFRESH REINDEX RELATIVE_P RELEASE RENAME REPEATABLE REPLACE REPLICA
- RESET RESTART RESTRICT RETURN RETURNING RETURNS REVOKE RIGHT ROLE ROLLBACK ROLLUP
+ RESET RESPECT RESTART RESTRICT RETURN RETURNING RETURNS REVOKE RIGHT ROLE ROLLBACK ROLLUP
ROUTINE ROUTINES ROW ROWS RULE
SAVEPOINT SCALAR SCHEMA SCHEMAS SCROLL SEARCH SECOND_P SECURITY SELECT
@@ -15213,7 +15216,7 @@ func_application: func_name '(' ')'
* (Note that many of the special SQL functions wouldn't actually make any
* sense as functional index entries, but we ignore that consideration here.)
*/
-func_expr: func_application within_group_clause filter_clause over_clause
+func_expr: func_application within_group_clause filter_clause opt_null_treatment over_clause
{
FuncCall *n = (FuncCall *) $1;
@@ -15246,7 +15249,8 @@ func_expr: func_application within_group_clause filter_clause over_clause
n->agg_within_group = true;
}
n->agg_filter = $3;
- n->over = $4;
+ n->null_treatment = $4;
+ n->over = $5;
$$ = (Node *) n;
}
| json_aggregate_func filter_clause over_clause
@@ -15790,6 +15794,14 @@ filter_clause:
| /*EMPTY*/ { $$ = NULL; }
;
+/*
+ * Window function option clauses
+ */
+opt_null_treatment:
+ RESPECT NULLS_P { $$ = RESPECT_NULLS; }
+ | IGNORE_P NULLS_P { $$ = IGNORE_NULLS; }
+ | /*EMPTY*/ { $$ = NULL_TREATMENT_NOT_SET; }
+ ;
/*
* Window Definitions
@@ -17111,6 +17123,7 @@ unreserved_keyword:
| HOUR_P
| IDENTITY_P
| IF_P
+ | IGNORE_P
| IMMEDIATE
| IMMUTABLE
| IMPLICIT_P
@@ -17223,6 +17236,7 @@ unreserved_keyword:
| REPLACE
| REPLICA
| RESET
+ | RESPECT
| RESTART
| RESTRICT
| RETURN
diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index b3f0b6a137..92af0d10f1 100644
--- a/src/backend/parser/parse_func.c
+++ b/src/backend/parser/parse_func.c
@@ -31,6 +31,7 @@
#include "parser/parse_target.h"
#include "parser/parse_type.h"
#include "utils/builtins.h"
+#include "utils/fmgroids.h"
#include "utils/lsyscache.h"
#include "utils/syscache.h"
@@ -99,6 +100,7 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
bool agg_distinct = (fn ? fn->agg_distinct : false);
bool func_variadic = (fn ? fn->func_variadic : false);
CoercionForm funcformat = (fn ? fn->funcformat : COERCE_EXPLICIT_CALL);
+ NullTreatment null_treatment = (fn ? fn->null_treatment : NULL_TREATMENT_NOT_SET);
bool could_be_projection;
Oid rettype;
Oid funcid;
@@ -534,6 +536,13 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
errmsg("window function %s cannot have WITHIN GROUP",
NameListToString(funcname)),
parser_errposition(pstate, location)));
+ /* Check RESPECT NULLS or IGNORE NULLS is specified. They are only allowed with nth_value */
+ if (null_treatment != NULL_TREATMENT_NOT_SET && funcid != F_NTH_VALUE)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("window function %s cannot have RESPECT NULLS or IGNORE NULLS",
+ NameListToString(funcname)),
+ parser_errposition(pstate, location)));
}
else if (fdresult == FUNCDETAIL_COERCION)
{
@@ -835,6 +844,10 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
wfunc->winagg = (fdresult == FUNCDETAIL_AGGREGATE);
wfunc->aggfilter = agg_filter;
wfunc->location = location;
+ if (null_treatment == IGNORE_NULLS)
+ wfunc->ignorenulls = true;
+ else
+ wfunc->ignorenulls = false;
/*
* agg_star is allowed for aggregate functions but distinct isn't
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index cc7b32b279..f13ae26a24 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -404,6 +404,13 @@ typedef struct RoleSpec
int location; /* token location, or -1 if unknown */
} RoleSpec;
+typedef enum NullTreatment
+{
+ NULL_TREATMENT_NOT_SET = 0,
+ RESPECT_NULLS,
+ IGNORE_NULLS
+} NullTreatment;
+
/*
* FuncCall - a function or aggregate invocation
*
@@ -431,6 +438,7 @@ typedef struct FuncCall
bool agg_distinct; /* arguments were labeled DISTINCT */
bool func_variadic; /* last argument was labeled VARIADIC */
CoercionForm funcformat; /* how to display this node */
+ NullTreatment null_treatment; /* RESPECT_NULLS or IGNORE NULLS */
int location; /* token location, or -1 if unknown */
} FuncCall;
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index be9c29f0bf..213297dbd3 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -559,6 +559,8 @@ typedef struct WindowFunc
bool winstar pg_node_attr(query_jumble_ignore);
/* is function a simple aggregate? */
bool winagg pg_node_attr(query_jumble_ignore);
+ /* true if IGNORE NULLS, false if RESPECT NULLS */
+ bool ignorenulls pg_node_attr(query_jumble_ignore);
/* token location, or -1 if unknown */
int location;
} WindowFunc;
diff --git a/src/include/parser/kwlist.h b/src/include/parser/kwlist.h
index f5b2e61ca5..c7e61a8f0e 100644
--- a/src/include/parser/kwlist.h
+++ b/src/include/parser/kwlist.h
@@ -198,6 +198,7 @@ PG_KEYWORD("hold", HOLD, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("hour", HOUR_P, UNRESERVED_KEYWORD, AS_LABEL)
PG_KEYWORD("identity", IDENTITY_P, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("if", IF_P, UNRESERVED_KEYWORD, BARE_LABEL)
+PG_KEYWORD("ignore", IGNORE_P, UNRESERVED_KEYWORD, AS_LABEL)
PG_KEYWORD("ilike", ILIKE, TYPE_FUNC_NAME_KEYWORD, BARE_LABEL)
PG_KEYWORD("immediate", IMMEDIATE, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("immutable", IMMUTABLE, UNRESERVED_KEYWORD, BARE_LABEL)
@@ -360,6 +361,7 @@ PG_KEYWORD("repeatable", REPEATABLE, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("replace", REPLACE, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("replica", REPLICA, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("reset", RESET, UNRESERVED_KEYWORD, BARE_LABEL)
+PG_KEYWORD("respect", RESPECT, UNRESERVED_KEYWORD, AS_LABEL)
PG_KEYWORD("restart", RESTART, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("restrict", RESTRICT, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("return", RETURN, UNRESERVED_KEYWORD, BARE_LABEL)
--
2.25.1
>From 3dc6f4bb897f76247589db018716bf5680d5331c Mon Sep 17 00:00:00 2001
From: Tatsuo Ishii <is...@postgresql.org>
Date: Sat, 22 Apr 2023 16:58:48 +0900
Subject: [PATCH v1 2/3] Implement IGNORE or RESPECT NULLS planner part.
---
src/backend/optimizer/util/clauses.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index a9c7bc342e..40fc62c447 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -2474,6 +2474,7 @@ eval_const_expressions_mutator(Node *node,
newexpr->winref = expr->winref;
newexpr->winstar = expr->winstar;
newexpr->winagg = expr->winagg;
+ newexpr->ignorenulls = expr->ignorenulls;
newexpr->location = expr->location;
return (Node *) newexpr;
--
2.25.1
>From a78feec9bf7b08c644c2b3089b2de9237d4fcd9e Mon Sep 17 00:00:00 2001
From: Tatsuo Ishii <is...@postgresql.org>
Date: Sat, 22 Apr 2023 17:00:06 +0900
Subject: [PATCH v1 3/3] Implement IGNORE or RESPECT NULLS executor and window
functions part.
---
src/backend/executor/nodeWindowAgg.c | 11 ++++++++++
src/backend/utils/adt/windowfuncs.c | 30 +++++++++++++++++++++++++---
src/include/windowapi.h | 2 ++
3 files changed, 40 insertions(+), 3 deletions(-)
diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c
index 3ac581a711..7e2affb12c 100644
--- a/src/backend/executor/nodeWindowAgg.c
+++ b/src/backend/executor/nodeWindowAgg.c
@@ -69,6 +69,7 @@ typedef struct WindowObjectData
int readptr; /* tuplestore read pointer for this fn */
int64 markpos; /* row that markptr is positioned on */
int64 seekpos; /* row that readptr is positioned on */
+ WindowFunc *wfunc; /* WindowFunc of this function */
} WindowObjectData;
/*
@@ -2617,6 +2618,7 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags)
winobj->winstate = winstate;
winobj->argstates = wfuncstate->args;
winobj->localmem = NULL;
+ winobj->wfunc = wfunc;
perfuncstate->winobj = winobj;
/* It's a real window function, so set up to call it. */
@@ -3620,3 +3622,12 @@ WinGetFuncArgCurrent(WindowObject winobj, int argno, bool *isnull)
return ExecEvalExpr((ExprState *) list_nth(winobj->argstates, argno),
econtext, isnull);
}
+
+/*
+ * Return current WindowFunc
+ */
+WindowFunc *
+WinGetWindowFunc(WindowObject winobj)
+{
+ return winobj->wfunc;
+}
diff --git a/src/backend/utils/adt/windowfuncs.c b/src/backend/utils/adt/windowfuncs.c
index b87a624fb2..919295ba13 100644
--- a/src/backend/utils/adt/windowfuncs.c
+++ b/src/backend/utils/adt/windowfuncs.c
@@ -693,6 +693,7 @@ window_nth_value(PG_FUNCTION_ARGS)
bool const_offset;
Datum result;
bool isnull;
+ bool isout;
int32 nth;
nth = DatumGetInt32(WinGetFuncArgCurrent(winobj, 1, &isnull));
@@ -705,9 +706,32 @@ window_nth_value(PG_FUNCTION_ARGS)
(errcode(ERRCODE_INVALID_ARGUMENT_FOR_NTH_VALUE),
errmsg("argument of nth_value must be greater than zero")));
- result = WinGetFuncArgInFrame(winobj, 0,
- nth - 1, WINDOW_SEEK_HEAD, const_offset,
- &isnull, NULL);
+ if (WinGetWindowFunc(winobj)->ignorenulls)
+ {
+ int i, n;
+
+ i = n = 0;
+
+ for (;;)
+ {
+ result = WinGetFuncArgInFrame(winobj, 0,
+ i++, WINDOW_SEEK_HEAD, false,
+ &isnull, &isout);
+ if (isout)
+ break;
+
+ if (!isnull)
+ {
+ if (n == nth - 1)
+ break;
+ n++;
+ }
+ }
+ }
+ else
+ result = WinGetFuncArgInFrame(winobj, 0,
+ nth - 1, WINDOW_SEEK_HEAD, const_offset,
+ &isnull, NULL);
if (isnull)
PG_RETURN_NULL();
diff --git a/src/include/windowapi.h b/src/include/windowapi.h
index b8c2c565d1..64f7d4c84d 100644
--- a/src/include/windowapi.h
+++ b/src/include/windowapi.h
@@ -61,4 +61,6 @@ extern Datum WinGetFuncArgInFrame(WindowObject winobj, int argno,
extern Datum WinGetFuncArgCurrent(WindowObject winobj, int argno,
bool *isnull);
+extern WindowFunc *WinGetWindowFunc(WindowObject winobj);
+
#endif /* WINDOWAPI_H */
--
2.25.1
