From c55037229c69a4fe0aa5eb26535cc00f68de5200 Mon Sep 17 00:00:00 2001 From: jian he Date: Thu, 30 Jul 2026 16:35:37 +0900 Subject: [PATCH] Reject whole-row references and qualified names in a DEFINE clause ISO/IEC 19075-5 6.5 limits the range variables in scope inside a DEFINE clause to the row pattern variables, so a bare relation name there has nothing to resolve against. It was accepted and produced a whole-row Var. Reject it in transformWholeRowRef() when p_expr_kind is EXPR_KIND_RPR_DEFINE. A row constructor has to be held back for the same reason. transformExpressionList() hands a trailing star to ExpandColumnRefStar(), which binds by RTE and yields individual column Vars, so ROW(t.*) reached neither check and was accepted. A view written that way over a join whose sides share a column name is unrestorable from the moment it is created. Skip the expansion under EXPR_KIND_RPR_DEFINE and let transformExpr() reach the checks. The A_Indirection arm keeps expanding: it transforms its parenthesized argument under the same expression kind, so a range variable still reaches the whole-row check, and what survives is field selection on a value, which names no relation. The check has to sit where the query actually writes a whole-row reference. transformColumnRef() also builds one speculatively, to retry a name that did not resolve as a column as a function call on the composite value, and rejecting there reported a construct the query does not contain -- a misspelled column came back as "whole-row reference is not allowed", losing the "Perhaps you meant to reference the column stock.price" the same typo gets anywhere else. Pass for_func_call so the check fires only at the four sites the reference was written at, and let the retry's own failure speak. The range variable check had the same shape and is corrected with it. It fired on the qualifier alone, before the rest of the name was looked at, so the two-part spelling of that typo lost the suggestion too. Only the pattern variable arm has to run that early, a pattern variable naming no range table entry. Move the range variable arm down to where the reference has resolved, beside the outer-query and schema-qualified checks. Down there the rule widens to every qualified name. 6.5 reserves the qualifier slot for a row pattern variable, so nothing else may occupy it, and a two-part name that is not a range variable's still resolves -- through p_post_columnref_hook, which reads one as a SQL function's parameter, a PL/pgSQL variable carrying its routine name or block label, or a field of a composite one. Reject those as well. Nothing becomes unreachable: unqualified, each of them still resolves, and a field of a composite value is reached by parenthesizing it, "(p).amount", which occupies no qualifier slot. The message names no particular reading, since the hook is public and an extension may add its own. A name a ref hook answers before the query parser is outside this, as it is outside every rule here. Under "#variable_conflict use_variable" PL/pgSQL claims any name one of its variables owns and returns first, so in such a function the qualified spellings keep working and a variable named after a pattern variable takes A.price. That pragma redirects name resolution wholesale -- it takes names a table column would otherwise own too -- and a clause of one statement is not the place to carve an exception out of it. The outer-query check has to ask which level the qualifier resolved at, not merely whether it resolved. A two-part name whose second part is not a column is retried as a function call on the whole row, and the FuncExpr that comes back carries the outer reference where the varlevelsup test cannot see it. Note that refnameNamespaceItem() counts the levels it searched, not the level it found, so the count means nothing unless the lookup succeeded. A whole-row Var still reaches a DEFINE clause without being written there: pulling up a subquery substitutes its output expressions into defineClause, and one can be a whole-row Var. The integration test that observed attribute number 0 in that position is rewritten to reach it that way. Reinstate the whole-row entry in the advanced.sgml list of what a DEFINE expression may not contain, removed while the statement was still false. --- doc/src/sgml/advanced.sgml | 4 +- doc/src/sgml/ref/select.sgml | 25 + src/backend/parser/parse_expr.c | 183 +++++-- src/backend/parser/parse_target.c | 25 +- src/test/regress/expected/rpr.out | 516 +++++++++++++++++- src/test/regress/expected/rpr_base.out | 33 ++ src/test/regress/expected/rpr_integration.out | 47 +- src/test/regress/sql/rpr.sql | 393 ++++++++++++- src/test/regress/sql/rpr_base.sql | 24 + src/test/regress/sql/rpr_integration.sql | 28 +- 10 files changed, 1187 insertions(+), 91 deletions(-) diff --git a/doc/src/sgml/advanced.sgml b/doc/src/sgml/advanced.sgml index b0f929266ff..8c0e2f384a6 100644 --- a/doc/src/sgml/advanced.sgml +++ b/doc/src/sgml/advanced.sgml @@ -565,8 +565,8 @@ WHERE pos < 3; it must return TRUE, FALSE or NULL. The expression may comprise column references and non-volatile functions. Window functions, aggregate functions, - set-returning functions and subqueries are not allowed. An example - of DEFINE is as follows. + set-returning functions, whole-row references and subqueries are not + allowed. An example of DEFINE is as follows. DEFINE diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml index 15def1cfdcb..085cc38940f 100644 --- a/doc/src/sgml/ref/select.sgml +++ b/doc/src/sgml/ref/select.sgml @@ -1240,6 +1240,31 @@ DEFINE definition_variable_name AS a subquery. + + The same applies to names that are not columns. A parameter or + variable of the routine whose body contains the query is readable from + a DEFINE condition, but only unqualified: the + PostgreSQL spellings that qualify one with + the routine name or a block label occupy the reserved slot and are + rejected there. To read a field of a composite parameter or record + variable, parenthesize it, as in (p).amount; that is + field selection on a value rather than a qualified name, so the slot + stays free. + + + + A PL/pgSQL function written with + #variable_conflict use_variable is an exception, as + it is everywhere else: that setting makes + PL/pgSQL resolve any name one of its + variables owns before the query does, so inside such a function + qualified spellings such as fn.threshold keep + working, and a variable whose name matches a pattern variable takes + A.price as well. This is the same shadowing the + setting applies to table columns; see + . + + The purpose of a WINDOW clause is to specify the behavior of window functions appearing in the query's diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index be0f99381ad..872d0ea46ad 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -73,7 +73,7 @@ static Node *transformXmlSerialize(ParseState *pstate, XmlSerialize *xs); static Node *transformBooleanTest(ParseState *pstate, BooleanTest *b); static Node *transformCurrentOfExpr(ParseState *pstate, CurrentOfExpr *cexpr); static Node *transformColumnRef(ParseState *pstate, ColumnRef *cref); -static Node *transformWholeRowRef(ParseState *pstate, +static Node *transformWholeRowRef(ParseState *pstate, bool for_func_call, ParseNamespaceItem *nsitem, int sublevels_up, int location); static Node *transformIndirection(ParseState *pstate, A_Indirection *ind); @@ -629,17 +629,23 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref) return node; /*---------- - * Qualified references in DEFINE need a tri-classification: + * A pattern variable qualifier (e.g. UP.price) is valid per ISO/IEC + * 19075-5 6.15 / 4.16 but not yet implemented, and has to be recognized + * here: a pattern variable names no range table entry, so leaving it to + * normal resolution would report a missing FROM-clause entry instead. * - * pattern variable qualifier (e.g. UP.price): valid per - * ISO/IEC 19075-5 6.15 / 4.16 but not yet implemented -- - * raise FEATURE_NOT_SUPPORTED. + * Like every other rule below, this one only reaches names the ref hooks + * left for the query parser to resolve. A PL that answers a name first + * keeps it: under "#variable_conflict use_variable" PL/pgSQL claims any + * name one of its variables owns, so a PL/pgSQL variable sharing a name + * with a pattern variable takes A.price, exactly as it takes a name a + * table column would otherwise own. That is what asking for + * use_variable means, and the DEFINE rules do not override it. * - * FROM-clause range variable qualifier: prohibited by - * ISO/IEC 19075-5 6.5 -- raise SYNTAX_ERROR. - * - * any other qualifier (typo, undefined name): fall through and let - * normal column resolution produce a sensible error. + * The other qualified forms DEFINE disallows are diagnosed after the + * reference resolves, below. Classifying them here on the qualifier + * alone would report a misspelled column as a problem with the qualifier + * and lose the "Perhaps you meant" hint normal resolution offers. * * The quoted text reflects only the ColumnRef portion; a trailing field * selection on a composite type (e.g. ".amount" in "(A.items).amount") @@ -652,31 +658,16 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref) list_length(cref->fields) != 1) { char *qualifier = strVal(linitial(cref->fields)); - bool is_pattern_var = false; foreach_node(String, pv, pstate->p_rpr_pattern_vars) { if (strcmp(strVal(pv), qualifier) == 0) - { - is_pattern_var = true; - break; - } + ereport(ERROR, + errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("pattern variable qualified expression \"%s\" is not supported in DEFINE clause", + NameListToString(cref->fields)), + parser_errposition(pstate, cref->location)); } - - if (is_pattern_var) - ereport(ERROR, - errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("pattern variable qualified expression \"%s\" is not supported in DEFINE clause", - NameListToString(cref->fields)), - parser_errposition(pstate, cref->location)); - else if (refnameNamespaceItem(pstate, NULL, qualifier, - cref->location, NULL) != NULL) - ereport(ERROR, - errcode(ERRCODE_SYNTAX_ERROR), - errmsg("range variable qualified expression \"%s\" is not allowed in DEFINE clause", - NameListToString(cref->fields)), - parser_errposition(pstate, cref->location)); - /* else: unknown qualifier -- fall through to normal resolution */ } /*---------- @@ -730,8 +721,8 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref) cref->location, &levels_up); if (nsitem) - node = transformWholeRowRef(pstate, nsitem, levels_up, - cref->location); + node = transformWholeRowRef(pstate, false, nsitem, + levels_up, cref->location); } break; } @@ -755,8 +746,8 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref) /* Whole-row reference? */ if (IsA(field2, A_Star)) { - node = transformWholeRowRef(pstate, nsitem, levels_up, - cref->location); + node = transformWholeRowRef(pstate, false, nsitem, + levels_up, cref->location); break; } @@ -768,8 +759,8 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref) if (node == NULL) { /* Try it as a function call on the whole row */ - node = transformWholeRowRef(pstate, nsitem, levels_up, - cref->location); + node = transformWholeRowRef(pstate, true, nsitem, + levels_up, cref->location); node = ParseFuncOrColumn(pstate, list_make1(makeString(colname)), list_make1(node), @@ -802,8 +793,8 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref) /* Whole-row reference? */ if (IsA(field3, A_Star)) { - node = transformWholeRowRef(pstate, nsitem, levels_up, - cref->location); + node = transformWholeRowRef(pstate, false, nsitem, + levels_up, cref->location); break; } @@ -815,8 +806,8 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref) if (node == NULL) { /* Try it as a function call on the whole row */ - node = transformWholeRowRef(pstate, nsitem, levels_up, - cref->location); + node = transformWholeRowRef(pstate, true, nsitem, + levels_up, cref->location); node = ParseFuncOrColumn(pstate, list_make1(makeString(colname)), list_make1(node), @@ -861,8 +852,8 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref) /* Whole-row reference? */ if (IsA(field4, A_Star)) { - node = transformWholeRowRef(pstate, nsitem, levels_up, - cref->location); + node = transformWholeRowRef(pstate, false, nsitem, + levels_up, cref->location); break; } @@ -874,8 +865,8 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref) if (node == NULL) { /* Try it as a function call on the whole row */ - node = transformWholeRowRef(pstate, nsitem, levels_up, - cref->location); + node = transformWholeRowRef(pstate, true, nsitem, + levels_up, cref->location); node = ParseFuncOrColumn(pstate, list_make1(makeString(colname)), list_make1(node), @@ -948,20 +939,78 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref) /* * Restrict column references in a row pattern DEFINE clause. node is now - * a successfully resolved reference, so reject the two forms RPR does not - * allow: a correlated reference to an outer query's column, and a - * schema/catalog-qualified reference (three or more name parts). Simple - * two-part qualifiers (pattern or range variable) are handled earlier, - * before resolution. + * a successfully resolved reference, so the qualified forms RPR does not + * allow can be rejected without mistaking a name that does not resolve at + * all for one of them: a correlated reference to an outer query's column, + * a range variable qualifier, and a schema/catalog-qualified reference. + * + * The error class follows the division the neighbouring restrictions use: + * ERRCODE_FEATURE_NOT_SUPPORTED for what the standard allows and this + * implementation does not, ERRCODE_SYNTAX_ERROR for every other rejected + * spelling, whether the standard forbids it or never gave it at all. */ if (pstate->p_expr_kind == EXPR_KIND_RPR_DEFINE) { - if (IsA(node, Var) && ((Var *) node)->varlevelsup > 0) + ParseNamespaceItem *qual_nsitem = NULL; + int qual_levels_up = 0; + + if (list_length(cref->fields) == 2) + qual_nsitem = refnameNamespaceItem(pstate, NULL, + strVal(linitial(cref->fields)), + cref->location, + &qual_levels_up); + + /* + * Ask which level the qualifier resolved at, not merely whether it + * resolved. A two-part name whose second part is not a column is + * retried as a function call on the whole row, and that yields a + * FuncExpr rather than a Var, so the outer reference its argument + * carries is invisible to the varlevelsup test. + * + * The level counts levels searched, not levels found, so it means + * nothing unless the search succeeded. + */ + if ((IsA(node, Var) && ((Var *) node)->varlevelsup > 0) || + (qual_nsitem != NULL && qual_levels_up > 0)) ereport(ERROR, errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot use outer query column in DEFINE clause"), parser_errposition(pstate, cref->location)); + if (qual_nsitem != NULL) + ereport(ERROR, + errcode(ERRCODE_SYNTAX_ERROR), + errmsg("range variable qualified expression \"%s\" is not allowed in DEFINE clause", + NameListToString(cref->fields)), + parser_errposition(pstate, cref->location)); + + /* + * ISO/IEC 19075-5 6.5 reserves the qualifier slot for a row pattern + * variable, so a name is rejected for occupying it whatever the + * qualifier turns out to name. What is left here resolved through + * p_post_columnref_hook, which reads a two-part name as a routine's + * parameter or variable, or as a field of a composite one, and the + * hook is public enough that an extension may add readings of its + * own; the message names none of them. Selecting a field from an + * unqualified value, written "(x).f", occupies no qualifier slot and + * remains the way to reach a composite. + * + * The pre hook's readings never arrive here, so this rule is not the + * last word on a qualified name: in a PL/pgSQL function written with + * "#variable_conflict use_variable", plpgsql_pre_column_ref() answers + * fn.var and rec.field itself and returns before any of this runs, + * leaving those spellings usable there. The pragma redirects name + * resolution wholesale -- it takes names a table column would + * otherwise own too -- and DEFINE does not carve itself out of it. + */ + if (list_length(cref->fields) == 2) + ereport(ERROR, + errcode(ERRCODE_SYNTAX_ERROR), + errmsg("qualified expression \"%s\" is not allowed in DEFINE clause", + NameListToString(cref->fields)), + errhint("Write the name without its qualifier, or write \"(x).field\" to select a field of a composite value."), + parser_errposition(pstate, cref->location)); + if (list_length(cref->fields) >= 3) ereport(ERROR, errcode(ERRCODE_SYNTAX_ERROR), @@ -1986,8 +2035,16 @@ transformSubLink(ParseState *pstate, SubLink *sublink) * are doable with the existing infrastructure -- they are * left as future work, not blocked on any other feature. * Until then this blanket rejection is intentional - * over-rejection, not a standard fit; it subsumes both (a) - * and (b) by making the subquery itself unreachable. + * over-rejection, not a standard fit. + * + * It rejects the SubLink, which is not the same as keeping + * the subquery unanalyzed: a construct that analyzes its + * query before building the SubLink, as + * transformJsonArrayQueryConstructor() does, has already + * resolved names and opened relations inside it by the time + * we get here, and reports its own errors first. Whoever + * implements (a) and (b) must not read this rejection as + * proof that nothing inside a DEFINE subquery runs. *---------- */ case EXPR_KIND_RPR_DEFINE: @@ -2750,11 +2807,29 @@ transformCurrentOfExpr(ParseState *pstate, CurrentOfExpr *cexpr) /* * Construct a whole-row reference to represent the notation "relation.*". + * + * for_func_call is true when transformColumnRef is building the reference + * speculatively, to retry a name that did not resolve as a column as a + * function call on the composite value. The query does not contain a + * whole-row reference in that case, so restrictions on writing one must not + * fire; whatever the retry resolves to is diagnosed by the caller. */ static Node * -transformWholeRowRef(ParseState *pstate, ParseNamespaceItem *nsitem, - int sublevels_up, int location) +transformWholeRowRef(ParseState *pstate, bool for_func_call, + ParseNamespaceItem *nsitem, int sublevels_up, + int location) { + /* + * A DEFINE clause cannot use a whole-row reference: ISO/IEC 19075-5 6.5 + * limits the range variables in scope to the row pattern variables. + */ + if (pstate->p_expr_kind == EXPR_KIND_RPR_DEFINE && !for_func_call) + ereport(ERROR, + errcode(ERRCODE_SYNTAX_ERROR), + errmsg("whole-row reference is not allowed in DEFINE clause"), + errhint("A DEFINE condition may reference individual columns only."), + parser_errposition(pstate, location)); + /* * Build the appropriate referencing node. Normally this can be a * whole-row Var, but if the nsitem is a JOIN USING alias then it contains diff --git a/src/backend/parser/parse_target.c b/src/backend/parser/parse_target.c index 0ea10f8e882..d217fed5be1 100644 --- a/src/backend/parser/parse_target.c +++ b/src/backend/parser/parse_target.c @@ -235,9 +235,18 @@ transformExpressionList(ParseState *pstate, List *exprlist, { ColumnRef *cref = (ColumnRef *) e; - if (IsA(llast(cref->fields), A_Star)) + /* + * It is something.*, expand into multiple items -- except in a + * DEFINE clause, where a reference to a FROM-clause relation is + * not allowed at all. Expanding here binds by RTE rather than by + * name, so it would bypass the checks in transformColumnRef() and + * transformWholeRowRef(). Fall through instead and let + * transformExpr() reach them, so that ROW(t.*) is rejected the + * same way (t.*) already is. + */ + if (IsA(llast(cref->fields), A_Star) && + exprKind != EXPR_KIND_RPR_DEFINE) { - /* It is something.*, expand into multiple items */ result = list_concat(result, ExpandColumnRefStar(pstate, cref, false)); @@ -250,7 +259,17 @@ transformExpressionList(ParseState *pstate, List *exprlist, if (IsA(llast(ind->indirection), A_Star)) { - /* It is something.*, expand into multiple items */ + /* + * It is something.*, expand into multiple items. + * + * No DEFINE test is needed here, unlike the ColumnRef arm + * above. ExpandIndirectionStar() transforms the + * parenthesized argument under the same expression kind, so a + * range variable still reaches transformWholeRowRef() and is + * rejected; what survives is field selection on a value, + * "(x).*", which occupies no qualifier slot and is allowed in + * DEFINE for the same reason "(x).f" is. + */ result = list_concat(result, ExpandIndirectionStar(pstate, ind, false, exprKind)); diff --git a/src/test/regress/expected/rpr.out b/src/test/regress/expected/rpr.out index c4958c1b8d8..59b6677b0cf 100644 --- a/src/test/regress/expected/rpr.out +++ b/src/test/regress/expected/rpr.out @@ -1327,6 +1327,293 @@ LATERAL ( ERROR: cannot use outer query column in DEFINE clause LINE 8: DEFINE A AS PREV(o.threshold, 1) > 0 ^ +-- An outer range variable is subject to the same two rules as a local one: a +-- whole-row reference is rejected as one, and a name that does not resolve +-- keeps its own diagnosis rather than being reported as a qualifier problem. +SELECT * FROM (VALUES (95)) AS o(threshold), +LATERAL ( + SELECT price FROM stock + WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS (o.*) IS NOT NULL + ) +) s; +ERROR: whole-row reference is not allowed in DEFINE clause +LINE 9: DEFINE A AS (o.*) IS NOT NULL + ^ +HINT: A DEFINE condition may reference individual columns only. +SELECT * FROM (VALUES (95)) AS o(threshold), +LATERAL ( + SELECT price FROM stock + WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS o.threshhold > 0 + ) +) s; +ERROR: column o.threshhold does not exist +LINE 9: DEFINE A AS o.threshhold > 0 + ^ +HINT: Perhaps you meant to reference the column "o.threshold". +-- A two-part name is not always a range variable qualifier: a SQL function's +-- parameter and a PL/pgSQL variable both resolve through +-- p_post_columnref_hook. The qualifier slot is reserved all the same, so +-- these are rejected for the spelling, not for what they name. +CREATE FUNCTION rpr_sqlfn(threshold int) RETURNS SETOF int +LANGUAGE sql AS $$ + SELECT price FROM stock + WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS price > rpr_sqlfn.threshold) +$$; +ERROR: qualified expression "rpr_sqlfn.threshold" is not allowed in DEFINE clause +LINE 9: DEFINE A AS price > rpr_sqlfn.threshold) + ^ +HINT: Write the name without its qualifier, or write "(x).field" to select a field of a composite value. +CREATE FUNCTION rpr_plfn(threshold int) RETURNS bigint +LANGUAGE plpgsql AS $$ +DECLARE + n bigint; +BEGIN + SELECT count(*) INTO n FROM ( + SELECT price FROM stock + WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS price > rpr_plfn.threshold) + ) s; + RETURN n; +END +$$; +SELECT rpr_plfn(0); +ERROR: qualified expression "rpr_plfn.threshold" is not allowed in DEFINE clause +LINE 8: DEFINE A AS price > rpr_plfn.threshold) + ^ +HINT: Write the name without its qualifier, or write "(x).field" to select a field of a composite value. +QUERY: SELECT count(*) FROM ( + SELECT price FROM stock + WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS price > rpr_plfn.threshold) + ) s +CONTEXT: PL/pgSQL function rpr_plfn(integer) line 5 at SQL statement +DROP FUNCTION rpr_plfn(int); +-- Unqualified, the same parameter is readable. +CREATE FUNCTION rpr_sqlfn(threshold int) RETURNS SETOF int +LANGUAGE sql AS $$ + SELECT price FROM stock + WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS price > threshold) +$$; +SELECT count(*) FROM rpr_sqlfn(0); + count +------- + 20 +(1 row) + +DROP FUNCTION rpr_sqlfn(int); +-- The qualifier slot is decided on the qualifier alone, before resolution, so +-- a pattern variable takes the slot even from the routine that contains the +-- query. Naming a pattern variable after the function makes rpr_pv.threshold +-- the pattern variable's, and the reservation is reported; that the function +-- has a parameter of that name, and the query has no such column, does not +-- enter into it. +CREATE FUNCTION rpr_pv(threshold int) RETURNS SETOF int +LANGUAGE sql AS $$ + SELECT price FROM stock + WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (rpr_pv) + DEFINE rpr_pv AS price > rpr_pv.threshold) +$$; +ERROR: pattern variable qualified expression "rpr_pv.threshold" is not supported in DEFINE clause +LINE 9: DEFINE rpr_pv AS price > rpr_pv.threshold) + ^ +-- The collision is in the qualifier, not in the DEFINE variable being +-- defined: any pattern variable of that name reserves it. +CREATE FUNCTION rpr_pv(threshold int) RETURNS SETOF int +LANGUAGE sql AS $$ + SELECT price FROM stock + WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (rpr_pv A) + DEFINE A AS price > rpr_pv.threshold) +$$; +ERROR: pattern variable qualified expression "rpr_pv.threshold" is not supported in DEFINE clause +LINE 9: DEFINE A AS price > rpr_pv.threshold) + ^ +-- A field of a composite parameter has no unqualified spelling, so it is +-- reached by parenthesizing the value: "(p).lo" selects a field rather than +-- qualifying a name, and occupies no qualifier slot. +CREATE TYPE rpr_pair AS (lo int, hi int); +CREATE FUNCTION rpr_compfn(p rpr_pair) RETURNS SETOF int +LANGUAGE sql AS $$ + SELECT price FROM stock + WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS price > p.lo) +$$; +ERROR: qualified expression "p.lo" is not allowed in DEFINE clause +LINE 9: DEFINE A AS price > p.lo) + ^ +HINT: Write the name without its qualifier, or write "(x).field" to select a field of a composite value. +CREATE FUNCTION rpr_compfn(p rpr_pair) RETURNS SETOF int +LANGUAGE sql AS $$ + SELECT price FROM stock + WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS price > (p).lo) +$$; +SELECT count(*) FROM rpr_compfn(ROW(0, 0)::rpr_pair); + count +------- + 20 +(1 row) + +DROP FUNCTION rpr_compfn(rpr_pair); +DROP TYPE rpr_pair; +-- The DEFINE rules apply to the names the ref hooks leave to the query +-- parser. Under use_variable resolution PL/pgSQL answers first and keeps +-- any name one of its variables owns, so the qualified spelling rejected +-- above is resolved by PL/pgSQL here and never reaches the rule. +CREATE FUNCTION rpr_plfn_var(threshold int) RETURNS bigint +LANGUAGE plpgsql AS $$ +#variable_conflict use_variable +DECLARE + n bigint; +BEGIN + SELECT count(*) INTO n FROM ( + SELECT price FROM stock + WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS price > rpr_plfn_var.threshold) + ) s; + RETURN n; +END +$$; +SELECT rpr_plfn_var(0); + rpr_plfn_var +-------------- + 20 +(1 row) + +DROP FUNCTION rpr_plfn_var(int); +-- The same applies to a pattern variable's name. Under the default +-- resolution PL/pgSQL declines the name, so the reservation is reached and +-- the collision is reported rather than resolved. +CREATE FUNCTION rpr_conflictfn_err() RETURNS bigint +LANGUAGE plpgsql AS $$ +DECLARE + a stock%ROWTYPE; + n bigint; +BEGIN + a.price := 95; + SELECT count(*) INTO n FROM ( + SELECT price FROM stock + WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS price > a.price) + ) s; + RETURN n; +END +$$; +SELECT rpr_conflictfn_err(); +ERROR: pattern variable qualified expression "a.price" is not supported in DEFINE clause +LINE 8: DEFINE A AS price > a.price) + ^ +QUERY: SELECT count(*) FROM ( + SELECT price FROM stock + WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS price > a.price) + ) s +CONTEXT: PL/pgSQL function rpr_conflictfn_err() line 7 at SQL statement +DROP FUNCTION rpr_conflictfn_err(); +CREATE FUNCTION rpr_conflictfn() RETURNS bigint +LANGUAGE plpgsql AS $$ +#variable_conflict use_variable +DECLARE + a stock%ROWTYPE; + n bigint; +BEGIN + a.price := 95; + SELECT count(*) INTO n FROM ( + SELECT price FROM stock + WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS price > a.price) + ) s; + RETURN n; +END +$$; +SELECT rpr_conflictfn(); + rpr_conflictfn +---------------- + 20 +(1 row) + +DROP FUNCTION rpr_conflictfn(); +-- An outer range variable used as a function-call qualifier reaches DEFINE as +-- a FuncExpr rather than a Var, so the level the qualifier resolved at, not +-- the shape of the resulting node, is what identifies the outer reference. +CREATE TABLE rpr_outer (threshold int); +INSERT INTO rpr_outer VALUES (95); +CREATE FUNCTION rpr_rowfn(rpr_outer) RETURNS int LANGUAGE sql AS 'SELECT 1'; +SELECT * FROM rpr_outer AS o, +LATERAL ( + SELECT price FROM stock + WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS o.rpr_rowfn > 0 + ) +) s; +ERROR: cannot use outer query column in DEFINE clause +LINE 9: DEFINE A AS o.rpr_rowfn > 0 + ^ +DROP FUNCTION rpr_rowfn(rpr_outer); +DROP TABLE rpr_outer; -- DEFINE rejects a schema-qualified column reference (three or more name -- parts) once it resolves; the qualified form itself is not allowed. (stock -- is a temp table, so it is qualified with pg_temp here.) @@ -1351,12 +1638,13 @@ WINDOW w AS ( PATTERN (A) DEFINE A AS (pg_temp.stock.*) IS NOT NULL ); -ERROR: qualified expression "pg_temp.stock.*" is not allowed in DEFINE clause +ERROR: whole-row reference is not allowed in DEFINE clause LINE 7: DEFINE A AS (pg_temp.stock.*) IS NOT NULL ^ --- A two-part table-qualified whole-row reference is rejected as well, through --- a separate range-variable check (a bare relation name is instead accepted --- as a whole-row Var). +HINT: A DEFINE condition may reference individual columns only. +-- A two-part table-qualified whole-row reference is rejected as well, and by +-- the whole-row check rather than by a qualifier rule: the error names the +-- whole-row reference, not the qualifier. -- 2-part (table.*): SELECT price FROM stock WINDOW w AS ( @@ -1366,9 +1654,227 @@ WINDOW w AS ( PATTERN (A) DEFINE A AS (stock.*) IS NOT NULL ); -ERROR: range variable qualified expression "stock.*" is not allowed in DEFINE clause +ERROR: whole-row reference is not allowed in DEFINE clause LINE 7: DEFINE A AS (stock.*) IS NOT NULL ^ +HINT: A DEFINE condition may reference individual columns only. +-- A row constructor reaches the same references through +-- transformExpressionList(), which expanded the star by RTE before either +-- check could see it. The first four below were accepted and returned rows; +-- the fifth was rejected, but as a missing FROM-clause entry. +-- ROW(schema.table.*): +SELECT price FROM stock +WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS ROW(pg_temp.stock.*) IS NOT NULL +); +ERROR: whole-row reference is not allowed in DEFINE clause +LINE 7: DEFINE A AS ROW(pg_temp.stock.*) IS NOT NULL + ^ +HINT: A DEFINE condition may reference individual columns only. +-- ROW(table.*): +SELECT price FROM stock +WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS ROW(stock.*) IS NOT NULL +); +ERROR: whole-row reference is not allowed in DEFINE clause +LINE 7: DEFINE A AS ROW(stock.*) IS NOT NULL + ^ +HINT: A DEFINE condition may reference individual columns only. +-- the ROW keyword is optional, so the bare constructor needs the same +-- treatment: +SELECT price FROM stock +WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS (stock.*, 1) IS NOT NULL +); +ERROR: whole-row reference is not allowed in DEFINE clause +LINE 7: DEFINE A AS (stock.*, 1) IS NOT NULL + ^ +HINT: A DEFINE condition may reference individual columns only. +-- redundant parentheses are not a way around it: +SELECT price FROM stock +WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS ROW((stock.*)) IS NOT NULL +); +ERROR: whole-row reference is not allowed in DEFINE clause +LINE 7: DEFINE A AS ROW((stock.*)) IS NOT NULL + ^ +HINT: A DEFINE condition may reference individual columns only. +-- a pattern variable qualifier is a separate class of rejection: +SELECT price FROM stock +WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS ROW(A.*) IS NOT NULL +); +ERROR: pattern variable qualified expression "a.*" is not supported in DEFINE clause +LINE 7: DEFINE A AS ROW(A.*) IS NOT NULL + ^ +-- The plain two-part form is the one the standard writes its DEFINE examples +-- with, and it is decided on the qualifier alone, before resolution. +SELECT price FROM stock +WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS A.price > 100 +); +ERROR: pattern variable qualified expression "a.price" is not supported in DEFINE clause +LINE 7: DEFINE A AS A.price > 100 + ^ +-- Deciding on the qualifier alone means a pattern variable takes a name a +-- range variable would otherwise answer to: the rejection names the pattern +-- variable, not the alias, even though "a" is a live alias here. +SELECT price FROM stock AS a +WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS a.price > 100 +); +ERROR: pattern variable qualified expression "a.price" is not supported in DEFINE clause +LINE 7: DEFINE A AS a.price > 100 + ^ +-- Each rejection above classifies the reference only after it resolves, so a +-- misspelled column keeps the diagnosis and the suggestion it gets anywhere +-- else. Only the two-part form changed: its gate used to fire on the +-- qualifier alone and report a range variable problem before the rest of the +-- name was looked at. The three-part gate already ran after resolution. +SELECT price FROM stock +WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS stock.pric > 0 +); +ERROR: column stock.pric does not exist +LINE 7: DEFINE A AS stock.pric > 0 + ^ +HINT: Perhaps you meant to reference the column "stock.price". +SELECT price FROM stock +WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS pg_temp.stock.pric > 0 +); +ERROR: column stock.pric does not exist +LINE 7: DEFINE A AS pg_temp.stock.pric > 0 + ^ +HINT: Perhaps you meant to reference the column "stock.price". +-- the same typo outside a DEFINE clause, for comparison: +SELECT price FROM stock WHERE stock.pric > 0; +ERROR: column stock.pric does not exist +LINE 1: SELECT price FROM stock WHERE stock.pric > 0; + ^ +HINT: Perhaps you meant to reference the column "stock.price". +-- Retrying an unresolved column as a function call on the whole row builds a +-- whole-row reference the query does not contain. That must not be reported +-- as one, and must not let the reference through either: rpr_tag(rpr_stock) +-- below resolves, so the retry succeeds and the result is rejected by the +-- qualifier rules rather than by the whole-row check. +CREATE FUNCTION rpr_tag(rpr_stock) RETURNS int + LANGUAGE sql IMMUTABLE AS $$SELECT 1$$; +SELECT price FROM rpr_stock +WINDOW w AS ( + PARTITION BY part_id + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS rpr_stock.rpr_tag > 0 +); +ERROR: range variable qualified expression "rpr_stock.rpr_tag" is not allowed in DEFINE clause +LINE 7: DEFINE A AS rpr_stock.rpr_tag > 0 + ^ +SELECT price FROM rpr_stock +WINDOW w AS ( + PARTITION BY part_id + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS public.rpr_stock.rpr_tag > 0 +); +ERROR: qualified expression "public.rpr_stock.rpr_tag" is not allowed in DEFINE clause +LINE 7: DEFINE A AS public.rpr_stock.rpr_tag > 0 + ^ +DROP FUNCTION rpr_tag(rpr_stock); +-- A JOIN USING alias has no whole-row Var of its own, so the same retry +-- expands it to a row constructor instead. That arm is only reachable inside +-- DEFINE now that the retry is no longer rejected on sight. +CREATE TEMP TABLE rpr_j_l (x int, y int); +CREATE TEMP TABLE rpr_j_r (x int, z int); +SELECT count(*) OVER w FROM (rpr_j_l JOIN rpr_j_r USING (x)) j +WINDOW w AS ( + ORDER BY x + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A+) + DEFINE A AS j.yy > 0 +); +ERROR: column j.yy does not exist +LINE 6: DEFINE A AS j.yy > 0 + ^ +SELECT count(*) OVER w FROM (rpr_j_l JOIN rpr_j_r USING (x)) j +WINDOW w AS ( + ORDER BY x + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A+) + DEFINE A AS j.y > 0 +); +ERROR: range variable qualified expression "j.y" is not allowed in DEFINE clause +LINE 6: DEFINE A AS j.y > 0 + ^ +SELECT count(*) OVER w FROM (rpr_j_l JOIN rpr_j_r USING (x)) j +WINDOW w AS ( + ORDER BY x + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A+) + DEFINE A AS (j.*) IS NOT NULL +); +ERROR: whole-row reference is not allowed in DEFINE clause +LINE 6: DEFINE A AS (j.*) IS NOT NULL + ^ +HINT: A DEFINE condition may reference individual columns only. +DROP TABLE rpr_j_l, rpr_j_r; +-- A row constructor over plain columns is unaffected. +SELECT company, tdate, count(*) OVER w AS cnt +FROM stock +WHERE company = 'company2' AND tdate <= '2023-07-03' +WINDOW w AS ( + PARTITION BY company + ORDER BY tdate + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A+) + DEFINE A AS ROW(price, price) IS NOT NULL +); + company | tdate | cnt +----------+------------+----- + company2 | 07-01-2023 | 3 + company2 | 07-02-2023 | 0 + company2 | 07-03-2023 | 0 +(3 rows) + -- -- 2-arg PREV/NEXT: functional tests -- diff --git a/src/test/regress/expected/rpr_base.out b/src/test/regress/expected/rpr_base.out index 03eb329b415..3f767959e8d 100644 --- a/src/test/regress/expected/rpr_base.out +++ b/src/test/regress/expected/rpr_base.out @@ -3823,6 +3823,21 @@ SELECT pg_get_viewdef('rpr_pin_v'::regclass, true) t (1 row) +-- The hazard this section guards against cannot be written in the first +-- place: a whole-row reference through a row constructor is rejected in +-- DEFINE, so no view can carry one as far as the deparser. +CREATE VIEW rpr_pin_row_v AS +SELECT count(*) OVER w AS cnt +FROM rpr_pin, rpr_pin_other +WHERE rpr_pin.id = rpr_pin_other.id +WINDOW w AS (ORDER BY rpr_pin.id + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A+) + DEFINE A AS ROW(rpr_pin.*) IS NOT NULL); +ERROR: whole-row reference is not allowed in DEFINE clause +LINE 8: DEFINE A AS ROW(rpr_pin.*) IS NOT NULL); + ^ +HINT: A DEFINE condition may reference individual columns only. -- a column merged by USING is pinned the same way CREATE TABLE rpr_pin_l (x INT, y INT); CREATE TABLE rpr_pin_r (x INT, z INT); @@ -4996,6 +5011,24 @@ WINDOW w AS ( ERROR: range variable qualified expression "rpr_composite.items" is not allowed in DEFINE clause LINE 7: DEFINE A AS (rpr_composite.items).amount > 10 ^ +-- A trailing star on a composite column is a different thing from a trailing +-- star on a relation: it names no relation, so the row constructor keeps +-- expanding it and the DEFINE restrictions do not apply. +SELECT COUNT(*) OVER w +FROM rpr_composite +WINDOW w AS ( + ORDER BY id + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A+) + DEFINE A AS ROW((items).*) IS NOT NULL +); + count +------- + 3 + 0 + 0 +(3 rows) + DROP TABLE rpr_composite; DROP TYPE rpr_item; -- ERROR: undefined column in DEFINE diff --git a/src/test/regress/expected/rpr_integration.out b/src/test/regress/expected/rpr_integration.out index 7084abcfd13..3309be12655 100644 --- a/src/test/regress/expected/rpr_integration.out +++ b/src/test/regress/expected/rpr_integration.out @@ -16,7 +16,7 @@ -- A2. Run condition pushdown bypass -- A3. Window dedup prevention (RPR vs non-RPR) -- A4. Window dedup prevention (same PATTERN, different DEFINE) --- A5. Unused window removal prevention +-- A5. Unused output removal around an RPR window -- A6. Inverse transition bypass -- A7. Cost estimation RPR awareness -- A8. Subquery flattening prevention @@ -928,42 +928,57 @@ SELECT c FROM ( (18 rows) DROP TABLE rpr_integ_two; --- Whole-row Var in DEFINE. Writing the bare relation name (rpr_integ) in --- DEFINE resolves to a whole-row Var (attribute number 0). The parser's junk --- targetlist entry carries it into the WindowAgg's input like any other +-- Whole-row Var in DEFINE is not allowed +SELECT sum(c) FROM ( + SELECT val, count(*) OVER w AS c FROM rpr_integ + WINDOW w AS (ORDER BY id + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A B+) + DEFINE B AS rpr_integ IS NOT NULL) +) t; +ERROR: whole-row reference is not allowed in DEFINE clause +LINE 6: DEFINE B AS rpr_integ IS NOT NULL) + ^ +HINT: A DEFINE condition may reference individual columns only. +-- It still reaches a DEFINE clause without being written there: pulling up a +-- subquery substitutes that subquery's output expressions into defineClause, +-- and one of them can be a whole-row Var (attribute number 0). The parser's +-- junk targetlist entry carries it into the WindowAgg's input like any other -- DEFINE column, so the pattern match sees the full row regardless of what -- the subquery projects. The unused scalar output "val" is therefore free to -- be replaced with NULL (nothing reads it), while c is kept because sum(c) -- reads it; the match result is unchanged. EXPLAIN (VERBOSE, COSTS OFF) SELECT sum(c) FROM ( - SELECT val, count(*) OVER w AS c FROM rpr_integ + SELECT val, count(*) OVER w AS c + FROM (SELECT r, r.id AS id, r.val AS val FROM rpr_integ r) s WINDOW w AS (ORDER BY id ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A B+) - DEFINE B AS rpr_integ IS NOT NULL) + DEFINE B AS r IS NOT NULL) ) t; - QUERY PLAN ------------------------------------------------------------------------------------------------ + QUERY PLAN +--------------------------------------------------------------------------------------- Aggregate Output: sum((count(*) OVER w)) -> WindowAgg - Output: NULL::integer, count(*) OVER w, rpr_integ.id, rpr_integ.* - Window: w AS (ORDER BY rpr_integ.id ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING) + Output: NULL::integer, count(*) OVER w, r.id, r.* + Window: w AS (ORDER BY r.id ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING) Pattern: a b+ -> Sort - Output: rpr_integ.id, rpr_integ.* - Sort Key: rpr_integ.id - -> Seq Scan on public.rpr_integ - Output: rpr_integ.id, rpr_integ.* + Output: r.id, r.* + Sort Key: r.id + -> Seq Scan on public.rpr_integ r + Output: r.id, r.* (11 rows) SELECT sum(c) FROM ( - SELECT val, count(*) OVER w AS c FROM rpr_integ + SELECT val, count(*) OVER w AS c + FROM (SELECT r, r.id AS id, r.val AS val FROM rpr_integ r) s WINDOW w AS (ORDER BY id ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A B+) - DEFINE B AS rpr_integ IS NOT NULL) + DEFINE B AS r IS NOT NULL) ) t; sum ----- diff --git a/src/test/regress/sql/rpr.sql b/src/test/regress/sql/rpr.sql index 6bb4adfe320..5b48d1e7dbc 100644 --- a/src/test/regress/sql/rpr.sql +++ b/src/test/regress/sql/rpr.sql @@ -701,6 +701,232 @@ LATERAL ( ) ) s; +-- An outer range variable is subject to the same two rules as a local one: a +-- whole-row reference is rejected as one, and a name that does not resolve +-- keeps its own diagnosis rather than being reported as a qualifier problem. +SELECT * FROM (VALUES (95)) AS o(threshold), +LATERAL ( + SELECT price FROM stock + WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS (o.*) IS NOT NULL + ) +) s; +SELECT * FROM (VALUES (95)) AS o(threshold), +LATERAL ( + SELECT price FROM stock + WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS o.threshhold > 0 + ) +) s; + +-- A two-part name is not always a range variable qualifier: a SQL function's +-- parameter and a PL/pgSQL variable both resolve through +-- p_post_columnref_hook. The qualifier slot is reserved all the same, so +-- these are rejected for the spelling, not for what they name. +CREATE FUNCTION rpr_sqlfn(threshold int) RETURNS SETOF int +LANGUAGE sql AS $$ + SELECT price FROM stock + WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS price > rpr_sqlfn.threshold) +$$; + +CREATE FUNCTION rpr_plfn(threshold int) RETURNS bigint +LANGUAGE plpgsql AS $$ +DECLARE + n bigint; +BEGIN + SELECT count(*) INTO n FROM ( + SELECT price FROM stock + WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS price > rpr_plfn.threshold) + ) s; + RETURN n; +END +$$; +SELECT rpr_plfn(0); +DROP FUNCTION rpr_plfn(int); + +-- Unqualified, the same parameter is readable. +CREATE FUNCTION rpr_sqlfn(threshold int) RETURNS SETOF int +LANGUAGE sql AS $$ + SELECT price FROM stock + WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS price > threshold) +$$; +SELECT count(*) FROM rpr_sqlfn(0); +DROP FUNCTION rpr_sqlfn(int); + +-- The qualifier slot is decided on the qualifier alone, before resolution, so +-- a pattern variable takes the slot even from the routine that contains the +-- query. Naming a pattern variable after the function makes rpr_pv.threshold +-- the pattern variable's, and the reservation is reported; that the function +-- has a parameter of that name, and the query has no such column, does not +-- enter into it. +CREATE FUNCTION rpr_pv(threshold int) RETURNS SETOF int +LANGUAGE sql AS $$ + SELECT price FROM stock + WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (rpr_pv) + DEFINE rpr_pv AS price > rpr_pv.threshold) +$$; + +-- The collision is in the qualifier, not in the DEFINE variable being +-- defined: any pattern variable of that name reserves it. +CREATE FUNCTION rpr_pv(threshold int) RETURNS SETOF int +LANGUAGE sql AS $$ + SELECT price FROM stock + WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (rpr_pv A) + DEFINE A AS price > rpr_pv.threshold) +$$; + +-- A field of a composite parameter has no unqualified spelling, so it is +-- reached by parenthesizing the value: "(p).lo" selects a field rather than +-- qualifying a name, and occupies no qualifier slot. +CREATE TYPE rpr_pair AS (lo int, hi int); +CREATE FUNCTION rpr_compfn(p rpr_pair) RETURNS SETOF int +LANGUAGE sql AS $$ + SELECT price FROM stock + WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS price > p.lo) +$$; +CREATE FUNCTION rpr_compfn(p rpr_pair) RETURNS SETOF int +LANGUAGE sql AS $$ + SELECT price FROM stock + WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS price > (p).lo) +$$; +SELECT count(*) FROM rpr_compfn(ROW(0, 0)::rpr_pair); +DROP FUNCTION rpr_compfn(rpr_pair); +DROP TYPE rpr_pair; + +-- The DEFINE rules apply to the names the ref hooks leave to the query +-- parser. Under use_variable resolution PL/pgSQL answers first and keeps +-- any name one of its variables owns, so the qualified spelling rejected +-- above is resolved by PL/pgSQL here and never reaches the rule. +CREATE FUNCTION rpr_plfn_var(threshold int) RETURNS bigint +LANGUAGE plpgsql AS $$ +#variable_conflict use_variable +DECLARE + n bigint; +BEGIN + SELECT count(*) INTO n FROM ( + SELECT price FROM stock + WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS price > rpr_plfn_var.threshold) + ) s; + RETURN n; +END +$$; +SELECT rpr_plfn_var(0); +DROP FUNCTION rpr_plfn_var(int); + +-- The same applies to a pattern variable's name. Under the default +-- resolution PL/pgSQL declines the name, so the reservation is reached and +-- the collision is reported rather than resolved. +CREATE FUNCTION rpr_conflictfn_err() RETURNS bigint +LANGUAGE plpgsql AS $$ +DECLARE + a stock%ROWTYPE; + n bigint; +BEGIN + a.price := 95; + SELECT count(*) INTO n FROM ( + SELECT price FROM stock + WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS price > a.price) + ) s; + RETURN n; +END +$$; +SELECT rpr_conflictfn_err(); +DROP FUNCTION rpr_conflictfn_err(); + +CREATE FUNCTION rpr_conflictfn() RETURNS bigint +LANGUAGE plpgsql AS $$ +#variable_conflict use_variable +DECLARE + a stock%ROWTYPE; + n bigint; +BEGIN + a.price := 95; + SELECT count(*) INTO n FROM ( + SELECT price FROM stock + WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS price > a.price) + ) s; + RETURN n; +END +$$; +SELECT rpr_conflictfn(); +DROP FUNCTION rpr_conflictfn(); + +-- An outer range variable used as a function-call qualifier reaches DEFINE as +-- a FuncExpr rather than a Var, so the level the qualifier resolved at, not +-- the shape of the resulting node, is what identifies the outer reference. +CREATE TABLE rpr_outer (threshold int); +INSERT INTO rpr_outer VALUES (95); +CREATE FUNCTION rpr_rowfn(rpr_outer) RETURNS int LANGUAGE sql AS 'SELECT 1'; +SELECT * FROM rpr_outer AS o, +LATERAL ( + SELECT price FROM stock + WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS o.rpr_rowfn > 0 + ) +) s; +DROP FUNCTION rpr_rowfn(rpr_outer); +DROP TABLE rpr_outer; + -- DEFINE rejects a schema-qualified column reference (three or more name -- parts) once it resolves; the qualified form itself is not allowed. (stock -- is a temp table, so it is qualified with pg_temp here.) @@ -722,9 +948,9 @@ WINDOW w AS ( PATTERN (A) DEFINE A AS (pg_temp.stock.*) IS NOT NULL ); --- A two-part table-qualified whole-row reference is rejected as well, through --- a separate range-variable check (a bare relation name is instead accepted --- as a whole-row Var). +-- A two-part table-qualified whole-row reference is rejected as well, and by +-- the whole-row check rather than by a qualifier rule: the error names the +-- whole-row reference, not the qualifier. -- 2-part (table.*): SELECT price FROM stock WINDOW w AS ( @@ -735,6 +961,167 @@ WINDOW w AS ( DEFINE A AS (stock.*) IS NOT NULL ); +-- A row constructor reaches the same references through +-- transformExpressionList(), which expanded the star by RTE before either +-- check could see it. The first four below were accepted and returned rows; +-- the fifth was rejected, but as a missing FROM-clause entry. +-- ROW(schema.table.*): +SELECT price FROM stock +WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS ROW(pg_temp.stock.*) IS NOT NULL +); +-- ROW(table.*): +SELECT price FROM stock +WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS ROW(stock.*) IS NOT NULL +); +-- the ROW keyword is optional, so the bare constructor needs the same +-- treatment: +SELECT price FROM stock +WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS (stock.*, 1) IS NOT NULL +); +-- redundant parentheses are not a way around it: +SELECT price FROM stock +WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS ROW((stock.*)) IS NOT NULL +); +-- a pattern variable qualifier is a separate class of rejection: +SELECT price FROM stock +WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS ROW(A.*) IS NOT NULL +); +-- The plain two-part form is the one the standard writes its DEFINE examples +-- with, and it is decided on the qualifier alone, before resolution. +SELECT price FROM stock +WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS A.price > 100 +); +-- Deciding on the qualifier alone means a pattern variable takes a name a +-- range variable would otherwise answer to: the rejection names the pattern +-- variable, not the alias, even though "a" is a live alias here. +SELECT price FROM stock AS a +WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS a.price > 100 +); +-- Each rejection above classifies the reference only after it resolves, so a +-- misspelled column keeps the diagnosis and the suggestion it gets anywhere +-- else. Only the two-part form changed: its gate used to fire on the +-- qualifier alone and report a range variable problem before the rest of the +-- name was looked at. The three-part gate already ran after resolution. +SELECT price FROM stock +WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS stock.pric > 0 +); +SELECT price FROM stock +WINDOW w AS ( + PARTITION BY company + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS pg_temp.stock.pric > 0 +); +-- the same typo outside a DEFINE clause, for comparison: +SELECT price FROM stock WHERE stock.pric > 0; + +-- Retrying an unresolved column as a function call on the whole row builds a +-- whole-row reference the query does not contain. That must not be reported +-- as one, and must not let the reference through either: rpr_tag(rpr_stock) +-- below resolves, so the retry succeeds and the result is rejected by the +-- qualifier rules rather than by the whole-row check. +CREATE FUNCTION rpr_tag(rpr_stock) RETURNS int + LANGUAGE sql IMMUTABLE AS $$SELECT 1$$; +SELECT price FROM rpr_stock +WINDOW w AS ( + PARTITION BY part_id + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS rpr_stock.rpr_tag > 0 +); +SELECT price FROM rpr_stock +WINDOW w AS ( + PARTITION BY part_id + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A) + DEFINE A AS public.rpr_stock.rpr_tag > 0 +); +DROP FUNCTION rpr_tag(rpr_stock); + +-- A JOIN USING alias has no whole-row Var of its own, so the same retry +-- expands it to a row constructor instead. That arm is only reachable inside +-- DEFINE now that the retry is no longer rejected on sight. +CREATE TEMP TABLE rpr_j_l (x int, y int); +CREATE TEMP TABLE rpr_j_r (x int, z int); +SELECT count(*) OVER w FROM (rpr_j_l JOIN rpr_j_r USING (x)) j +WINDOW w AS ( + ORDER BY x + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A+) + DEFINE A AS j.yy > 0 +); +SELECT count(*) OVER w FROM (rpr_j_l JOIN rpr_j_r USING (x)) j +WINDOW w AS ( + ORDER BY x + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A+) + DEFINE A AS j.y > 0 +); +SELECT count(*) OVER w FROM (rpr_j_l JOIN rpr_j_r USING (x)) j +WINDOW w AS ( + ORDER BY x + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A+) + DEFINE A AS (j.*) IS NOT NULL +); +DROP TABLE rpr_j_l, rpr_j_r; + +-- A row constructor over plain columns is unaffected. +SELECT company, tdate, count(*) OVER w AS cnt +FROM stock +WHERE company = 'company2' AND tdate <= '2023-07-03' +WINDOW w AS ( + PARTITION BY company + ORDER BY tdate + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL + PATTERN (A+) + DEFINE A AS ROW(price, price) IS NOT NULL +); + -- -- 2-arg PREV/NEXT: functional tests -- diff --git a/src/test/regress/sql/rpr_base.sql b/src/test/regress/sql/rpr_base.sql index 93cb431c921..7ba578dc50a 100644 --- a/src/test/regress/sql/rpr_base.sql +++ b/src/test/regress/sql/rpr_base.sql @@ -2463,6 +2463,18 @@ CREATE VIEW rpr_pin_v2 AS SELECT pg_get_viewdef('rpr_pin_v'::regclass, true) = pg_get_viewdef('rpr_pin_v2'::regclass, true) AS identical; +-- The hazard this section guards against cannot be written in the first +-- place: a whole-row reference through a row constructor is rejected in +-- DEFINE, so no view can carry one as far as the deparser. +CREATE VIEW rpr_pin_row_v AS +SELECT count(*) OVER w AS cnt +FROM rpr_pin, rpr_pin_other +WHERE rpr_pin.id = rpr_pin_other.id +WINDOW w AS (ORDER BY rpr_pin.id + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A+) + DEFINE A AS ROW(rpr_pin.*) IS NOT NULL); + -- a column merged by USING is pinned the same way CREATE TABLE rpr_pin_l (x INT, y INT); CREATE TABLE rpr_pin_r (x INT, z INT); @@ -3205,6 +3217,18 @@ WINDOW w AS ( PATTERN (A+) DEFINE A AS (rpr_composite.items).amount > 10 ); + +-- A trailing star on a composite column is a different thing from a trailing +-- star on a relation: it names no relation, so the row constructor keeps +-- expanding it and the DEFINE restrictions do not apply. +SELECT COUNT(*) OVER w +FROM rpr_composite +WINDOW w AS ( + ORDER BY id + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A+) + DEFINE A AS ROW((items).*) IS NOT NULL +); DROP TABLE rpr_composite; DROP TYPE rpr_item; diff --git a/src/test/regress/sql/rpr_integration.sql b/src/test/regress/sql/rpr_integration.sql index 95b4a829417..1b236122dc3 100644 --- a/src/test/regress/sql/rpr_integration.sql +++ b/src/test/regress/sql/rpr_integration.sql @@ -16,7 +16,7 @@ -- A2. Run condition pushdown bypass -- A3. Window dedup prevention (RPR vs non-RPR) -- A4. Window dedup prevention (same PATTERN, different DEFINE) --- A5. Unused window removal prevention +-- A5. Unused output removal around an RPR window -- A6. Inverse transition bypass -- A7. Cost estimation RPR awareness -- A8. Subquery flattening prevention @@ -548,28 +548,40 @@ SELECT c FROM ( DROP TABLE rpr_integ_two; --- Whole-row Var in DEFINE. Writing the bare relation name (rpr_integ) in --- DEFINE resolves to a whole-row Var (attribute number 0). The parser's junk --- targetlist entry carries it into the WindowAgg's input like any other +-- Whole-row Var in DEFINE is not allowed +SELECT sum(c) FROM ( + SELECT val, count(*) OVER w AS c FROM rpr_integ + WINDOW w AS (ORDER BY id + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A B+) + DEFINE B AS rpr_integ IS NOT NULL) +) t; + +-- It still reaches a DEFINE clause without being written there: pulling up a +-- subquery substitutes that subquery's output expressions into defineClause, +-- and one of them can be a whole-row Var (attribute number 0). The parser's +-- junk targetlist entry carries it into the WindowAgg's input like any other -- DEFINE column, so the pattern match sees the full row regardless of what -- the subquery projects. The unused scalar output "val" is therefore free to -- be replaced with NULL (nothing reads it), while c is kept because sum(c) -- reads it; the match result is unchanged. EXPLAIN (VERBOSE, COSTS OFF) SELECT sum(c) FROM ( - SELECT val, count(*) OVER w AS c FROM rpr_integ + SELECT val, count(*) OVER w AS c + FROM (SELECT r, r.id AS id, r.val AS val FROM rpr_integ r) s WINDOW w AS (ORDER BY id ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A B+) - DEFINE B AS rpr_integ IS NOT NULL) + DEFINE B AS r IS NOT NULL) ) t; SELECT sum(c) FROM ( - SELECT val, count(*) OVER w AS c FROM rpr_integ + SELECT val, count(*) OVER w AS c + FROM (SELECT r, r.id AS id, r.val AS val FROM rpr_integ r) s WINDOW w AS (ORDER BY id ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A B+) - DEFINE B AS rpr_integ IS NOT NULL) + DEFINE B AS r IS NOT NULL) ) t; -- The walk that decides which windows are still live runs on a targetlist