>> The attached test patch is mostly the same as in the previous patch
>> set, but it doesn't fail on row_number anymore as the main patch
>> only rejects aggregate functions. The test patch also adds a test for
>
>> +SELECT sum(orbit) RESPECT NULLS OVER () FROM planets; -- succeeds
>
> I think the standard does not allow to specify RESPECT NULLS other
> than lead, lag, first_value, last_value and nth_value. Unless we agree
> that PostgreSQL violates the standard in this regard, you should not
> allow to use RESPECT NULLS for the window functions, expect lead etc.
> and aggregates.
>
> See my patch.
>
>> +/*
>> + * Window function option clauses
>> + */
>> +opt_null_treatment:
>> + RESPECT NULLS_P
>> { $$ = RESPECT_NULLS; }
>> + | IGNORE_P NULLS_P
>> { $$ = IGNORE_NULLS; }
>> + | /*EMPTY*/
>> { $$ = NULL_TREATMENT_NOT_SET; }
>> + ;
>
> With this, you can check if null treatment clause is used or not in
> each window function.
>
> In my previous patch I did the check in parse/analysis but I think
> it's better to be checked in each window function. This way,
>
> - need not to add a column to pg_proc.
>
> - allow user defined window functions to decide by themselves whether
> they can accept null treatment option.
Attached is the patch to implement this (on top of your patch).
test=# SELECT row_number() RESPECT NULLS OVER () FROM (SELECT 1) AS s;
ERROR: window function row_number cannot have RESPECT NULLS or IGNORE NULLS
Best reagards,
--
Tatsuo Ishii
SRA OSS LLC
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c
index fac0e05dee..b8519d9890 100644
--- a/src/backend/executor/nodeWindowAgg.c
+++ b/src/backend/executor/nodeWindowAgg.c
@@ -74,6 +74,7 @@ typedef struct WindowObjectData
int64 *win_nonnulls; /* tracks non-nulls in ignore nulls mode */
int nonnulls_size; /* track size of the win_nonnulls array */
int nonnulls_len; /* track length of the win_nonnulls array */
+ WindowFunc *wfunc; /* WindowFunc of this function */
} WindowObjectData;
/*
@@ -2634,6 +2635,8 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags)
winobj->nonnulls_len = 0;
}
+ winobj->wfunc = wfunc;
+
/* It's a real window function, so set up to call it. */
fmgr_info_cxt(wfunc->winfnoid, &perfuncstate->flinfo,
econtext->ecxt_per_query_memory);
@@ -3881,3 +3884,24 @@ WinGetFuncArgCurrent(WindowObject winobj, int argno, bool *isnull)
return ExecEvalExpr((ExprState *) list_nth(winobj->argstates, argno),
econtext, isnull);
}
+
+/*
+ * Error out that this window function cannot have null treatement.
+ */
+void
+ErrorOutNullTreatment(WindowObject winobj)
+{
+ char *fname;
+
+ Assert(WindowObjectIsValid(winobj));
+
+ if (winobj->wfunc->null_treatment == NULL_TREATMENT_NOT_SET)
+ return;
+
+ fname = get_func_name(winobj->wfunc->winfnoid);
+
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("window function %s cannot have RESPECT NULLS or IGNORE NULLS",
+ fname)));
+}
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 01fd16acf9..05e64c4569 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -2475,6 +2475,7 @@ eval_const_expressions_mutator(Node *node,
newexpr->winstar = expr->winstar;
newexpr->winagg = expr->winagg;
newexpr->ignore_nulls = expr->ignore_nulls;
+ newexpr->null_treatment = expr->null_treatment;
newexpr->location = expr->location;
return (Node *) newexpr;
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 58c00bfd4f..e131428e85 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
@@ -633,7 +634,6 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
opt_frame_clause frame_extent frame_bound
%type <ival> opt_window_exclusion_clause
%type <str> opt_existing_window_name
-%type <boolean> null_treatment
%type <boolean> opt_if_not_exists
%type <boolean> opt_unique_null_treatment
%type <ival> generated_when override_kind
@@ -662,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> 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
@@ -15247,7 +15249,7 @@ func_expr: func_application within_group_clause filter_clause null_treatment ove
n->agg_within_group = true;
}
n->agg_filter = $3;
- n->ignore_nulls = $4;
+ n->null_treatment = $4;
n->over = $5;
$$ = (Node *) n;
}
@@ -15797,9 +15799,9 @@ filter_clause:
* Window Definitions
*/
null_treatment:
- IGNORE_P NULLS_P { $$ = true; }
- | RESPECT_P NULLS_P { $$ = false; }
- | /*EMPTY*/ { $$ = false; }
+ RESPECT_P NULLS_P { $$ = RESPECT_NULLS; }
+ | IGNORE_P NULLS_P { $$ = IGNORE_NULLS; }
+ | /*EMPTY*/ { $$ = NULL_TREATMENT_NOT_SET; }
;
window_clause:
diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index afa4bcc8d1..63af8ca6aa 100644
--- a/src/backend/parser/parse_func.c
+++ b/src/backend/parser/parse_func.c
@@ -98,7 +98,7 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
bool agg_star = (fn ? fn->agg_star : false);
bool agg_distinct = (fn ? fn->agg_distinct : false);
bool func_variadic = (fn ? fn->func_variadic : false);
- bool ignore_nulls = (fn ? fn->ignore_nulls : false);
+ NullTreatment null_treatment = (fn ? fn->null_treatment : NULL_TREATMENT_NOT_SET);
CoercionForm funcformat = (fn ? fn->funcformat : COERCE_EXPLICIT_CALL);
bool could_be_projection;
Oid rettype;
@@ -516,11 +516,12 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
NameListToString(funcname)),
parser_errposition(pstate, location)));
- /* It also can't treat nulls as a window function */
- if (ignore_nulls)
+ /* Aggregate functions cannot have null treatment clause */
+ if (null_treatment != NULL_TREATMENT_NOT_SET)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("aggregate functions do not accept RESPECT/IGNORE NULLS"),
+ errmsg("aggregate function %s cannot have RESPECT NULLS or IGNORE NULLS",
+ NameListToString(funcname)),
parser_errposition(pstate, location)));
}
}
@@ -842,7 +843,8 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
wfunc->winstar = agg_star;
wfunc->winagg = (fdresult == FUNCDETAIL_AGGREGATE);
wfunc->aggfilter = agg_filter;
- wfunc->ignore_nulls = ignore_nulls;
+ wfunc->null_treatment = null_treatment;
+ wfunc->ignore_nulls = (null_treatment == IGNORE_NULLS);
wfunc->location = location;
/*
diff --git a/src/backend/utils/adt/windowfuncs.c b/src/backend/utils/adt/windowfuncs.c
index b87a624fb2..297e787927 100644
--- a/src/backend/utils/adt/windowfuncs.c
+++ b/src/backend/utils/adt/windowfuncs.c
@@ -85,6 +85,9 @@ window_row_number(PG_FUNCTION_ARGS)
WindowObject winobj = PG_WINDOW_OBJECT();
int64 curpos = WinGetCurrentPosition(winobj);
+ /* row_number() does not support null treatment */
+ ErrorOutNullTreatment(winobj);
+
WinSetMarkPosition(winobj, curpos);
PG_RETURN_INT64(curpos + 1);
}
@@ -140,6 +143,9 @@ window_rank(PG_FUNCTION_ARGS)
rank_context *context;
bool up;
+ /* rank() does not support null treatment */
+ ErrorOutNullTreatment(winobj);
+
up = rank_up(winobj);
context = (rank_context *)
WinGetPartitionLocalMemory(winobj, sizeof(rank_context));
@@ -202,6 +208,9 @@ window_dense_rank(PG_FUNCTION_ARGS)
rank_context *context;
bool up;
+ /* dense_rank() does not support null treatment */
+ ErrorOutNullTreatment(winobj);
+
up = rank_up(winobj);
context = (rank_context *)
WinGetPartitionLocalMemory(winobj, sizeof(rank_context));
@@ -266,6 +275,9 @@ window_percent_rank(PG_FUNCTION_ARGS)
Assert(totalrows > 0);
+ /* percent_rank() does not support null treatment */
+ ErrorOutNullTreatment(winobj);
+
up = rank_up(winobj);
context = (rank_context *)
WinGetPartitionLocalMemory(winobj, sizeof(rank_context));
@@ -335,6 +347,9 @@ window_cume_dist(PG_FUNCTION_ARGS)
Assert(totalrows > 0);
+ /* cume_dist() does not support null treatment */
+ ErrorOutNullTreatment(winobj);
+
up = rank_up(winobj);
context = (rank_context *)
WinGetPartitionLocalMemory(winobj, sizeof(rank_context));
@@ -412,6 +427,9 @@ window_ntile(PG_FUNCTION_ARGS)
WindowObject winobj = PG_WINDOW_OBJECT();
ntile_context *context;
+ /* ntile() does not support null treatment */
+ ErrorOutNullTreatment(winobj);
+
context = (ntile_context *)
WinGetPartitionLocalMemory(winobj, sizeof(ntile_context));
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 073e2469ba..32fbab46a0 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -432,6 +432,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 221b5e6218..545b5e5ac8 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -532,6 +532,13 @@ typedef struct GroupingFunc
int location;
} GroupingFunc;
+typedef enum NullTreatment
+{
+ NULL_TREATMENT_NOT_SET = 0,
+ RESPECT_NULLS,
+ IGNORE_NULLS
+} NullTreatment;
+
/*
* WindowFunc
*
@@ -559,7 +566,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);
- /* ignore nulls */
+ /* null treatement */
+ NullTreatment null_treatment pg_node_attr(query_jumble_ignore);
bool ignore_nulls;
/* token location, or -1 if unknown */
int location;
diff --git a/src/include/windowapi.h b/src/include/windowapi.h
index b8c2c565d1..8a50478ee9 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 void ErrorOutNullTreatment(WindowObject winobj);
+
#endif /* WINDOWAPI_H */
