From 39bb9f4d281b5be83671c153f655d294d6701449 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 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, and drop the integration tests that relied on the old behaviour. 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 a list of individual column Vars, so ROW(t.*) reached neither the range-variable check in transformColumnRef() nor this one 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, which is the failure being prevented here. Skip the expansion when the expression kind is EXPR_KIND_RPR_DEFINE and let transformExpr() reach the checks instead. The A_Indirection arm keeps expanding: (c).* over a composite column names no relation and is a legitimate reference. The gate this installs is still not sound, in two ways. It turns a misspelled column into a whole-row diagnosis. Any qualified name whose last part is not a column reaches transformWholeRowRef(), so inside DEFINE DEFINE A AS public.stock.pric > 0 ERROR: whole-row reference is not allowed in DEFINE clause HINT: A DEFINE condition may reference individual columns only. while the same typo elsewhere still says the column does not exist and offers "Perhaps you meant to reference the column stock.price". The user is told about a construct they did not write, and loses the suggestion. The error class does not match its neighbours. The two restrictions a few lines up divide as ERRCODE_FEATURE_NOT_SUPPORTED for what the standard allows but this implementation does not, and ERRCODE_SYNTAX_ERROR for what the standard itself forbids. Whole-row is the latter and is raised as SYNTAX_ERROR, which is consistent -- but only by accident, since nothing states the rule and the aggregate restriction nearby breaks it. --- src/backend/parser/parse_expr.c | 11 +++ src/backend/parser/parse_target.c | 13 ++- src/test/regress/expected/rpr.out | 88 ++++++++++++++++++- src/test/regress/expected/rpr_base.out | 14 +++ src/test/regress/expected/rpr_integration.out | 42 ++------- src/test/regress/sql/rpr.sql | 63 +++++++++++++ src/test/regress/sql/rpr_base.sql | 12 +++ src/test/regress/sql/rpr_integration.sql | 19 +--- 8 files changed, 206 insertions(+), 56 deletions(-) diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index be0f99381ad..b3ee1e03ab0 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -2755,6 +2755,17 @@ static Node * transformWholeRowRef(ParseState *pstate, 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) + 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..d8a6810744a 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)); diff --git a/src/test/regress/expected/rpr.out b/src/test/regress/expected/rpr.out index c4958c1b8d8..ee710f1b407 100644 --- a/src/test/regress/expected/rpr.out +++ b/src/test/regress/expected/rpr.out @@ -1351,9 +1351,10 @@ 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 ^ +HINT: A DEFINE condition may reference individual columns only. -- 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). @@ -1369,6 +1370,91 @@ WINDOW w AS ( ERROR: range variable qualified expression "stock.*" is not allowed in DEFINE clause LINE 7: 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 +); +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: range variable qualified expression "stock.*" is not allowed in DEFINE clause +LINE 7: 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 +); +ERROR: range variable qualified expression "stock.*" is not allowed in DEFINE clause +LINE 7: 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 +); +ERROR: range variable qualified expression "stock.*" is not allowed in DEFINE clause +LINE 7: 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 +); +ERROR: pattern variable qualified expression "a.*" is not supported in DEFINE clause +LINE 7: DEFINE A AS ROW(A.*) IS NOT NULL + ^ +-- 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 5483f43558b..1480dcc022e 100644 --- a/src/test/regress/expected/rpr_base.out +++ b/src/test/regress/expected/rpr_base.out @@ -3837,6 +3837,20 @@ 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: range variable qualified expression "rpr_pin.*" is not allowed in DEFINE clause +LINE 8: 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); diff --git a/src/test/regress/expected/rpr_integration.out b/src/test/regress/expected/rpr_integration.out index 2b300d71801..2034410a09c 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 @@ -702,14 +702,7 @@ WINDOW w AS (ORDER BY t.id (10 rows) DROP TABLE rpr_integ_u; --- 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 --- 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) +-- 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 @@ -717,33 +710,10 @@ SELECT sum(c) FROM ( PATTERN (A B+) DEFINE B AS rpr_integ IS NOT NULL) ) t; - 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) - 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.* -(11 rows) - -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; - sum ------ - 10 -(1 row) - +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. -- ============================================================ -- A6. Inverse transition bypass -- ============================================================ diff --git a/src/test/regress/sql/rpr.sql b/src/test/regress/sql/rpr.sql index 6bb4adfe320..9ee9c664043 100644 --- a/src/test/regress/sql/rpr.sql +++ b/src/test/regress/sql/rpr.sql @@ -735,6 +735,69 @@ 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 +); +-- 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 cb84d828f38..f76d6abd4ab 100644 --- a/src/test/regress/sql/rpr_base.sql +++ b/src/test/regress/sql/rpr_base.sql @@ -2469,6 +2469,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); diff --git a/src/test/regress/sql/rpr_integration.sql b/src/test/regress/sql/rpr_integration.sql index 498aa12b127..d63b0b50140 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 @@ -428,22 +428,7 @@ WINDOW w AS (ORDER BY t.id DROP TABLE rpr_integ_u; --- 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 --- 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 - 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; - +-- 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