From 02523d535df4cd4f4f6ece5ee78aa2be90b22be8 Mon Sep 17 00:00:00 2001 From: jian he Date: Mon, 10 Aug 2026 12:03:19 +0900 Subject: [PATCH] Correct resno and dedup handling of RPR DEFINE targetlist entries transformDefineClause() adds a junk TargetEntry for each column that only a DEFINE clause references, and took its resno from list_length(*targetlist) + 1. That value is right for the entry being added, but it leaves pstate->p_next_resno -- the authority for the next resno -- where it was, so the next junk entry drawn from it repeats a resno DEFINE has already used. Every other clause that extends the targetlist is transformed before the window definitions, which leaves a later window's PARTITION BY or ORDER BY key as the only thing that can draw next: planning such a query tripped the apply_tlist_labeling() assertion, and non-assert builds carried a duplicate-resno Query onward. Draw from p_next_resno instead; that also restores the count free_parsestate() checks against MaxTupleAttributeNumber. The dedup scan in the same loop compared only varno and varattno. A DEFINE expression may only reference the query level it appears in, so the entry the scan looks for always has varlevelsup 0 -- but an outer reference already in the targetlist of a lateral subquery can carry the same varno and varattno at varlevelsup 1. The scan read that as a match and skipped the junk entry, leaving the DEFINE column out of the subplan targetlist, and setrefs.c failed with "variable not found in subplan target list". Use equal(), which compares varlevelsup along with the rest of the Var. --- src/backend/parser/parse_rpr.c | 6 +- src/test/regress/expected/rpr_integration.out | 94 +++++++++++++++++++ src/test/regress/sql/rpr_integration.sql | 70 ++++++++++++++ 3 files changed, 166 insertions(+), 4 deletions(-) diff --git a/src/backend/parser/parse_rpr.c b/src/backend/parser/parse_rpr.c index ac29c26e208..21d3b8899bb 100644 --- a/src/backend/parser/parse_rpr.c +++ b/src/backend/parser/parse_rpr.c @@ -379,9 +379,7 @@ transformDefineClause(ParseState *pstate, WindowDef *windef, foreach_node(TargetEntry, tle, *targetlist) { - if (IsA(tle->expr, Var) && - ((Var *) tle->expr)->varno == var->varno && - ((Var *) tle->expr)->varattno == var->varattno) + if (equal(tle->expr, var)) { found = true; break; @@ -392,7 +390,7 @@ transformDefineClause(ParseState *pstate, WindowDef *windef, TargetEntry *newtle; newtle = makeTargetEntry((Expr *) copyObject(var), - list_length(*targetlist) + 1, + (AttrNumber) pstate->p_next_resno++, NULL, true); *targetlist = lappend(*targetlist, newtle); diff --git a/src/test/regress/expected/rpr_integration.out b/src/test/regress/expected/rpr_integration.out index d0b5cc5a44f..8acedf63464 100644 --- a/src/test/regress/expected/rpr_integration.out +++ b/src/test/regress/expected/rpr_integration.out @@ -1353,6 +1353,50 @@ ORDER BY o.id, r.id; 10 | 8 | 3 (6 rows) +-- A lateral outer reference can share varno and varattno with a DEFINE-only +-- column: here o.b and y are both attribute 2 at their own query levels. +-- Only varlevelsup separates them, so the junk targetlist entry for y has to +-- be added even though a Var with the same varno and varattno is present. +CREATE TABLE rpr_lat_o (a int, b int); +CREATE TABLE rpr_lat_i (x int, y int); +INSERT INTO rpr_lat_o VALUES (1, 10); +INSERT INTO rpr_lat_i VALUES (1, 5), (2, 6); +-- The overlapping shape: the outer reference is o.b, attribute 2. +SELECT * +FROM rpr_lat_o o, +LATERAL ( + SELECT o.b AS lat, count(*) OVER w AS c + FROM rpr_lat_i + WINDOW w AS (ORDER BY x + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A+) + DEFINE A AS y > 0) +) s; + a | b | lat | c +---+----+-----+--- + 1 | 10 | 10 | 2 + 1 | 10 | 10 | 0 +(2 rows) + +-- Control: the outer reference is o.a, attribute 1, which cannot be mistaken +-- for y. The counts must match the query above. +SELECT * +FROM rpr_lat_o o, +LATERAL ( + SELECT o.a AS lat, count(*) OVER w AS c + FROM rpr_lat_i + WINDOW w AS (ORDER BY x + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A+) + DEFINE A AS y > 0) +) s; + a | b | lat | c +---+----+-----+--- + 1 | 10 | 1 | 2 + 1 | 10 | 1 | 0 +(2 rows) + +DROP TABLE rpr_lat_o, rpr_lat_i; -- ============================================================ -- B7. RPR + Recursive CTE -- ============================================================ @@ -1603,6 +1647,56 @@ SELECT cnt FROM ( (16 rows) DROP TABLE rpr_over1, rpr_over2; +-- A DEFINE-only column and a later window's sort key both become junk +-- targetlist entries. Each draws its resno from p_next_resno, which is what +-- keeps the two distinct: a targetlist that gives one resno to two entries is +-- not a valid Query, and the parser is the only place that can prevent it. +SELECT id, count(*) OVER w1 AS c1, count(*) OVER w2 AS c2 +FROM (VALUES (1,1,10),(2,1,20)) t(id, grp, val) +WINDOW w1 AS (ORDER BY id + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (S U+) + DEFINE U AS val > PREV(val)), + w2 AS (PARTITION BY grp ORDER BY id) +ORDER BY id; + id | c1 | c2 +----+----+---- + 1 | 2 | 1 + 2 | 0 | 2 +(2 rows) + +-- The same pair reached through a plain ORDER BY window. +SELECT id, count(*) OVER w1 AS c1, count(*) OVER w2 AS c2 +FROM (VALUES (1,1,10),(2,1,20)) t(id, grp, val) +WINDOW w1 AS (ORDER BY id + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (S U+) + DEFINE U AS val > PREV(val)), + w2 AS (ORDER BY grp) +ORDER BY id; + id | c1 | c2 +----+----+---- + 1 | 2 | 2 + 2 | 0 | 2 +(2 rows) + +-- Control: the opposite declaration order draws the sort key first, so the two +-- never compete for a resno. It must return the same rows as the first query +-- above. +SELECT id, count(*) OVER w1 AS c1, count(*) OVER w2 AS c2 +FROM (VALUES (1,1,10),(2,1,20)) t(id, grp, val) +WINDOW w2 AS (PARTITION BY grp ORDER BY id), + w1 AS (ORDER BY id + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (S U+) + DEFINE U AS val > PREV(val)) +ORDER BY id; + id | c1 | c2 +----+----+---- + 1 | 2 | 1 + 2 | 0 | 2 +(2 rows) + -- Cleanup DROP TABLE rpr_integ; DROP TABLE rpr_integ2; diff --git a/src/test/regress/sql/rpr_integration.sql b/src/test/regress/sql/rpr_integration.sql index c5f5e850925..a79b518bf9f 100644 --- a/src/test/regress/sql/rpr_integration.sql +++ b/src/test/regress/sql/rpr_integration.sql @@ -839,6 +839,41 @@ LATERAL ( WHERE r.cnt > 0 AND o.id IN (5, 10) ORDER BY o.id, r.id; +-- A lateral outer reference can share varno and varattno with a DEFINE-only +-- column: here o.b and y are both attribute 2 at their own query levels. +-- Only varlevelsup separates them, so the junk targetlist entry for y has to +-- be added even though a Var with the same varno and varattno is present. +CREATE TABLE rpr_lat_o (a int, b int); +CREATE TABLE rpr_lat_i (x int, y int); +INSERT INTO rpr_lat_o VALUES (1, 10); +INSERT INTO rpr_lat_i VALUES (1, 5), (2, 6); + +-- The overlapping shape: the outer reference is o.b, attribute 2. +SELECT * +FROM rpr_lat_o o, +LATERAL ( + SELECT o.b AS lat, count(*) OVER w AS c + FROM rpr_lat_i + WINDOW w AS (ORDER BY x + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A+) + DEFINE A AS y > 0) +) s; + +-- Control: the outer reference is o.a, attribute 1, which cannot be mistaken +-- for y. The counts must match the query above. +SELECT * +FROM rpr_lat_o o, +LATERAL ( + SELECT o.a AS lat, count(*) OVER w AS c + FROM rpr_lat_i + WINDOW w AS (ORDER BY x + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A+) + DEFINE A AS y > 0) +) s; +DROP TABLE rpr_lat_o, rpr_lat_i; + -- ============================================================ -- B7. RPR + Recursive CTE -- ============================================================ @@ -1004,6 +1039,41 @@ SELECT cnt FROM ( ) s; DROP TABLE rpr_over1, rpr_over2; +-- A DEFINE-only column and a later window's sort key both become junk +-- targetlist entries. Each draws its resno from p_next_resno, which is what +-- keeps the two distinct: a targetlist that gives one resno to two entries is +-- not a valid Query, and the parser is the only place that can prevent it. +SELECT id, count(*) OVER w1 AS c1, count(*) OVER w2 AS c2 +FROM (VALUES (1,1,10),(2,1,20)) t(id, grp, val) +WINDOW w1 AS (ORDER BY id + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (S U+) + DEFINE U AS val > PREV(val)), + w2 AS (PARTITION BY grp ORDER BY id) +ORDER BY id; + +-- The same pair reached through a plain ORDER BY window. +SELECT id, count(*) OVER w1 AS c1, count(*) OVER w2 AS c2 +FROM (VALUES (1,1,10),(2,1,20)) t(id, grp, val) +WINDOW w1 AS (ORDER BY id + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (S U+) + DEFINE U AS val > PREV(val)), + w2 AS (ORDER BY grp) +ORDER BY id; + +-- Control: the opposite declaration order draws the sort key first, so the two +-- never compete for a resno. It must return the same rows as the first query +-- above. +SELECT id, count(*) OVER w1 AS c1, count(*) OVER w2 AS c2 +FROM (VALUES (1,1,10),(2,1,20)) t(id, grp, val) +WINDOW w2 AS (PARTITION BY grp ORDER BY id), + w1 AS (ORDER BY id + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (S U+) + DEFINE U AS val > PREV(val)) +ORDER BY id; + -- Cleanup DROP TABLE rpr_integ; DROP TABLE rpr_integ2;