From 7e6c3c727013da423696b57c6d9ae94b6807b598 Mon Sep 17 00:00:00 2001 From: jian he Date: Tue, 28 Jul 2026 15:59:56 +0900 Subject: [PATCH] Retain DEFINE columns only for a window some window function uses remove_unused_subquery_outputs() keeps a subquery output column alive when a DEFINE clause reads it, so that pattern matching still sees a column the upper query does not select. It did so for every window clause carrying a DEFINE, including one that no window function references. select_active_windows() drops such a window when the subquery is planned, so retaining its columns holds on to values nothing will read. When the subquery uses row pattern recognition, collect its window functions and retain DEFINE columns only for a window clause some window function actually references. The test pins it: a subquery declares an RPR window that nothing reads, and the column only that window's DEFINE mentions is replaced with a null Const. Two things are left in an unfinished state. The test is in the wrong file. rpr_integration.sql exists as the single checkpoint for planner/RPR interactions and already carries a section on this very function -- "A5. Unused window removal prevention". This test went into rpr_base.sql instead, next to the arithmetic examples of the DEFINE clause tests, which splits one topic across two files and leaves A5 describing only half of it. It belongs in A5, and A5's title no longer matches what the section demonstrates. The function still holds two answers to the same question. Further down, a separate guard refuses to replace an unused window function with NULL whenever its window clause has a DEFINE, on the grounds that the pattern match has to run regardless. That guard and this change disagree about whether a window nothing reads may disappear. Only one of them can be right, and the reasoning behind either is not recorded where the other is. Beyond that, retention is still all-or-nothing per window clause: a window is either kept whole or dropped whole, with no analysis of whether the match result has any consumer. A precise version remains open work. --- src/backend/optimizer/path/allpaths.c | 35 +++++++++++++++++++------- src/test/regress/expected/rpr_base.out | 26 +++++++++++++++++++ src/test/regress/sql/rpr_base.sql | 12 +++++++++ 3 files changed, 64 insertions(+), 9 deletions(-) diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index 06c90fcb98c..fc184c78816 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -4838,6 +4838,7 @@ remove_unused_subquery_outputs(Query *subquery, RelOptInfo *rel, { Bitmapset *attrs_used; ListCell *lc; + WindowFuncLists *wflists = NULL; /* * Just point directly to extra_used_attrs. No need to bms_copy as none of @@ -4886,6 +4887,24 @@ remove_unused_subquery_outputs(Query *subquery, RelOptInfo *rel, if (bms_is_member(0 - FirstLowInvalidHeapAttributeNumber, attrs_used)) return; + /* + * If the subquery uses row pattern recognition, collect its window + * functions so we can tell which window clauses are active (i.e. + * referenced by a window function). An RPR window that no window + * function references will be dropped by select_active_windows when the + * subquery is planned, so below we should not let such a dead window's + * DEFINE clause retain otherwise-unused output columns. + */ + foreach_node(WindowClause, wc, subquery->windowClause) + { + if (wc->rpPattern != NULL) + { + wflists = find_window_functions((Node *) subquery->targetList, + list_length(subquery->windowClause)); + break; + } + } + /* * Run through the tlist and zap entries we don't need. It's okay to * modify the tlist items in-place because set_subquery_pathlist made a @@ -4943,7 +4962,9 @@ remove_unused_subquery_outputs(Query *subquery, RelOptInfo *rel, foreach_node(WindowClause, wc, subquery->windowClause) { - if (wc->defineClause != NIL) + if (wc->defineClause != NIL && + wflists != NULL && + wflists->windowFuncs[wc->winref] != NIL) { /* * flags == 0 is safe: DEFINE rejects aggregates, window @@ -4954,15 +4975,11 @@ remove_unused_subquery_outputs(Query *subquery, RelOptInfo *rel, foreach_node(Var, dvar, vars) { - /* - * Match varno as well as varattno: a Var pulled from - * a DEFINE clause can share an attribute number with - * an unrelated output column of a different relation, - * which would otherwise be over-retained. Checking - * varlevelsup is just paranoia, since outer - * references in DEFINE are rejected during parse - * analysis. + * Match varno too: varattno alone can collide with an + * unrelated column of another relation. varlevelsup + * is paranoia, since DEFINE rejects outer references + * at parse time. */ if (dvar->varno == var->varno && dvar->varattno == var->varattno && diff --git a/src/test/regress/expected/rpr_base.out b/src/test/regress/expected/rpr_base.out index 1e3cb8dca3f..5483f43558b 100644 --- a/src/test/regress/expected/rpr_base.out +++ b/src/test/regress/expected/rpr_base.out @@ -185,6 +185,32 @@ ORDER BY dt; 01-05-2024 | 158 | 1100 | 0 (5 rows) +-- w2 is declared but no window function references it, so its DEFINE must +-- not keep price alive: the subquery output for price becomes a null Const +EXPLAIN (VERBOSE, COSTS OFF, BUFFERS OFF) +SELECT cnt FROM ( + SELECT count(*) OVER w1 AS cnt, price + FROM stock_price + WINDOW w1 AS (ORDER BY dt), + w2 AS (ORDER BY dt + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (a b) + DEFINE b AS price > PREV(price)) +) t; + QUERY PLAN +----------------------------------------------------------------- + Subquery Scan on t + Output: t.cnt + -> WindowAgg + Output: count(*) OVER w1, NULL::numeric, stock_price.dt + Window: w1 AS (ORDER BY stock_price.dt) + -> Sort + Output: stock_price.dt + Sort Key: stock_price.dt + -> Seq Scan on public.stock_price + Output: stock_price.dt +(10 rows) + DROP TABLE stock_price; -- Pattern variables with no DEFINE entry CREATE TABLE rpr_auto (id INT, val INT); diff --git a/src/test/regress/sql/rpr_base.sql b/src/test/regress/sql/rpr_base.sql index 28c8d96f282..cb84d828f38 100644 --- a/src/test/regress/sql/rpr_base.sql +++ b/src/test/regress/sql/rpr_base.sql @@ -158,6 +158,18 @@ WINDOW w AS ( DEFINE CALC AS (price + volume / 100) > 160 ) ORDER BY dt; +-- w2 is declared but no window function references it, so its DEFINE must +-- not keep price alive: the subquery output for price becomes a null Const +EXPLAIN (VERBOSE, COSTS OFF, BUFFERS OFF) +SELECT cnt FROM ( + SELECT count(*) OVER w1 AS cnt, price + FROM stock_price + WINDOW w1 AS (ORDER BY dt), + w2 AS (ORDER BY dt + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (a b) + DEFINE b AS price > PREV(price)) +) t; DROP TABLE stock_price;