From 3cc8a4040599e20c6b0338135235f094c10afe01 Mon Sep 17 00:00:00 2001 From: jian he Date: Mon, 10 Aug 2026 12:17:13 +0900 Subject: [PATCH] Register DEFINE-only correlated parameters in the WindowAgg's extParam finalize_plan() walked a WindowAgg's startOffset and endOffset but not its defineClause, so a correlated PARAM_EXEC referenced only from DEFINE never reached extParam. UpdateChangedParamSet() intersects a param change with the node's allParam, so the change never reached the subtree, and a caching node above it kept serving the first outer row's result: an inlined function whose argument is used only in DEFINE, with DISTINCT planned as a HashAgg, returned the hash table built for the first row to every later one. While here, have set_upper_references() build a new defineClause list rather than replacing the cells of the existing one in place: create_windowagg_plan() aliases the WindowAgg's list to the WindowClause's, so the in-place form had setrefs.c writing into the parse tree. No test distinguishes the two forms -- SS_finalize_plan() and the rest of createplan.c have run by then, and the paths that plan one Query more than once copy it first -- so this is here to keep setrefs.c from writing through the alias, not to fix an observable bug. --- src/backend/optimizer/plan/setrefs.c | 31 ++++++++++--------- src/backend/optimizer/plan/subselect.c | 2 ++ src/test/regress/expected/rpr_integration.out | 27 ++++++++++++++++ src/test/regress/sql/rpr_integration.sql | 21 +++++++++++++ 4 files changed, 66 insertions(+), 15 deletions(-) diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c index 46229312d8a..c9db0a7034e 100644 --- a/src/backend/optimizer/plan/setrefs.c +++ b/src/backend/optimizer/plan/setrefs.c @@ -2611,25 +2611,26 @@ set_upper_references(PlannerInfo *root, Plan *plan, int rtoffset) */ if (IsA(plan, WindowAgg)) { + List *new_defineClause = NIL; WindowAgg *wplan = (WindowAgg *) plan; - if (wplan->defineClause != NIL) + foreach_node(TargetEntry, tle, wplan->defineClause) { - foreach(l, wplan->defineClause) - { - TargetEntry *tle = (TargetEntry *) lfirst(l); - - tle = flatCopyTargetEntry(tle); - tle->expr = (Expr *) - fix_upper_expr(root, - (Node *) tle->expr, - subplan_itlist, - OUTER_VAR, - rtoffset, - NUM_EXEC_QUAL(plan)); - lfirst(l) = tle; - } + TargetEntry *newtle; + + newtle = flatCopyTargetEntry(tle); + newtle->expr = (Expr *) + fix_upper_expr(root, + (Node *) tle->expr, + subplan_itlist, + OUTER_VAR, + rtoffset, + NUM_EXEC_QUAL(plan)); + + new_defineClause = lappend(new_defineClause, newtle); } + + wplan->defineClause = new_defineClause; } pfree(subplan_itlist); diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c index 6aa8971c95d..20422a48a8c 100644 --- a/src/backend/optimizer/plan/subselect.c +++ b/src/backend/optimizer/plan/subselect.c @@ -3050,6 +3050,8 @@ finalize_plan(PlannerInfo *root, Plan *plan, &context); finalize_primnode(((WindowAgg *) plan)->endOffset, &context); + finalize_primnode((Node *) ((WindowAgg *) plan)->defineClause, + &context); break; case T_Gather: diff --git a/src/test/regress/expected/rpr_integration.out b/src/test/regress/expected/rpr_integration.out index 8acedf63464..5762f6411af 100644 --- a/src/test/regress/expected/rpr_integration.out +++ b/src/test/regress/expected/rpr_integration.out @@ -1647,6 +1647,33 @@ SELECT cnt FROM ( (16 rows) DROP TABLE rpr_over1, rpr_over2; +-- A correlated PARAM_EXEC used only inside DEFINE must reach the WindowAgg's +-- extParam. Otherwise chgParam never gets to the HashAgg that DISTINCT plans +-- above it, and its hash table for the first outer row is re-served. +-- The SRF must be inlined as a lateral subquery for its argument to become a +-- PARAM_EXEC; a FunctionScan plan would pass either way. +CREATE TABLE rpr_hcache_thr (threshold int); +INSERT INTO rpr_hcache_thr VALUES (10), (200); +CREATE TABLE rpr_hcache_stock (price int); +INSERT INTO rpr_hcache_stock SELECT g FROM generate_series(1, 100) g; +CREATE FUNCTION rpr_hcache_fn(th int) RETURNS SETOF bigint LANGUAGE sql STABLE AS $$ + SELECT DISTINCT count(*) OVER w FROM rpr_hcache_stock + 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 +-----------+---- + 10 | 0 + 10 | 90 + 200 | 0 +(3 rows) + +DROP FUNCTION rpr_hcache_fn(int); +DROP TABLE rpr_hcache_thr, rpr_hcache_stock; -- 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 diff --git a/src/test/regress/sql/rpr_integration.sql b/src/test/regress/sql/rpr_integration.sql index a79b518bf9f..8e9049ffee2 100644 --- a/src/test/regress/sql/rpr_integration.sql +++ b/src/test/regress/sql/rpr_integration.sql @@ -1039,6 +1039,27 @@ SELECT cnt FROM ( ) s; DROP TABLE rpr_over1, rpr_over2; +-- A correlated PARAM_EXEC used only inside DEFINE must reach the WindowAgg's +-- extParam. Otherwise chgParam never gets to the HashAgg that DISTINCT plans +-- above it, and its hash table for the first outer row is re-served. +-- The SRF must be inlined as a lateral subquery for its argument to become a +-- PARAM_EXEC; a FunctionScan plan would pass either way. +CREATE TABLE rpr_hcache_thr (threshold int); +INSERT INTO rpr_hcache_thr VALUES (10), (200); +CREATE TABLE rpr_hcache_stock (price int); +INSERT INTO rpr_hcache_stock SELECT g FROM generate_series(1, 100) g; +CREATE FUNCTION rpr_hcache_fn(th int) RETURNS SETOF bigint LANGUAGE sql STABLE AS $$ + SELECT DISTINCT count(*) OVER w FROM rpr_hcache_stock + 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; + -- 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