From 532f2376acc567670635116a85d9e0d73bd2bb24 Mon Sep 17 00:00:00 2001 From: jian he Date: Tue, 21 Jul 2026 20:47:05 +0900 Subject: [PATCH] Fix RPR junk targetlist entries and DEFINE diagnostics transformDefineClause() adds a junk TargetEntry for each column that only a DEFINE clause references. Two defects in that code: Its resno came from list_length(*targetlist) + 1 rather than pstate->p_next_resno. A later window's junk sort key does consume p_next_resno, so the two could be handed the same resno; planning such a query tripped the apply_tlist_labeling() assertion, and non-assert builds were left with a duplicate-resno Query. Its dedup scan compared only varno and varattno. A lateral outer reference can share both with a local column that DEFINE references, so the junk entry was skipped; the planner then replaced the outer reference with a PARAM_EXEC and setrefs.c failed with "variable not found in subplan target list". Use equal() instead, which also covers vartype and varnullingrels divergence. Two diagnostics were misleading. define_walker() tested nesting depth before testing whether the inner navigation is the whole argument, so sibling navigations such as PREV(FIRST(v) + LAST(v)) were rejected as nesting more than two levels deep; swapping the tests reports them as not being a direct argument and leaves genuine three-level nesting alone. The grammar rule for a quantifier written as two operator tokens reported an unusable second token without naming it, and pointed the cursor at the first token, which is the valid one; the neighbouring rules for "*" and "+" quantifiers already name the token and point at it. Also refresh comments that no longer matched the code: allStatesAbsorbable is pinned false once a context records a match rather than merely fluctuating, a compound navigation's inner offset is deliberately walked twice, absorption is skipped under SKIP TO NEXT ROW because overlapping matches make it impossible rather than because contexts are discarded immediately, and pull_var_clause() may be called with no flags because DEFINE rejects aggregates, window functions and subqueries at parse time. Initialize the RPR navigation markpos to -1 for consistency with the other window objects; begin_partition() overwrites it either way. Add regression tests for both junk targetlist cases and for the sibling navigation diagnostic. The quantifier message was already covered, so its expected output changes here. While at it, give the integration tests that had accumulated at the end of the file their own numbered sections, matching the convention the rest of that file already follows. --- src/backend/executor/README.rpr | 8 +- src/backend/executor/execRPR.c | 12 +- src/backend/executor/nodeWindowAgg.c | 2 +- src/backend/optimizer/path/allpaths.c | 5 + src/backend/optimizer/plan/rpr.c | 10 +- src/backend/parser/gram.y | 4 +- src/backend/parser/parse_rpr.c | 29 ++- src/include/nodes/execnodes.h | 6 +- src/test/regress/expected/rpr_base.out | 22 +- src/test/regress/expected/rpr_integration.out | 190 +++++++++++++++--- src/test/regress/sql/rpr_base.sql | 11 + src/test/regress/sql/rpr_integration.sql | 168 +++++++++++++--- 12 files changed, 369 insertions(+), 98 deletions(-) diff --git a/src/backend/executor/README.rpr b/src/backend/executor/README.rpr index 23d979111bd..4970386155d 100644 --- a/src/backend/executor/README.rpr +++ b/src/backend/executor/README.rpr @@ -956,11 +956,13 @@ Two boolean flags make the absorption decision efficient: states. Once false, it never becomes true again. - allStatesAbsorbable (dynamic: can fluctuate) + allStatesAbsorbable (dynamic until a match is recorded) "Can this context be absorbed?" - true if all states are in an absorbable region. + true if all states are in an absorbable region and no match is + recorded. Becomes false when a non-absorbable state is added; reverts to true - when it is removed. + when it is removed. Recording a match also sets it false and that + does not revert, since absorbing would free the match. VIII-5. Absorption Order diff --git a/src/backend/executor/execRPR.c b/src/backend/executor/execRPR.c index 48d01e4a50a..16515c2aebb 100644 --- a/src/backend/executor/execRPR.c +++ b/src/backend/executor/execRPR.c @@ -177,8 +177,9 @@ static void nfa_reevaluate_dependent_vars(WindowAggState *winstate, * - Monotonic: true->false only (optimization: skip recalc when false) * * ctx.allStatesAbsorbable: can this context be absorbed? - * - True if ALL states have isAbsorbable=true - * - Dynamic: can change false->true (when non-absorbable states die) + * - True if ALL states have isAbsorbable=true and no match is recorded + * - Dynamic: false->true when non-absorbable states die, but a + * recorded match pins it false * * Absorption Algorithm: * For each pair (older Ctx1, newer Ctx2): @@ -586,9 +587,10 @@ nfa_record_context_absorbed(WindowAggState *winstate, int64 absorbedLen) * hasAbsorbableState: true if context has at least one absorbable state. * This flag is monotonic (true -> false only). Once all absorbable states * die, no new absorbable states can be created through transitions. - * allStatesAbsorbable: true if ALL states in context are absorbable. - * This flag is dynamic and can change false -> true when non-absorbable - * states die off. + * allStatesAbsorbable: true if ALL states in context are absorbable and no + * match is recorded. Dynamic (false -> true as non-absorbable states die + * off), except that a recorded match pins it false: absorbing would free + * a match no absorbing context can reproduce. * * Optimization: Once hasAbsorbableState becomes false, both flags remain false * permanently, so we skip recalculation. diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c index 1361d03a293..10a2184d7e2 100644 --- a/src/backend/executor/nodeWindowAgg.c +++ b/src/backend/executor/nodeWindowAgg.c @@ -1289,7 +1289,7 @@ prepare_tuplestore(WindowAggState *winstate) winstate->nav_winobj->readptr = tuplestore_alloc_read_pointer(winstate->buffer, EXEC_FLAG_BACKWARD); - winstate->nav_winobj->markpos = 0; + winstate->nav_winobj->markpos = -1; } /* diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index 559c6049243..06c90fcb98c 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -4945,6 +4945,11 @@ remove_unused_subquery_outputs(Query *subquery, RelOptInfo *rel, { if (wc->defineClause != NIL) { + /* + * flags == 0 is safe: DEFINE rejects aggregates, window + * functions and subqueries at parse time, and this runs + * before any PlaceHolderVar could be planted. + */ List *vars = pull_var_clause((Node *) wc->defineClause, 0); foreach_node(Var, dvar, vars) diff --git a/src/backend/optimizer/plan/rpr.c b/src/backend/optimizer/plan/rpr.c index 7a881e44187..b2c678752c5 100644 --- a/src/backend/optimizer/plan/rpr.c +++ b/src/backend/optimizer/plan/rpr.c @@ -2062,10 +2062,12 @@ buildRPRPattern(RPRPatternNode *pattern, List *defineClause, * * Runtime conditions for absorption: * - * 1. SKIP TO PAST LAST ROW required (not SKIP TO NEXT ROW): With NEXT - * ROW, after each match the search resumes from the next row, so contexts - * are immediately discarded. No redundant contexts accumulate, making - * absorption unnecessary. + * 1. SKIP TO PAST LAST ROW required (not SKIP TO NEXT ROW): with NEXT + * ROW, matches overlap and every row must report its own match, so + * absorption (sharing one result) is not semantically possible. A + * completed context does linger until its own start row is queried; that + * is the inherent cost of per-row match reporting, not redundancy + * absorption could remove. * * 2. Unbounded frame end required (not ROWS with bounded end): With a * bounded frame (e.g., ROWS BETWEEN CURRENT ROW AND 10 FOLLOWING), diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 60c589577b7..56a05c3eea6 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -17941,9 +17941,9 @@ row_pattern_quantifier_opt: else ereport(ERROR, errcode(ERRCODE_SYNTAX_ERROR), - errmsg("invalid quantifier combination"), + errmsg("invalid token \"%s\" after \"?\" quantifier", rpr_invalid_quantifier_token($2)), errhint("Did you mean \"??\" for reluctant quantifier?"), - parser_errposition(@1)); + parser_errposition(@2)); } /* {n}, {n,}, {,m}, {n,m} quantifiers */ | '{' Iconst '}' diff --git a/src/backend/parser/parse_rpr.c b/src/backend/parser/parse_rpr.c index 0b8766269e1..67465da9831 100644 --- a/src/backend/parser/parse_rpr.c +++ b/src/backend/parser/parse_rpr.c @@ -371,6 +371,10 @@ transformDefineClause(ParseState *pstate, WindowDef *windef, * one is present in the targetlist. This is needed so the planner * propagates the referenced columns through the plan tree, making * them available to the WindowAgg's DEFINE evaluation. + * + * Compare with equal(): a lateral outer reference can share varno and + * varattno with a local column that DEFINE references, and conflating + * them would drop the DEFINE column from the targetlist. */ vars = pull_var_clause(expr, 0); foreach_node(Var, var, vars) @@ -379,9 +383,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 +394,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); @@ -446,7 +448,11 @@ transformDefineClause(ParseState *pstate, WindowDef *windef, * walks nav.arg in PHASE_NAV_ARG to collect nesting/column-ref state, * applies compound flatten or raises a nesting error, then walks the * (post-flatten) offset(s) in PHASE_NAV_OFFSET to enforce the - * constant-offset and no-nested-nav rules. No subtree is walked twice. + * constant-offset and no-nested-nav rules. A compound form's inner + * offset is covered by both walks: the PHASE_NAV_ARG pass only asks + * whether nav.arg as a whole holds a column reference, so the offset + * is walked again in PHASE_NAV_OFFSET to catch one it would have + * leaked. */ /* @@ -467,7 +473,7 @@ transformDefineClause(ParseState *pstate, WindowDef *windef, * Var sightings feed the column-ref rule for the enclosing nav scope; * RPRNavExpr sightings inside PHASE_NAV_ARG feed the nesting decision. * See the comment block above DefinePhase for the overall design and - * how each subtree is walked exactly once. + * the phase transitions. */ static bool define_walker(Node *node, void *context) @@ -538,18 +544,19 @@ define_walker(Node *node, void *context) { RPRNavExpr *inner; - /* Reject triple-or-deeper nesting */ - if (ctx->nav_count > 1) + /* Reject an inner nav that is not the whole argument */ + if (!IsA(nav->arg, RPRNavExpr)) ereport(ERROR, errcode(ERRCODE_SYNTAX_ERROR), - errmsg("cannot nest row pattern navigation more than two levels deep"), + errmsg("row pattern navigation operation must be a direct argument of the outer navigation"), errhint("Only PREV(FIRST()), PREV(LAST()), NEXT(FIRST()), and NEXT(LAST()) compound forms are allowed."), parser_errposition(ctx->pstate, nav->location)); - if (!IsA(nav->arg, RPRNavExpr)) + /* Reject triple-or-deeper nesting; siblings caught above */ + if (ctx->nav_count > 1) ereport(ERROR, errcode(ERRCODE_SYNTAX_ERROR), - errmsg("row pattern navigation operation must be a direct argument of the outer navigation"), + errmsg("cannot nest row pattern navigation more than two levels deep"), errhint("Only PREV(FIRST()), PREV(LAST()), NEXT(FIRST()), and NEXT(LAST()) compound forms are allowed."), parser_errposition(ctx->pstate, nav->location)); diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 6a3216bbf8d..c808951b5ae 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -2595,8 +2595,10 @@ typedef struct RPRNFAState * hasAbsorbableState: can this context absorb others? (>=1 absorbable state) * - Monotonic: true->false only, cannot recover once false * - Used to skip absorption attempts once all absorbable states are gone - * allStatesAbsorbable: can this context be absorbed? (ALL states absorbable) - * - Dynamic: can change false->true (when non-absorbable states die) + * allStatesAbsorbable: can this context be absorbed? (ALL states + * absorbable, no recorded match) + * - Dynamic: false->true when non-absorbable states die; a recorded + * match pins it false * - Used to determine if this context is eligible for absorption */ typedef struct RPRNFAContext diff --git a/src/test/regress/expected/rpr_base.out b/src/test/regress/expected/rpr_base.out index 1468c145e84..f56935f2dcc 100644 --- a/src/test/regress/expected/rpr_base.out +++ b/src/test/regress/expected/rpr_base.out @@ -3940,8 +3940,8 @@ LINE 1: ...N CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A* *?|B) DEFI... ^ HINT: Did you mean "*?" for reluctant quantifier? SELECT count(*) OVER w FROM rpr_glue WINDOW w AS (ORDER BY id ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A? *?|B) DEFINE A AS val > 0, B AS val <= 0); -ERROR: invalid quantifier combination -LINE 1: ...EEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A? *?|B) DE... +ERROR: invalid token "*?" after "?" quantifier +LINE 1: ...N CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A? *?|B) DEFI... ^ HINT: Did you mean "??" for reluctant quantifier? SELECT count(*) OVER w FROM rpr_glue WINDOW w AS (ORDER BY id ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A{2}*?|B) DEFINE A AS val > 0, B AS val <= 0); @@ -4013,8 +4013,8 @@ LINE 1: ...S BETWEEN CURRENT ROW AND 1 FOLLOWING PATTERN (A* ?+) DEFINE... ^ HINT: Did you mean "*?" for reluctant quantifier? SELECT FROM rpr_err WINDOW w AS ( ROWS BETWEEN CURRENT ROW AND 1 FOLLOWING PATTERN (A? ??) DEFINE A AS TRUE); -ERROR: invalid quantifier combination -LINE 1: ...OWS BETWEEN CURRENT ROW AND 1 FOLLOWING PATTERN (A? ??) DEFI... +ERROR: invalid token "??" after "?" quantifier +LINE 1: ...S BETWEEN CURRENT ROW AND 1 FOLLOWING PATTERN (A? ??) DEFINE... ^ HINT: Did you mean "??" for reluctant quantifier? SELECT FROM rpr_err WINDOW w AS ( ROWS BETWEEN CURRENT ROW AND 1 FOLLOWING PATTERN (A {1,2}??) DEFINE A AS TRUE); @@ -4369,6 +4369,20 @@ ERROR: cannot nest row pattern navigation more than two levels deep LINE 6: DEFINE A AS PREV(FIRST(PREV(v))) > 0 ^ HINT: Only PREV(FIRST()), PREV(LAST()), NEXT(FIRST()), and NEXT(LAST()) compound forms are allowed. +-- Sibling navigations: prohibited, but they are not a deeper nesting, +-- so the inner navigation must be reported as not being the direct +-- argument rather than as a third level. +SELECT count(*) OVER w +FROM generate_series(1,10) s(v) +WINDOW w AS ( + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A+) + DEFINE A AS PREV(FIRST(v) + LAST(v)) > 0 +); +ERROR: row pattern navigation operation must be a direct argument of the outer navigation +LINE 6: DEFINE A AS PREV(FIRST(v) + LAST(v)) > 0 + ^ +HINT: Only PREV(FIRST()), PREV(LAST()), NEXT(FIRST()), and NEXT(LAST()) compound forms are allowed. -- A navigation offset must be a run-time constant, not a navigation operation SELECT count(*) OVER w FROM generate_series(1,10) s(v) diff --git a/src/test/regress/expected/rpr_integration.out b/src/test/regress/expected/rpr_integration.out index bf9f9766836..027f91f35b5 100644 --- a/src/test/regress/expected/rpr_integration.out +++ b/src/test/regress/expected/rpr_integration.out @@ -29,7 +29,11 @@ -- B7. RPR + Recursive CTE -- B8. RPR + Incremental sort -- B9. RPR + Volatile function in DEFINE --- B10. RPR + Correlated subquery +-- B10. RPR + Correlated subquery in WHERE +-- B11. RPR + Junk targetlist pruning +-- B12. RPR + Correlated navigation offsets +-- B13. RPR + DEFINE-only parameter caching +-- B14. RPR + Multiple window definitions -- CREATE TABLE rpr_integ (id INT, val INT); INSERT INTO rpr_integ VALUES @@ -1353,6 +1357,53 @@ ORDER BY o.id, r.id; 10 | 8 | 3 (6 rows) +-- A lateral outer reference carried in the subquery's SELECT list can +-- share varno and varattno with a local column that only DEFINE +-- references. The two must not be taken for one targetlist entry: +-- the planner turns the outer reference into a PARAM_EXEC, so +-- dropping the junk entry would leave the DEFINE column out of every +-- subplan targetlist and setrefs.c would fail with "variable not +-- found in subplan target list". Here o.b and the DEFINE column y +-- are both attribute 2 of relation 1 at their own query levels. +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); +-- Result: the shared attribute number does not disturb the match. +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) + +-- Result: attribute 1 never collided; the counts must match 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 -- ============================================================ @@ -1566,13 +1617,18 @@ ORDER BY o.id; 10 | 45 | 2 (10 rows) --- A column referenced only by DEFINE must not keep an unrelated column that --- merely shares its attribute number. DEFINE references a (rpr_over1); c --- (rpr_over2) has the same attno but is unused, so it must be dropped. +-- ============================================================ +-- B11. RPR + Junk targetlist pruning +-- ============================================================ +-- Verify that the junk targetlist entry planted for a DEFINE-only +-- column does not keep an unrelated column alive. DEFINE references +-- a (rpr_over1); c (rpr_over2) carries the same attribute number but +-- is unused, so the plan must drop it. CREATE TABLE rpr_over1 (a int); CREATE TABLE rpr_over2 (c int); INSERT INTO rpr_over1 VALUES (1),(2),(3); INSERT INTO rpr_over2 VALUES (1),(2),(3); +-- Plan: only the DEFINE column survives in the subquery output. EXPLAIN (VERBOSE, COSTS OFF) SELECT cnt FROM ( SELECT a AS oa, c AS oc, count(*) OVER w AS cnt @@ -1601,11 +1657,16 @@ SELECT cnt FROM ( (16 rows) DROP TABLE rpr_over1, rpr_over2; --- A row pattern navigation offset that resolves to a correlated PARAM_EXEC --- (here through SRF inlining of rpr_srf_f(g.n)) must be re-resolved on every --- rescan, not frozen at executor init. The inlined WindowAgg is the inner --- side of a nestloop and is rescanned once per outer row, so each row sees its --- own PREV(v, n) offset; a frozen offset would report the same value for all. +-- ============================================================ +-- B12. RPR + Correlated navigation offsets +-- ============================================================ +-- Verify that a navigation offset resolving to a correlated +-- PARAM_EXEC is re-resolved on every rescan rather than frozen at +-- executor init. Each function below is inlined into the inner side +-- of a nestloop and rescanned once per outer row, so every row must +-- see its own offset; a frozen offset would report one value for all. +-- Three shapes are covered: a backward PREV offset, a forward +-- FIRST-family offset, and a compound navigation's outer offset. CREATE TABLE rpr_srf (v int); INSERT INTO rpr_srf SELECT generate_series(1, 10); CREATE FUNCTION rpr_srf_f(k int) RETURNS SETOF bigint AS $$ @@ -1614,8 +1675,8 @@ CREATE FUNCTION rpr_srf_f(k int) RETURNS SETOF bigint AS $$ WINDOW w AS (ORDER BY v ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A+) DEFINE A AS v > PREV(v, k)) $$ LANGUAGE sql STABLE; --- The offset reads "runtime"; the WindowAgg inlines into the nestloop and is --- rescanned per outer row. +-- Plan: the offset reads "runtime"; the WindowAgg inlines into the +-- nestloop and is rescanned per outer row. EXPLAIN (COSTS OFF) SELECT g.n, max(s) FROM (VALUES (1), (2), (3)) g(n), LATERAL rpr_srf_f(g.n) s GROUP BY g.n ORDER BY g.n; @@ -1636,7 +1697,8 @@ GROUP BY g.n ORDER BY g.n; -> Seq Scan on rpr_srf (13 rows) --- Each outer row yields its own offset (9, 8, 7), not one frozen value. +-- Result: each outer row yields its own offset (9, 8, 7), not one +-- frozen value. SELECT g.n, max(s) AS m FROM (VALUES (1), (2), (3)) g(n), LATERAL rpr_srf_f(g.n) s GROUP BY g.n ORDER BY g.n; n | m @@ -1646,18 +1708,18 @@ GROUP BY g.n ORDER BY g.n; 3 | 7 (3 rows) --- A forward FIRST-family offset with a correlated PARAM_EXEC must likewise be --- re-resolved per scan (navFirstOffset / navFirstOffsetKind), not frozen at init. --- PATTERN (B A+) anchors the match start at B so A can reference FIRST(v, k) --- k rows ahead; each outer k yields its own forward offset (k=0 matches all --- ten rows, k>=1 makes the first A fail), proving per-scan re-resolution. +-- A forward FIRST-family offset must likewise be re-resolved per scan +-- (navFirstOffset / navFirstOffsetKind). PATTERN (B A+) anchors the +-- match start at B so A can reference FIRST(v, k) k rows ahead, and +-- each outer k yields its own forward offset. CREATE FUNCTION rpr_srf_first(k int) RETURNS SETOF bigint AS $$ SELECT count(*) OVER w FROM rpr_srf WINDOW w AS (ORDER BY v ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (B A+) DEFINE A AS v > FIRST(v, k)) $$ LANGUAGE sql STABLE; --- The forward offset reads "runtime" and the WindowAgg inlines into the nestloop. +-- Plan: the forward offset reads "runtime" and the WindowAgg inlines +-- into the nestloop. EXPLAIN (COSTS OFF) SELECT g.n, max(s) FROM (VALUES (0), (1), (2)) g(n), LATERAL rpr_srf_first(g.n) s GROUP BY g.n ORDER BY g.n; @@ -1679,8 +1741,8 @@ GROUP BY g.n ORDER BY g.n; -> Seq Scan on rpr_srf (14 rows) --- k=0 -> 10, k>=1 -> 0: distinct per outer row, so the forward offset is not --- frozen at ExecInit. +-- Result: k=0 matches all ten rows and k>=1 makes the first A fail, +-- so the forward offset is not frozen at ExecInit. SELECT g.n, max(s) AS m FROM (VALUES (0), (1), (2)) g(n), LATERAL rpr_srf_first(g.n) s GROUP BY g.n ORDER BY g.n; n | m @@ -1691,18 +1753,19 @@ GROUP BY g.n ORDER BY g.n; (3 rows) DROP FUNCTION rpr_srf_first(int); --- A compound navigation whose OUTER offset is a correlated PARAM_EXEC must be --- re-resolved per scan as well. An outer offset that overflows int64 flips the --- backward trim to RETAIN_ALL for that scan only; a smaller offset on the next --- rescan must go back to FIXED. k=1 -> 9, k=3 -> 7, k=overflow -> 0 (out of --- range, no match), proving both the per-scan outer-offset resolution and the --- RETAIN_ALL <-> FIXED toggle across rescans. +-- A compound navigation's OUTER offset must be re-resolved per scan +-- as well. An outer offset that overflows int64 flips the backward +-- trim to RETAIN_ALL for that scan only; a smaller offset on the next +-- rescan must go back to FIXED. CREATE FUNCTION rpr_srf_cmp(k int8) RETURNS SETOF bigint AS $$ SELECT count(*) OVER w FROM rpr_srf WINDOW w AS (ORDER BY v ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A B+) DEFINE B AS v > PREV(LAST(v, 1), k)) $$ LANGUAGE sql STABLE; +-- Result: k=1 -> 9, k=3 -> 7, overflow -> 0 (out of range, no match), +-- proving both the per-scan outer-offset resolution and the +-- RETAIN_ALL <-> FIXED toggle across rescans. SELECT g.n, max(s) AS m FROM (VALUES (1::int8), (3::int8), (9223372036854775807::int8)) g(n), LATERAL rpr_srf_cmp(g.n) s @@ -1717,13 +1780,16 @@ GROUP BY g.n ORDER BY g.n; DROP FUNCTION rpr_srf_cmp(int8); DROP FUNCTION rpr_srf_f(int); DROP TABLE rpr_srf; --- A correlated PARAM_EXEC that appears only inside DEFINE must be registered in --- the WindowAgg's extParam so the chgParam signal reaches a caching node above. --- Here the inlined function's argument is used only in DEFINE, DISTINCT is --- planned as a HashAgg, and HashAgg rescan is gated on chgParam; without the --- param in extParam the hash table built for the first outer row would be --- re-served for the rest. Each threshold must get its own answer set --- (10 -> {0, 90}, 200 -> {0}); a stale cache would add a spurious 200|90. +-- ============================================================ +-- B13. RPR + DEFINE-only parameter caching +-- ============================================================ +-- Verify that a correlated PARAM_EXEC appearing only inside DEFINE is +-- registered in the WindowAgg's extParam, so the chgParam signal +-- reaches a caching node above. Here the inlined function's argument +-- is used only in DEFINE, DISTINCT is planned as a HashAgg, and +-- HashAgg rescan is gated on chgParam; without the param in extParam +-- the hash table built for the first outer row would be re-served for +-- the rest. CREATE TABLE rpr_hcache_thr (threshold int); INSERT INTO rpr_hcache_thr VALUES (10), (200); CREATE TABLE rpr_hcache_stock (price int); @@ -1733,6 +1799,8 @@ CREATE FUNCTION rpr_hcache_fn(th int) RETURNS SETOF bigint LANGUAGE sql STABLE A WINDOW w AS (ORDER BY price ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING AFTER MATCH SKIP PAST LAST ROW INITIAL PATTERN (a+) DEFINE a AS price > th) $$; +-- Result: each threshold gets its own answer set (10 -> {0, 90}, +-- 200 -> {0}); a stale cache would add a spurious 200|90. SELECT o.threshold, f FROM rpr_hcache_thr o, LATERAL rpr_hcache_fn(o.threshold) f ORDER BY 1, 2; threshold | f @@ -1744,6 +1812,62 @@ ORDER BY 1, 2; DROP FUNCTION rpr_hcache_fn(int); DROP TABLE rpr_hcache_thr, rpr_hcache_stock; +-- ============================================================ +-- B14. RPR + Multiple window definitions +-- ============================================================ +-- Verify that a column referenced only by DEFINE coexists with a +-- later window's PARTITION BY / ORDER BY key that the SELECT list +-- does not carry. Both are added to the targetlist as junk entries +-- and must draw their resno from p_next_resno; otherwise the two +-- collide and apply_tlist_labeling trips an assertion during +-- planning. Here val is DEFINE-only and grp belongs to w2, with the +-- RPR window defined first so its junk entry is created first. +-- Result: the DEFINE-only column and the later window's key coexist. +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) + +-- Result: same collision 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) + +-- Result: defining the windows in the opposite order never collided, +-- and must keep returning 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_base.sql b/src/test/regress/sql/rpr_base.sql index be11cd35941..96645142b76 100644 --- a/src/test/regress/sql/rpr_base.sql +++ b/src/test/regress/sql/rpr_base.sql @@ -2846,6 +2846,17 @@ WINDOW w AS ( DEFINE A AS PREV(FIRST(PREV(v))) > 0 ); +-- Sibling navigations: prohibited, but they are not a deeper nesting, +-- so the inner navigation must be reported as not being the direct +-- argument rather than as a third level. +SELECT count(*) OVER w +FROM generate_series(1,10) s(v) +WINDOW w AS ( + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A+) + DEFINE A AS PREV(FIRST(v) + LAST(v)) > 0 +); + -- A navigation offset must be a run-time constant, not a navigation operation SELECT count(*) OVER w FROM generate_series(1,10) s(v) diff --git a/src/test/regress/sql/rpr_integration.sql b/src/test/regress/sql/rpr_integration.sql index f3a2cf86ffd..4be35d64f1e 100644 --- a/src/test/regress/sql/rpr_integration.sql +++ b/src/test/regress/sql/rpr_integration.sql @@ -29,7 +29,11 @@ -- B7. RPR + Recursive CTE -- B8. RPR + Incremental sort -- B9. RPR + Volatile function in DEFINE --- B10. RPR + Correlated subquery +-- B10. RPR + Correlated subquery in WHERE +-- B11. RPR + Junk targetlist pruning +-- B12. RPR + Correlated navigation offsets +-- B13. RPR + DEFINE-only parameter caching +-- B14. RPR + Multiple window definitions -- CREATE TABLE rpr_integ (id INT, val INT); @@ -839,6 +843,44 @@ LATERAL ( WHERE r.cnt > 0 AND o.id IN (5, 10) ORDER BY o.id, r.id; +-- A lateral outer reference carried in the subquery's SELECT list can +-- share varno and varattno with a local column that only DEFINE +-- references. The two must not be taken for one targetlist entry: +-- the planner turns the outer reference into a PARAM_EXEC, so +-- dropping the junk entry would leave the DEFINE column out of every +-- subplan targetlist and setrefs.c would fail with "variable not +-- found in subplan target list". Here o.b and the DEFINE column y +-- are both attribute 2 of relation 1 at their own query levels. +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); + +-- Result: the shared attribute number does not disturb the match. +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; + +-- Result: attribute 1 never collided; the counts must match 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 -- ============================================================ @@ -988,13 +1030,19 @@ SELECT o.id, o.val, FROM rpr_integ o ORDER BY o.id; --- A column referenced only by DEFINE must not keep an unrelated column that --- merely shares its attribute number. DEFINE references a (rpr_over1); c --- (rpr_over2) has the same attno but is unused, so it must be dropped. +-- ============================================================ +-- B11. RPR + Junk targetlist pruning +-- ============================================================ +-- Verify that the junk targetlist entry planted for a DEFINE-only +-- column does not keep an unrelated column alive. DEFINE references +-- a (rpr_over1); c (rpr_over2) carries the same attribute number but +-- is unused, so the plan must drop it. CREATE TABLE rpr_over1 (a int); CREATE TABLE rpr_over2 (c int); INSERT INTO rpr_over1 VALUES (1),(2),(3); INSERT INTO rpr_over2 VALUES (1),(2),(3); + +-- Plan: only the DEFINE column survives in the subquery output. EXPLAIN (VERBOSE, COSTS OFF) SELECT cnt FROM ( SELECT a AS oa, c AS oc, count(*) OVER w AS cnt @@ -1004,11 +1052,16 @@ SELECT cnt FROM ( ) s; DROP TABLE rpr_over1, rpr_over2; --- A row pattern navigation offset that resolves to a correlated PARAM_EXEC --- (here through SRF inlining of rpr_srf_f(g.n)) must be re-resolved on every --- rescan, not frozen at executor init. The inlined WindowAgg is the inner --- side of a nestloop and is rescanned once per outer row, so each row sees its --- own PREV(v, n) offset; a frozen offset would report the same value for all. +-- ============================================================ +-- B12. RPR + Correlated navigation offsets +-- ============================================================ +-- Verify that a navigation offset resolving to a correlated +-- PARAM_EXEC is re-resolved on every rescan rather than frozen at +-- executor init. Each function below is inlined into the inner side +-- of a nestloop and rescanned once per outer row, so every row must +-- see its own offset; a frozen offset would report one value for all. +-- Three shapes are covered: a backward PREV offset, a forward +-- FIRST-family offset, and a compound navigation's outer offset. CREATE TABLE rpr_srf (v int); INSERT INTO rpr_srf SELECT generate_series(1, 10); CREATE FUNCTION rpr_srf_f(k int) RETURNS SETOF bigint AS $$ @@ -1017,48 +1070,50 @@ CREATE FUNCTION rpr_srf_f(k int) RETURNS SETOF bigint AS $$ WINDOW w AS (ORDER BY v ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A+) DEFINE A AS v > PREV(v, k)) $$ LANGUAGE sql STABLE; --- The offset reads "runtime"; the WindowAgg inlines into the nestloop and is --- rescanned per outer row. +-- Plan: the offset reads "runtime"; the WindowAgg inlines into the +-- nestloop and is rescanned per outer row. EXPLAIN (COSTS OFF) SELECT g.n, max(s) FROM (VALUES (1), (2), (3)) g(n), LATERAL rpr_srf_f(g.n) s GROUP BY g.n ORDER BY g.n; --- Each outer row yields its own offset (9, 8, 7), not one frozen value. +-- Result: each outer row yields its own offset (9, 8, 7), not one +-- frozen value. SELECT g.n, max(s) AS m FROM (VALUES (1), (2), (3)) g(n), LATERAL rpr_srf_f(g.n) s GROUP BY g.n ORDER BY g.n; --- A forward FIRST-family offset with a correlated PARAM_EXEC must likewise be --- re-resolved per scan (navFirstOffset / navFirstOffsetKind), not frozen at init. --- PATTERN (B A+) anchors the match start at B so A can reference FIRST(v, k) --- k rows ahead; each outer k yields its own forward offset (k=0 matches all --- ten rows, k>=1 makes the first A fail), proving per-scan re-resolution. +-- A forward FIRST-family offset must likewise be re-resolved per scan +-- (navFirstOffset / navFirstOffsetKind). PATTERN (B A+) anchors the +-- match start at B so A can reference FIRST(v, k) k rows ahead, and +-- each outer k yields its own forward offset. CREATE FUNCTION rpr_srf_first(k int) RETURNS SETOF bigint AS $$ SELECT count(*) OVER w FROM rpr_srf WINDOW w AS (ORDER BY v ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (B A+) DEFINE A AS v > FIRST(v, k)) $$ LANGUAGE sql STABLE; --- The forward offset reads "runtime" and the WindowAgg inlines into the nestloop. +-- Plan: the forward offset reads "runtime" and the WindowAgg inlines +-- into the nestloop. EXPLAIN (COSTS OFF) SELECT g.n, max(s) FROM (VALUES (0), (1), (2)) g(n), LATERAL rpr_srf_first(g.n) s GROUP BY g.n ORDER BY g.n; --- k=0 -> 10, k>=1 -> 0: distinct per outer row, so the forward offset is not --- frozen at ExecInit. +-- Result: k=0 matches all ten rows and k>=1 makes the first A fail, +-- so the forward offset is not frozen at ExecInit. SELECT g.n, max(s) AS m FROM (VALUES (0), (1), (2)) g(n), LATERAL rpr_srf_first(g.n) s GROUP BY g.n ORDER BY g.n; DROP FUNCTION rpr_srf_first(int); --- A compound navigation whose OUTER offset is a correlated PARAM_EXEC must be --- re-resolved per scan as well. An outer offset that overflows int64 flips the --- backward trim to RETAIN_ALL for that scan only; a smaller offset on the next --- rescan must go back to FIXED. k=1 -> 9, k=3 -> 7, k=overflow -> 0 (out of --- range, no match), proving both the per-scan outer-offset resolution and the --- RETAIN_ALL <-> FIXED toggle across rescans. +-- A compound navigation's OUTER offset must be re-resolved per scan +-- as well. An outer offset that overflows int64 flips the backward +-- trim to RETAIN_ALL for that scan only; a smaller offset on the next +-- rescan must go back to FIXED. CREATE FUNCTION rpr_srf_cmp(k int8) RETURNS SETOF bigint AS $$ SELECT count(*) OVER w FROM rpr_srf WINDOW w AS (ORDER BY v ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A B+) DEFINE B AS v > PREV(LAST(v, 1), k)) $$ LANGUAGE sql STABLE; +-- Result: k=1 -> 9, k=3 -> 7, overflow -> 0 (out of range, no match), +-- proving both the per-scan outer-offset resolution and the +-- RETAIN_ALL <-> FIXED toggle across rescans. SELECT g.n, max(s) AS m FROM (VALUES (1::int8), (3::int8), (9223372036854775807::int8)) g(n), LATERAL rpr_srf_cmp(g.n) s @@ -1067,13 +1122,16 @@ DROP FUNCTION rpr_srf_cmp(int8); DROP FUNCTION rpr_srf_f(int); DROP TABLE rpr_srf; --- A correlated PARAM_EXEC that appears only inside DEFINE must be registered in --- the WindowAgg's extParam so the chgParam signal reaches a caching node above. --- Here the inlined function's argument is used only in DEFINE, DISTINCT is --- planned as a HashAgg, and HashAgg rescan is gated on chgParam; without the --- param in extParam the hash table built for the first outer row would be --- re-served for the rest. Each threshold must get its own answer set --- (10 -> {0, 90}, 200 -> {0}); a stale cache would add a spurious 200|90. +-- ============================================================ +-- B13. RPR + DEFINE-only parameter caching +-- ============================================================ +-- Verify that a correlated PARAM_EXEC appearing only inside DEFINE is +-- registered in the WindowAgg's extParam, so the chgParam signal +-- reaches a caching node above. Here the inlined function's argument +-- is used only in DEFINE, DISTINCT is planned as a HashAgg, and +-- HashAgg rescan is gated on chgParam; without the param in extParam +-- the hash table built for the first outer row would be re-served for +-- the rest. CREATE TABLE rpr_hcache_thr (threshold int); INSERT INTO rpr_hcache_thr VALUES (10), (200); CREATE TABLE rpr_hcache_stock (price int); @@ -1083,11 +1141,55 @@ CREATE FUNCTION rpr_hcache_fn(th int) RETURNS SETOF bigint LANGUAGE sql STABLE A WINDOW w AS (ORDER BY price ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING AFTER MATCH SKIP PAST LAST ROW INITIAL PATTERN (a+) DEFINE a AS price > th) $$; +-- Result: each threshold gets its own answer set (10 -> {0, 90}, +-- 200 -> {0}); a stale cache would add a spurious 200|90. SELECT o.threshold, f FROM rpr_hcache_thr o, LATERAL rpr_hcache_fn(o.threshold) f ORDER BY 1, 2; DROP FUNCTION rpr_hcache_fn(int); DROP TABLE rpr_hcache_thr, rpr_hcache_stock; +-- ============================================================ +-- B14. RPR + Multiple window definitions +-- ============================================================ +-- Verify that a column referenced only by DEFINE coexists with a +-- later window's PARTITION BY / ORDER BY key that the SELECT list +-- does not carry. Both are added to the targetlist as junk entries +-- and must draw their resno from p_next_resno; otherwise the two +-- collide and apply_tlist_labeling trips an assertion during +-- planning. Here val is DEFINE-only and grp belongs to w2, with the +-- RPR window defined first so its junk entry is created first. + +-- Result: the DEFINE-only column and the later window's key coexist. +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; + +-- Result: same collision 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; + +-- Result: defining the windows in the opposite order never collided, +-- and must keep returning 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; -- 2.50.1 (Apple Git-155)