From e5c8e73ac1b05197dd73c3da9f041a47402b67b1 Mon Sep 17 00:00:00 2001 From: Henson Choi Date: Fri, 18 Sep 2026 12:04:44 +0900 Subject: [PATCH] Add what a DEFINE clause reads to the window input target A DEFINE expression is evaluated by the WindowAgg but lives outside the target list, so nothing in the ordinary target list machinery has a reason to ask for what it reads. Everything it reads still has to reach the WindowAgg's input tuple, in the shape the DEFINE copy will be executed with. Two things were missing. The first is shape. When a composite arrives through a subquery, pullup substitutes the subquery's ROW(...) into both the window's own ORDER BY or PARTITION BY copy and the DEFINE copy, and eval_const_expressions() then splits the DEFINE copy's IS [NOT] NULL test into one test per field. The sortgroupref on the other copy keeps make_window_input_target() from flattening it, so the fields that split leaves behind reach the WindowAgg input under no name at all, and setrefs.c fails with "variable not found in subplan target list". Fix by having make_window_input_target() add whatever a DEFINE clause reads that the input target does not offer yet. It runs after the clause has been preprocessed, so what it reads is the shape the clause will be executed with, whatever a rewrite has made of it along the way. The walk stops at an expression the target already computes whole, since setrefs.c resolves the DEFINE copy of it against that column. Stopping is what the GROUP BY cases need rather than an optimization: the Vars underneath a grouping expression are not available on their own, so descending into one would ask the grouping step for a column it cannot produce. The second is reachability. An input target can only ask for a column that reached the top of the join tree, and an intermediate join emits only what something above has declared a need for. Declare it in build_base_rel_tlists(), beside the HAVING clause, which is the same problem: a boolean expression an upper planner node evaluates, living outside the target list. havingQual is handled with exactly these two sites and is never put in the target list at all, so follow it. That leaves the parser's Var planting with nothing to do, and it goes. It was there because remove_unused_subquery_outputs() and remove_useless_outer_joins() run from query_planner() and drop what the query no longer reads; marking the columns needed in build_base_rel_tlists() answers both, and it runs before either of them. Removing the planting takes the group clause back out of transformWindowDefinitions(), and stops parse analysis from rewriting the user's target list at all. A DEFINE clause reading a grouping expression keeps working, now through the stop rule above rather than through a planting rule of its own. The measurable effect is in rpr_integration: a column only a DEFINE clause reads no longer rides on the WindowAgg's own output, where nothing reads it, and appears on its input alone. Two cases had no coverage and get it here: a DEFINE clause reading a compound GROUP BY expression, which is the one place the stop rule is load-bearing, and a window function nested in a subquery output the upper query drops, where the pass that settles which windows are still live misses it and a dead window's column is retained. The second is an over-retention, not a wrong answer, and making that pass exact is left to its own commit. ExecInitWindowAgg() now checks the varId-to-list-position invariant that buildRPRPattern() establishes and it consumes, by comparing the name the pattern holds for each position against the DEFINE entry's own. A reorder would evaluate one variable's search condition for another, which is a wrong answer with nothing to show for it. --- src/backend/executor/README.rpr | 44 ++- src/backend/executor/nodeWindowAgg.c | 12 + src/backend/optimizer/path/allpaths.c | 5 +- src/backend/optimizer/plan/initsplan.c | 29 ++ src/backend/optimizer/plan/planner.c | 62 +++++ src/backend/parser/analyze.c | 3 +- src/backend/parser/parse_clause.c | 5 +- src/backend/parser/parse_rpr.c | 131 +-------- src/include/parser/parse_clause.h | 3 +- src/include/parser/parse_rpr.h | 3 +- src/test/regress/expected/rpr_base.out | 255 ++++++++++++++++++ src/test/regress/expected/rpr_integration.out | 92 +++++-- src/test/regress/sql/rpr_base.sql | 160 +++++++++++ src/test/regress/sql/rpr_integration.sql | 56 +++- src/tools/pgindent/typedefs.list | 1 - 15 files changed, 671 insertions(+), 190 deletions(-) diff --git a/src/backend/executor/README.rpr b/src/backend/executor/README.rpr index 202b5df3f76..6b6f24e753b 100644 --- a/src/backend/executor/README.rpr +++ b/src/backend/executor/README.rpr @@ -274,27 +274,17 @@ use. After that it processes each DEFINE variable as follows: Boolean (coerce_to_boolean) right away, so that the steps below see the final expression form (3) Wraps in a TargetEntry with the variable name set in resname - (4) Ensures each bare Var the expression reads is present in the - query targetlist, planting any that is missing as a resjunk - entry (define_plant_walker), so that setrefs.c can resolve it - against the WindowAgg's input. The walk stops at a subexpression - GROUP BY computes and plants nothing for it. - - Two properties of a planted entry are load-bearing. Each is a bare - Var, never a whole subexpression the target list already carries: - the DEFINE copy and the target list copy are preprocessed - independently, so eval_const_expressions() can dissolve the DEFINE - copy of a subexpression and leave a bare Var behind with nothing in - the input to resolve it against, whereas a bare Var has no shape to - lose. And each is marked resjunk, which is one of the properties - remove_unused_subquery_outputs() keeps a column alive for (XIII-2); - make_window_input_target() adds nothing of its own for a DEFINE - clause. + +Parse analysis puts nothing in the query targetlist for a DEFINE clause. +Getting what it reads to the WindowAgg is the planner's job, and it is done +the way havingQual's is: build_base_rel_tlists() marks the columns needed so +they reach the top of the join tree, and make_window_input_target() asks for +them again in the node's own input target (XIII-2). After all variables are processed: - (5) Validates navigation nesting and offsets (define_walker) + (4) Validates navigation nesting and offsets (define_walker) -Step (5) is where the shape every later phase assumes gets established. +Step (4) is where the shape every later phase assumes gets established. define_walker() requires the argument of each navigation to contain at least one column reference, and requires each offset to be a run-time constant: an offset may contain neither a column reference nor another navigation. The @@ -2221,10 +2211,10 @@ XIII-2. Keeping a DEFINE-only Column Alive A column that nothing reads except a DEFINE expression still has to survive subquery pruning. remove_unused_subquery_outputs() pulls the Vars out of every still-live DEFINE clause and refuses to replace a targetlist entry -that one of them names, matching on varno, varattno and varlevelsup. That -guard stands alone: nothing downstream re-adds a DEFINE column to the -WindowAgg's input target, so a column dropped there is one the pattern match -can no longer read. +that one of them names, matching on varno, varattno and varlevelsup. +make_window_input_target() later asks for whatever the DEFINE clause reads, +but it can only ask for a column that still exists, so a column dropped here +is one the pattern match can no longer read. The order within that function matters. The window function targetlist entries are settled first and the surviving set of active windows is read @@ -2245,10 +2235,12 @@ matches the DEFINE copy of an expression against the targetlist copy and insists the two agree; a mismatch is not a wrong answer at runtime but a "variable not found in subplan target list" failure at plan time. -This is also why the parser's Var planting (III-3) stops at a subexpression -GROUP BY computes. The grouping step already produces a Var for it, and -planting underneath would offer the columns below it to the grouping logic, -which would then reject them as ungrouped. +This is also why make_window_input_target() stops at a subexpression the +input target already computes whole. The grouping step produces only that +expression, so asking for the Vars underneath it would ask the grouping step +for columns it cannot produce. This is the one rule the havingQual handling +it is modelled on does not need: make_group_input_target() builds the input +of the grouping step, while the window input target sits above it. XIII-4. Navigation Arguments and Subquery Pull-up diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c index e9026f17c00..cc4ec220545 100644 --- a/src/backend/executor/nodeWindowAgg.c +++ b/src/backend/executor/nodeWindowAgg.c @@ -3097,6 +3097,18 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags) { ExprState *exprstate; + /* + * That index is established in buildRPRPattern() and consumed + * here, with nothing in between checking it. Every step that + * touches the list preserves its order today, but a reorder would + * evaluate one variable's search condition for another and give a + * wrong answer with nothing to show for it, so check the name the + * pattern holds for this position against the entry's own. + */ + Assert(foreach_current_index(te) < node->rpPattern->numVars); + Assert(strcmp(node->rpPattern->varNames[foreach_current_index(te)], + te->resname) == 0); + exprstate = ExecInitExpr(te->expr, (PlanState *) winstate); winstate->defineClauseExprs = diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index 9b47be9bf59..f2a32606f6d 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -5070,8 +5070,9 @@ remove_unused_subquery_outputs(Query *subquery, RelOptInfo *rel, * column in its DEFINE clause, don't remove it. The DEFINE * expression needs these columns in the tuplestore slot for pattern * matching evaluation, even if the outer query doesn't reference - * them. This is the only protection: nothing downstream re-adds a - * DEFINE column to the WindowAgg's input target. + * them. make_window_input_target() later asks for whatever the + * DEFINE clause reads, but it can only ask for a column that still + * exists. */ if (IsA(texpr, Var)) { diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c index fb6f81453ea..fc4baf79863 100644 --- a/src/backend/optimizer/plan/initsplan.c +++ b/src/backend/optimizer/plan/initsplan.c @@ -281,6 +281,35 @@ build_base_rel_tlists(PlannerInfo *root, List *final_tlist) list_free(having_vars); } } + + /* + * A row pattern DEFINE clause is not in the target list, so nothing above + * has asked for the columns it reads. The WindowAgg evaluates it all the + * same, and setrefs.c has to resolve it against the window's input, so + * mark those columns needed here and let them propagate up through the + * join steps the way the target list's own columns do. + */ + foreach_node(WindowClause, wc, root->parse->windowClause) + { + List *define_vars; + + if (wc->defineClause == NIL) + continue; + + /* + * PVC_INCLUDE_PLACEHOLDERS is the only flag needed: DEFINE rejects + * aggregates, window functions and subqueries at parse time. + */ + define_vars = pull_var_clause((Node *) wc->defineClause, + PVC_INCLUDE_PLACEHOLDERS); + + if (define_vars != NIL) + { + add_vars_to_targetlist(root, define_vars, + bms_make_singleton(0)); + list_free(define_vars); + } + } } /* diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 825a4ef1e3e..14ce1b85836 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -236,6 +236,7 @@ static void optimize_window_clauses(PlannerInfo *root, WindowFuncLists *wflists); static List *select_active_windows(PlannerInfo *root, WindowFuncLists *wflists); static void name_active_windows(List *activeWindows); +static bool add_define_inputs_walker(Node *node, PathTarget *input_target); static PathTarget *make_window_input_target(PlannerInfo *root, PathTarget *final_target, List *activeWindows); @@ -6421,6 +6422,50 @@ common_prefix_cmp(const void *a, const void *b) return 0; } +/* + * add_define_inputs_walker + * Add to a WindowAgg's input target whatever a DEFINE clause reads that + * the target does not offer yet. + * + * This is the window's counterpart of the HAVING handling a few functions up: + * build_base_rel_tlists() marks the columns needed so they reach the top of + * the join tree, and the node's own input target has to ask for them again + * because the upper planner projects through explicit targets rather than + * propagating attr_needed. make_group_input_target() does the same for + * havingQual. + * + * The walk stops at any expression the target already computes whole, since + * setrefs.c resolves the DEFINE copy of it against that column. Stopping + * matters rather than merely saving work: under GROUP BY the Vars underneath + * a grouping expression are not available on their own, so descending into + * one would ask the grouping step for a column it cannot produce. This is + * the one rule HAVING does not need, the Agg's input target sitting below + * the grouping step rather than above it. + */ +static bool +add_define_inputs_walker(Node *node, PathTarget *input_target) +{ + ListCell *lc; + + if (node == NULL) + return false; + + foreach(lc, input_target->exprs) + { + if (equal(node, lfirst(lc))) + return false; + } + + if (IsA(node, Var) || IsA(node, PlaceHolderVar)) + { + add_new_column_to_pathtarget(input_target, (Expr *) node); + return false; + } + + return expression_tree_walker(node, add_define_inputs_walker, + input_target); +} + /* * make_window_input_target * Generate appropriate PathTarget for initial input to WindowAgg nodes. @@ -6554,6 +6599,23 @@ make_window_input_target(PlannerInfo *root, PVC_INCLUDE_PLACEHOLDERS); add_new_columns_to_pathtarget(input_target, flattenable_vars); + /* + * A row pattern DEFINE clause is evaluated by the WindowAgg itself, so + * everything it reads has to reach this target too. Nothing above has a + * reason to put it here: DEFINE is not part of the query's final target + * list, and the window's own PARTITION BY/ORDER BY entries are added + * whole, which does not make the Vars inside them available separately. + * Add what is missing now, once the clause has the shape it will be + * executed with. + */ + foreach(lc, activeWindows) + { + WindowClause *wc = lfirst_node(WindowClause, lc); + + if (wc->defineClause != NIL) + add_define_inputs_walker((Node *) wc->defineClause, input_target); + } + /* clean up cruft */ list_free(flattenable_vars); list_free(flattenable_cols); diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index cdde6375ad4..14202e5cae6 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -1864,8 +1864,7 @@ transformSelectStmt(ParseState *pstate, SelectStmt *stmt, /* transform window clauses after we have seen all window functions */ qry->windowClause = transformWindowDefinitions(pstate, pstate->p_windowdefs, - &qry->targetList, - qry->groupClause); + &qry->targetList); /* resolve any still-unresolved output columns as being type text */ if (pstate->p_resolve_unknowns) diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c index 4aff235eede..16217f0ac9f 100644 --- a/src/backend/parser/parse_clause.c +++ b/src/backend/parser/parse_clause.c @@ -2766,8 +2766,7 @@ transformSortClause(ParseState *pstate, List * transformWindowDefinitions(ParseState *pstate, List *windowdefs, - List **targetlist, - List *groupClause) + List **targetlist) { List *result = NIL; Index winref = 0; @@ -2963,7 +2962,7 @@ transformWindowDefinitions(ParseState *pstate, windef->endOffset); /* Process Row Pattern Recognition related clauses */ - transformRPR(pstate, wc, windef, targetlist, groupClause); + transformRPR(pstate, wc, windef); wc->winref = winref; diff --git a/src/backend/parser/parse_rpr.c b/src/backend/parser/parse_rpr.c index 62746fc33bd..0a5b4151348 100644 --- a/src/backend/parser/parse_rpr.c +++ b/src/backend/parser/parse_rpr.c @@ -50,20 +50,10 @@ typedef struct RPRNavKind inner_kind; /* kind of first nested nav in current arg */ } DefineWalkCtx; -/* Target list planting walker context -- see define_plant_walker. */ -typedef struct -{ - ParseState *pstate; - List **targetlist; - List *groupExprs; /* expressions GROUP BY computes */ -} DefinePlantCtx; - /* Forward declarations */ static void validateRPRPatternVarCount(ParseState *pstate, RPRPatternNode *node, List **varNames); -static List *transformDefineClause(ParseState *pstate, WindowDef *windef, - List **targetlist, List *groupClause); -static bool define_plant_walker(Node *node, void *context); +static List *transformDefineClause(ParseState *pstate, WindowDef *windef); static bool define_walker(Node *node, void *context); static bool rpr_frame_is_supported(int frameOptions); @@ -81,8 +71,7 @@ static bool rpr_frame_is_supported(int frameOptions); * Returns early if windef has no rpCommonSyntax (non-RPR window). */ void -transformRPR(ParseState *pstate, WindowClause *wc, WindowDef *windef, - List **targetlist, List *groupClause) +transformRPR(ParseState *pstate, WindowClause *wc, WindowDef *windef) { /* Nothing to do unless the window carries a row pattern */ if (windef->rpCommonSyntax == NULL) @@ -128,8 +117,7 @@ transformRPR(ParseState *pstate, WindowClause *wc, WindowDef *windef, wc->rpSkipTo = windef->rpCommonSyntax->rpSkipTo; /* Transform DEFINE clause into list of TargetEntry's */ - wc->defineClause = transformDefineClause(pstate, windef, targetlist, - groupClause); + wc->defineClause = transformDefineClause(pstate, windef); /* Store PATTERN parse tree for deparsing */ wc->rpPattern = windef->rpCommonSyntax->rpPattern; @@ -248,24 +236,10 @@ validateRPRPatternVarCount(ParseState *pstate, RPRPatternNode *node, * parse_expr.c via the p_rpr_pattern_vars check. */ static List * -transformDefineClause(ParseState *pstate, WindowDef *windef, - List **targetlist, List *groupClause) +transformDefineClause(ParseState *pstate, WindowDef *windef) { List *defineClause = NIL; List *patternVarNames = NIL; - List *groupExprs = NIL; - - /* - * Collect what GROUP BY computes, so that the planting below can stop at - * one. Taken before any planting, since the entries planted are not - * grouping columns and carry no sortgroupref. - */ - foreach_node(SortGroupClause, sgc, groupClause) - { - TargetEntry *tle = get_sortgroupclause_tle(sgc, *targetlist); - - groupExprs = lappend(groupExprs, tle->expr); - } /* * The grammar builds an RPCommonSyntax only for a window specification @@ -331,16 +305,12 @@ transformDefineClause(ParseState *pstate, WindowDef *windef, { TargetEntry *teDefine; Node *expr; - DefinePlantCtx ctx; /* - * Transform the DEFINE expression and coerce it to boolean. We must - * NOT add the whole expression to the query targetlist, because it - * may contain RPRNavExpr nodes (PREV/NEXT/FIRST/LAST) that can only - * be evaluated inside the owning WindowAgg. Coercing here, before - * define_plant_walker() runs below, keeps that walk on the final - * expression form and surfaces a type mismatch before the targetlist - * is touched. + * Transform the DEFINE expression and coerce it to boolean. The + * result belongs in wc->defineClause, never in the query targetlist + * as a whole: it may contain RPRNavExpr nodes (PREV/NEXT/FIRST/LAST) + * that only the owning WindowAgg can evaluate. */ expr = transformExpr(pstate, restarget->val, EXPR_KIND_RPR_DEFINE); @@ -354,46 +324,6 @@ transformDefineClause(ParseState *pstate, WindowDef *windef, /* build transformed DEFINE clause (list of TargetEntry) */ defineClause = lappend(defineClause, teDefine); - - /* - * A DEFINE expression lives in wc->defineClause, not in the - * targetlist, so make_window_input_target() never sees it when - * deciding what the WindowAgg's input must carry. Yet setrefs.c must - * resolve every Var in the DEFINE clause to a column of that input, - * and fails on any column nothing else put there. Hence what a - * DEFINE expression reads must be planted in the targetlist as - * resjunk entries. - * - * Plant bare Vars, not subexpressions. A subexpression the target - * list already carries looks like a shortcut, but the two copies are - * preprocessed independently: given DEFINE A AS ROW(v, 1) IS NOT - * NULL, eval_const_expressions() breaks the DEFINE copy into per - * field tests and leaves a bare v behind, with nothing in the input - * to resolve it against. A bare Var has no such shape to lose. - * - * Whatever is planted has to reach the WindowAgg's input on its own: - * make_window_input_target() derives that input from final_target and - * adds nothing of its own for DEFINE, and - * remove_unused_subquery_outputs() keeps a column alive only for an - * entry that is resjunk or bears a sortgroupref, or that its own - * DEFINE guard matches. A resjunk entry qualifies. - * - * The walk stops at a subexpression GROUP BY computes and plants - * nothing for it. parseCheckAggregates() replaces such a - * subexpression with the grouping step's Var on both sides -- here - * and in the target list entry holding the same expression -- so the - * two copies still meet, and that entry bears a sortgroupref, which - * the paragraph above says is enough. Planting the columns - * underneath it instead would offer them to the grouping logic on - * their own, which does not make them available that way, and reports - * them as ungrouped. The stop reads groupClause rather than a - * sortgroupref, or the window's own ORDER BY would trip it in a query - * that does no grouping at all. - */ - ctx.pstate = pstate; - ctx.targetlist = targetlist; - ctx.groupExprs = groupExprs; - (void) define_plant_walker(expr, &ctx); } pstate->p_rpr_pattern_vars = NIL; @@ -416,51 +346,6 @@ transformDefineClause(ParseState *pstate, WindowDef *windef, return defineClause; } -/* - * define_plant_walker - * Plant in the target list what a DEFINE expression reads. - * - * Vars are planted one at a time as resjunk entries, except under a - * subexpression GROUP BY computes, where the walk stops and plants nothing -- - * see the planting comment in transformDefineClause() for why. - */ -static bool -define_plant_walker(Node *node, void *context) -{ - DefinePlantCtx *ctx = (DefinePlantCtx *) context; - - if (node == NULL) - return false; - - /* A subexpression GROUP BY computes needs nothing planted for it. */ - foreach_ptr(Node, gexpr, ctx->groupExprs) - { - if (equal(node, gexpr)) - return false; - } - - if (IsA(node, Var)) - { - Var *var = (Var *) node; - - foreach_node(TargetEntry, tle, *ctx->targetlist) - { - if (equal(tle->expr, var)) - return false; - } - - *ctx->targetlist = - lappend(*ctx->targetlist, - makeTargetEntry((Expr *) copyObject(var), - (AttrNumber) ctx->pstate->p_next_resno++, - NULL, - true)); - return false; - } - - return expression_tree_walker(node, define_plant_walker, ctx); -} - /* * define_walker * Single-pass DEFINE clause validator. At each node, enforces: diff --git a/src/include/parser/parse_clause.h b/src/include/parser/parse_clause.h index d8efc3a79ce..ca815a9d1bb 100644 --- a/src/include/parser/parse_clause.h +++ b/src/include/parser/parse_clause.h @@ -35,8 +35,7 @@ extern List *transformSortClause(ParseState *pstate, List *orderlist, extern List *transformWindowDefinitions(ParseState *pstate, List *windowdefs, - List **targetlist, - List *groupClause); + List **targetlist); extern List *transformDistinctClause(ParseState *pstate, List **targetlist, List *sortClause, bool is_agg); diff --git a/src/include/parser/parse_rpr.h b/src/include/parser/parse_rpr.h index 958bc229956..ff5037e585e 100644 --- a/src/include/parser/parse_rpr.h +++ b/src/include/parser/parse_rpr.h @@ -17,7 +17,6 @@ #include "parser/parse_node.h" extern void transformRPR(ParseState *pstate, WindowClause *wc, - WindowDef *windef, List **targetlist, - List *groupClause); + WindowDef *windef); #endif /* PARSE_RPR_H */ diff --git a/src/test/regress/expected/rpr_base.out b/src/test/regress/expected/rpr_base.out index 539a4fb4bc7..503c9fdf0f9 100644 --- a/src/test/regress/expected/rpr_base.out +++ b/src/test/regress/expected/rpr_base.out @@ -5184,6 +5184,95 @@ WINDOW w AS ( DROP TABLE rpr_composite; DROP TYPE rpr_item; +-- A composite value that reaches DEFINE by way of a subquery Var only takes +-- its ROW(...) shape after pullup, and the ORDER BY copy's sortgroupref +-- keeps it from being flattened. make_window_input_target() adds the fields +-- the split leaves behind. +CREATE TABLE rpr_ordrow (a int, b int); +INSERT INTO rpr_ordrow SELECT g, g % 4 FROM generate_series(1, 10) g; +SELECT count(*) OVER w AS c +FROM (SELECT ROW(a, b) AS x FROM rpr_ordrow) s +WINDOW w AS (ORDER BY x + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL PATTERN (P Q+) DEFINE P AS TRUE, Q AS x IS NOT NULL); + c +---- + 10 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 +(10 rows) + +-- Control: without ORDER BY, x is flattened normally and this succeeds too. +SELECT count(*) OVER w AS c +FROM (SELECT ROW(a, b) AS x FROM rpr_ordrow) s +WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL PATTERN (P Q+) DEFINE P AS TRUE, Q AS x IS NOT NULL); + c +---- + 10 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 +(10 rows) + +DROP TABLE rpr_ordrow; +-- The same split by way of a pulled-up composite target, both as a plain +-- subquery and as a view. +CREATE TABLE rpr_partrow (a int, b int); +INSERT INTO rpr_partrow VALUES (1, 1), (2, 2), (3, 3); +SELECT count(*) OVER w +FROM (SELECT b, row(a, 1) AS k FROM rpr_partrow) s +WINDOW w AS (PARTITION BY k ORDER BY b + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (p q+) DEFINE q AS k IS NOT NULL); + count +------- + 0 + 0 + 0 +(3 rows) + +CREATE TYPE rpr_partrow_t AS (x int, y int); +CREATE VIEW rpr_partrow_v AS SELECT b, row(a, 1)::rpr_partrow_t AS k FROM rpr_partrow; +SELECT count(*) OVER w FROM rpr_partrow_v +WINDOW w AS (PARTITION BY k ORDER BY b + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (p q+) DEFINE q AS k IS NOT NULL); + count +------- + 0 + 0 + 0 +(3 rows) + +-- Control: PATTERN/DEFINE aside, the same window clause runs fine. +SELECT count(*) OVER w +FROM (SELECT b, row(a, 1) AS k FROM rpr_partrow) s +WINDOW w AS (PARTITION BY k ORDER BY b + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING); + count +------- + 1 + 1 + 1 +(3 rows) + +DROP VIEW rpr_partrow_v; +DROP TYPE rpr_partrow_t; +DROP TABLE rpr_partrow; -- ERROR: undefined column in DEFINE SELECT COUNT(*) OVER w FROM rpr_err @@ -8479,6 +8568,115 @@ ORDER BY k; (3 rows) DROP TABLE rpr_join5, rpr_join6; +-- A DEFINE clause reading a USING column whose two sides differ in typmod. +-- The merged column stays a join alias Var, pullup leaves its joinaliasvars +-- entry a non-trivial expression, and the outer join's nullingrels wrap that +-- in a PlaceHolderVar. The target list copy and the DEFINE copy are wrapped +-- by separate calls, so their phids differ and equal() does not match them -- +-- the window input has to carry the DEFINE clause's own PlaceHolderVar. +CREATE TABLE rpr_phv_src (n int); +CREATE TABLE rpr_phv_dim (c varchar(10), tdate date); +CREATE TABLE rpr_phv_out (k varchar); +INSERT INTO rpr_phv_src VALUES (2), (4); +INSERT INTO rpr_phv_dim VALUES ('zz', '2024-01-01'), ('zzzz', '2024-01-02'); +INSERT INTO rpr_phv_out VALUES ('zz'), ('zzzz'); +SELECT j.c, j.tdate, count(*) OVER w AS cnt +FROM rpr_phv_out o1 + LEFT JOIN ( (SELECT n, repeat('z', n)::varchar(5) AS c FROM rpr_phv_src) s + JOIN rpr_phv_dim USING (c) ) j + ON o1.k = j.c +WINDOW w AS (ORDER BY j.tdate + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL PATTERN (p q+) + DEFINE p AS TRUE, q AS c > ''); + c | tdate | cnt +------+------------+----- + zz | 01-01-2024 | 2 + zzzz | 01-02-2024 | 0 +(2 rows) + +-- The same with one more join level above it. Reaching the window input is +-- not enough on its own: an intermediate join emits only what something above +-- has declared a need for, so what a DEFINE clause reads is marked needed at +-- relation 0 the way the target list's own columns are. +SELECT j.c, j.tdate, count(*) OVER w AS cnt +FROM rpr_phv_out o1 + LEFT JOIN rpr_phv_out o2 ON o1.k = o2.k + LEFT JOIN ( (SELECT n, repeat('z', n)::varchar(5) AS c FROM rpr_phv_src) s + JOIN rpr_phv_dim USING (c) ) j + ON o2.k = j.c +WINDOW w AS (ORDER BY j.tdate + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL PATTERN (p q+) + DEFINE p AS TRUE, q AS c > ''); + c | tdate | cnt +------+------------+----- + zz | 01-01-2024 | 2 + zzzz | 01-02-2024 | 0 +(2 rows) + +-- Control: with both sides of USING at the same typmod the merged column is a +-- plain Var of one side, no PlaceHolderVar is built, and neither shape above +-- needs any of this. +SELECT j.c, j.tdate, count(*) OVER w AS cnt +FROM rpr_phv_out o1 + LEFT JOIN rpr_phv_out o2 ON o1.k = o2.k + LEFT JOIN ( (SELECT n, repeat('z', n)::varchar(10) AS c + FROM rpr_phv_src) s + JOIN rpr_phv_dim USING (c) ) j + ON o2.k = j.c +WINDOW w AS (ORDER BY j.tdate + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL PATTERN (p q+) + DEFINE p AS TRUE, q AS c > ''); + c | tdate | cnt +------+------------+----- + zz | 01-01-2024 | 2 + zzzz | 01-02-2024 | 0 +(2 rows) + +DROP TABLE rpr_phv_src, rpr_phv_dim, rpr_phv_out; +-- A WINDOW clause no window function names is never executed, but what its +-- DEFINE reads is marked needed at relation 0 all the same, and an outer join +-- is not removable while something above still needs the inner side. +-- remove_unused_subquery_outputs() empties defineClause for a dead window, +-- but it only runs for a subquery; at the top level nothing does. The three +-- plans below isolate it: no WINDOW clause and a plain one both lose the +-- join, and only the row pattern one keeps it. +CREATE TABLE rpr_jr (id int, v int); +CREATE TABLE rpr_jr_u (id int PRIMARY KEY, uval int); +INSERT INTO rpr_jr SELECT g, g * 10 FROM generate_series(1, 5) g; +INSERT INTO rpr_jr_u SELECT g, g * 100 FROM generate_series(1, 5) g; +EXPLAIN (COSTS OFF) +SELECT t.id FROM rpr_jr t LEFT JOIN rpr_jr_u u ON t.id = u.id; + QUERY PLAN +---------------------- + Seq Scan on rpr_jr t +(1 row) + +EXPLAIN (COSTS OFF) +SELECT t.id FROM rpr_jr t LEFT JOIN rpr_jr_u u ON t.id = u.id +WINDOW w AS (ORDER BY t.id); + QUERY PLAN +---------------------- + Seq Scan on rpr_jr t +(1 row) + +EXPLAIN (COSTS OFF) +SELECT t.id FROM rpr_jr t LEFT JOIN rpr_jr_u u ON t.id = u.id +WINDOW w AS (ORDER BY t.id + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A B+) DEFINE B AS uval > PREV(uval)); + QUERY PLAN +------------------------------------ + Hash Left Join + Hash Cond: (t.id = u.id) + -> Seq Scan on rpr_jr t + -> Hash + -> Seq Scan on rpr_jr_u u +(5 rows) + +DROP TABLE rpr_jr, rpr_jr_u; -- ============================================================ -- Complex Expression Tests -- ============================================================ @@ -9561,6 +9759,63 @@ SELECT v, cnt FROM rpr_srf_inline(3) ORDER BY v; DROP TABLE rpr_srf_t; DROP FUNCTION rpr_srf_inline(int); DROP TABLE rpr_planner; +-- A DEFINE clause reading a compound GROUP BY expression. After grouping +-- only the expression itself exists, so make_window_input_target() has to +-- take it whole and stop: asking for the Vars underneath would ask the +-- grouping step for columns it cannot produce. "((a + b))" alone on the +-- Output lines, with no bare a or b anywhere above the HashAggregate, is the +-- assertion. +CREATE TABLE rpr_gexp (a int, b int); +INSERT INTO rpr_gexp VALUES (1, 1), (2, 2), (3, 3), (4, 4); +SELECT a + b AS ab, count(*) OVER w AS c +FROM rpr_gexp +GROUP BY a + b +WINDOW w AS (ORDER BY a + b + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (X+) DEFINE X AS a + b > 2); + ab | c +----+--- + 2 | 0 + 4 | 3 + 6 | 0 + 8 | 0 +(4 rows) + +EXPLAIN (VERBOSE, COSTS OFF) +SELECT a + b AS ab, count(*) OVER w AS c +FROM rpr_gexp +GROUP BY a + b +WINDOW w AS (ORDER BY a + b + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (X+) DEFINE X AS a + b > 2); + QUERY PLAN +-------------------------------------------------------------------------------------------------------- + WindowAgg + Output: ((a + b)), count(*) OVER w + Window: w AS (ORDER BY ((rpr_gexp.a + rpr_gexp.b)) ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING) + Pattern: x+# + -> Sort + Output: ((a + b)) + Sort Key: ((rpr_gexp.a + rpr_gexp.b)) + -> HashAggregate + Output: ((a + b)) + Group Key: (rpr_gexp.a + rpr_gexp.b) + -> Seq Scan on public.rpr_gexp + Output: (a + b) +(12 rows) + +-- Reaching below the grouping expression is rejected, as it would be in any +-- other clause evaluated after grouping. +SELECT a + b AS ab, count(*) OVER w AS c +FROM rpr_gexp +GROUP BY a + b +WINDOW w AS (ORDER BY a + b + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (X+) DEFINE X AS a > 2); +ERROR: column "rpr_gexp.a" must appear in the GROUP BY clause or be used in an aggregate function +LINE 6: PATTERN (X+) DEFINE X AS a > 2); + ^ +DROP TABLE rpr_gexp; -- ============================================================ -- Stress Tests -- ============================================================ diff --git a/src/test/regress/expected/rpr_integration.out b/src/test/regress/expected/rpr_integration.out index 7c290417e66..8980887a700 100644 --- a/src/test/regress/expected/rpr_integration.out +++ b/src/test/regress/expected/rpr_integration.out @@ -34,7 +34,7 @@ -- B8. RPR + Incremental sort -- B9. RPR + Volatile function in DEFINE -- B10. RPR + Correlated subquery in WHERE --- B11. RPR + Junk targetlist pruning +-- B11. RPR + DEFINE-only column pruning -- B12. RPR + Correlated navigation offsets -- B13. RPR + DEFINE-only parameter caching -- B14. RPR + Multiple window definitions @@ -507,7 +507,7 @@ SELECT count(*), sum(cnt) FROM ( Aggregate Output: count(*), sum((count(*) OVER w)) -> WindowAgg - Output: count(*) OVER w, rpr_integ.val + Output: count(*) OVER w Window: w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING) Pattern: a+# Nav Mark Lookback: 1 @@ -605,9 +605,10 @@ SELECT count(*) FROM ( -- The same column has to survive at the top level, where -- remove_unused_subquery_outputs() never runs at all: "val" is referenced only --- by DEFINE, so the parser's resjunk targetlist entry is the only thing --- carrying it into the WindowAgg's input. The trailing "val" on the --- WindowAgg's Output line is the assertion. +-- by DEFINE, so build_base_rel_tlists() marking it needed is the only thing +-- carrying it up the join tree, and make_window_input_target() is what asks +-- for it again. "val" on the Sort and Seq Scan Output lines, below a +-- WindowAgg that does not output it, is the assertion. EXPLAIN (VERBOSE, COSTS OFF) SELECT id, count(*) OVER w AS cnt FROM rpr_integ @@ -618,7 +619,7 @@ WINDOW w AS (ORDER BY id QUERY PLAN ----------------------------------------------------------------------------------------- WindowAgg - Output: id, count(*) OVER w, val + Output: id, count(*) OVER w Window: w AS (ORDER BY rpr_integ.id ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING) Pattern: a b+ Nav Mark Lookback: 1 @@ -721,7 +722,7 @@ WINDOW w AS (ORDER BY t.id QUERY PLAN --------------------------------------------------------------------------------- WindowAgg - Output: t.id, count(*) OVER w, (COALESCE(rpr_integ_u.uval, 0)) + Output: t.id, count(*) OVER w Window: w AS (ORDER BY t.id ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING) Pattern: a b+ Nav Mark Lookback: 1 @@ -916,6 +917,63 @@ SELECT c FROM ( Output: rpr_integ.id (10 rows) +-- The same shape with the window function one level down, inside an +-- expression. The pre-pass that settles which winrefs are still live only +-- replaces an entry whose top-level node is a WindowFunc, so this one is +-- still standing when the live set is read and w2 is reported live; the loop +-- after it replaces the entry all the same and w2 goes inactive anyway. +-- "val" is therefore kept for a window that never runs, where the bare case +-- above drops it. Not a wrong answer -- the direction is over-retention -- +-- but the two cases should agree, and making the live set exact is left to +-- its own commit. The difference between this plan and the one above is the +-- assertion. +EXPLAIN (VERBOSE, COSTS OFF) +SELECT c FROM ( + SELECT count(*) OVER w1 AS c, (count(*) OVER w2) + 1 AS unread, val + FROM rpr_integ + WINDOW w1 AS (ORDER BY id), + w2 AS (ORDER BY id + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A B+) + DEFINE B AS val > PREV(val)) +) t; + QUERY PLAN +----------------------------------------------------------------------------- + Subquery Scan on t + Output: t.c + -> WindowAgg + Output: count(*) OVER w1, NULL::bigint, rpr_integ.val, rpr_integ.id + Window: w1 AS (ORDER BY rpr_integ.id) + -> Sort + Output: rpr_integ.id, rpr_integ.val + Sort Key: rpr_integ.id + -> Seq Scan on public.rpr_integ + Output: rpr_integ.id, rpr_integ.val +(10 rows) + +SELECT c FROM ( + SELECT count(*) OVER w1 AS c, (count(*) OVER w2) + 1 AS unread, val + FROM rpr_integ + WINDOW w1 AS (ORDER BY id), + w2 AS (ORDER BY id + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A B+) + DEFINE B AS val > PREV(val)) +) t; + c +---- + 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 + 10 +(10 rows) + CREATE TABLE rpr_integ_two (id int, v1 int, v2 int); INSERT INTO rpr_integ_two SELECT i, i * 10, i * 100 FROM generate_series(1, 5) i; -- Whether a window is active is decided per window clause, not for row pattern @@ -1003,9 +1061,9 @@ LINE 6: DEFINE B AS rpr_integ IS NOT NULL) HINT: A DEFINE condition may reference individual columns only. -- It still reaches a DEFINE clause without being written there: pulling up a -- subquery substitutes that subquery's output expressions into defineClause, --- and one of them can be 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 +-- and one of them can be a whole-row Var (attribute number 0). The window +-- input target takes it 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. @@ -1023,7 +1081,7 @@ SELECT sum(c) FROM ( Aggregate Output: sum((count(*) OVER w)) -> WindowAgg - Output: NULL::integer, count(*) OVER w, r.id, r.* + Output: NULL::integer, count(*) OVER w, r.id Window: w AS (ORDER BY r.id ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING) Pattern: a b+ -> Sort @@ -1257,7 +1315,7 @@ WINDOW QUERY PLAN --------------------------------------------------------------------------------------------------- WindowAgg - Output: (count(*) OVER w_rpr), count(*) OVER w_normal, id, val + Output: (count(*) OVER w_rpr), count(*) OVER w_normal, id Window: w_normal AS (ORDER BY rpr_integ.id ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING) -> WindowAgg Output: id, val, count(*) OVER w_rpr @@ -1919,8 +1977,8 @@ 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. +-- Only varlevelsup separates them, so the window input target has to take y +-- 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); @@ -2167,10 +2225,10 @@ ORDER BY o.id; (10 rows) -- ============================================================ --- B11. RPR + Junk targetlist pruning +-- B11. RPR + DEFINE-only column pruning -- ============================================================ --- Verify that the junk targetlist entry planted for a DEFINE-only --- column does not keep an unrelated column alive. DEFINE references +-- Verify that carrying a DEFINE-only column to the WindowAgg's input +-- 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); diff --git a/src/test/regress/sql/rpr_base.sql b/src/test/regress/sql/rpr_base.sql index db6afdcc257..4aadcf0f608 100644 --- a/src/test/regress/sql/rpr_base.sql +++ b/src/test/regress/sql/rpr_base.sql @@ -3313,9 +3313,52 @@ WINDOW w AS ( PATTERN (A+) DEFINE A AS ROW((items).*) IS NOT NULL ); + DROP TABLE rpr_composite; DROP TYPE rpr_item; +-- A composite value that reaches DEFINE by way of a subquery Var only takes +-- its ROW(...) shape after pullup, and the ORDER BY copy's sortgroupref +-- keeps it from being flattened. make_window_input_target() adds the fields +-- the split leaves behind. +CREATE TABLE rpr_ordrow (a int, b int); +INSERT INTO rpr_ordrow SELECT g, g % 4 FROM generate_series(1, 10) g; +SELECT count(*) OVER w AS c +FROM (SELECT ROW(a, b) AS x FROM rpr_ordrow) s +WINDOW w AS (ORDER BY x + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL PATTERN (P Q+) DEFINE P AS TRUE, Q AS x IS NOT NULL); +-- Control: without ORDER BY, x is flattened normally and this succeeds too. +SELECT count(*) OVER w AS c +FROM (SELECT ROW(a, b) AS x FROM rpr_ordrow) s +WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL PATTERN (P Q+) DEFINE P AS TRUE, Q AS x IS NOT NULL); +DROP TABLE rpr_ordrow; + +-- The same split by way of a pulled-up composite target, both as a plain +-- subquery and as a view. +CREATE TABLE rpr_partrow (a int, b int); +INSERT INTO rpr_partrow VALUES (1, 1), (2, 2), (3, 3); +SELECT count(*) OVER w +FROM (SELECT b, row(a, 1) AS k FROM rpr_partrow) s +WINDOW w AS (PARTITION BY k ORDER BY b + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (p q+) DEFINE q AS k IS NOT NULL); +CREATE TYPE rpr_partrow_t AS (x int, y int); +CREATE VIEW rpr_partrow_v AS SELECT b, row(a, 1)::rpr_partrow_t AS k FROM rpr_partrow; +SELECT count(*) OVER w FROM rpr_partrow_v +WINDOW w AS (PARTITION BY k ORDER BY b + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (p q+) DEFINE q AS k IS NOT NULL); +-- Control: PATTERN/DEFINE aside, the same window clause runs fine. +SELECT count(*) OVER w +FROM (SELECT b, row(a, 1) AS k FROM rpr_partrow) s +WINDOW w AS (PARTITION BY k ORDER BY b + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING); +DROP VIEW rpr_partrow_v; +DROP TYPE rpr_partrow_t; +DROP TABLE rpr_partrow; + -- ERROR: undefined column in DEFINE SELECT COUNT(*) OVER w FROM rpr_err @@ -5051,6 +5094,88 @@ ORDER BY k; DROP TABLE rpr_join5, rpr_join6; +-- A DEFINE clause reading a USING column whose two sides differ in typmod. +-- The merged column stays a join alias Var, pullup leaves its joinaliasvars +-- entry a non-trivial expression, and the outer join's nullingrels wrap that +-- in a PlaceHolderVar. The target list copy and the DEFINE copy are wrapped +-- by separate calls, so their phids differ and equal() does not match them -- +-- the window input has to carry the DEFINE clause's own PlaceHolderVar. +CREATE TABLE rpr_phv_src (n int); +CREATE TABLE rpr_phv_dim (c varchar(10), tdate date); +CREATE TABLE rpr_phv_out (k varchar); +INSERT INTO rpr_phv_src VALUES (2), (4); +INSERT INTO rpr_phv_dim VALUES ('zz', '2024-01-01'), ('zzzz', '2024-01-02'); +INSERT INTO rpr_phv_out VALUES ('zz'), ('zzzz'); + +SELECT j.c, j.tdate, count(*) OVER w AS cnt +FROM rpr_phv_out o1 + LEFT JOIN ( (SELECT n, repeat('z', n)::varchar(5) AS c FROM rpr_phv_src) s + JOIN rpr_phv_dim USING (c) ) j + ON o1.k = j.c +WINDOW w AS (ORDER BY j.tdate + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL PATTERN (p q+) + DEFINE p AS TRUE, q AS c > ''); + +-- The same with one more join level above it. Reaching the window input is +-- not enough on its own: an intermediate join emits only what something above +-- has declared a need for, so what a DEFINE clause reads is marked needed at +-- relation 0 the way the target list's own columns are. +SELECT j.c, j.tdate, count(*) OVER w AS cnt +FROM rpr_phv_out o1 + LEFT JOIN rpr_phv_out o2 ON o1.k = o2.k + LEFT JOIN ( (SELECT n, repeat('z', n)::varchar(5) AS c FROM rpr_phv_src) s + JOIN rpr_phv_dim USING (c) ) j + ON o2.k = j.c +WINDOW w AS (ORDER BY j.tdate + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL PATTERN (p q+) + DEFINE p AS TRUE, q AS c > ''); + +-- Control: with both sides of USING at the same typmod the merged column is a +-- plain Var of one side, no PlaceHolderVar is built, and neither shape above +-- needs any of this. +SELECT j.c, j.tdate, count(*) OVER w AS cnt +FROM rpr_phv_out o1 + LEFT JOIN rpr_phv_out o2 ON o1.k = o2.k + LEFT JOIN ( (SELECT n, repeat('z', n)::varchar(10) AS c + FROM rpr_phv_src) s + JOIN rpr_phv_dim USING (c) ) j + ON o2.k = j.c +WINDOW w AS (ORDER BY j.tdate + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + INITIAL PATTERN (p q+) + DEFINE p AS TRUE, q AS c > ''); + +DROP TABLE rpr_phv_src, rpr_phv_dim, rpr_phv_out; + +-- A WINDOW clause no window function names is never executed, but what its +-- DEFINE reads is marked needed at relation 0 all the same, and an outer join +-- is not removable while something above still needs the inner side. +-- remove_unused_subquery_outputs() empties defineClause for a dead window, +-- but it only runs for a subquery; at the top level nothing does. The three +-- plans below isolate it: no WINDOW clause and a plain one both lose the +-- join, and only the row pattern one keeps it. +CREATE TABLE rpr_jr (id int, v int); +CREATE TABLE rpr_jr_u (id int PRIMARY KEY, uval int); +INSERT INTO rpr_jr SELECT g, g * 10 FROM generate_series(1, 5) g; +INSERT INTO rpr_jr_u SELECT g, g * 100 FROM generate_series(1, 5) g; + +EXPLAIN (COSTS OFF) +SELECT t.id FROM rpr_jr t LEFT JOIN rpr_jr_u u ON t.id = u.id; + +EXPLAIN (COSTS OFF) +SELECT t.id FROM rpr_jr t LEFT JOIN rpr_jr_u u ON t.id = u.id +WINDOW w AS (ORDER BY t.id); + +EXPLAIN (COSTS OFF) +SELECT t.id FROM rpr_jr t LEFT JOIN rpr_jr_u u ON t.id = u.id +WINDOW w AS (ORDER BY t.id + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A B+) DEFINE B AS uval > PREV(uval)); + +DROP TABLE rpr_jr, rpr_jr_u; + -- ============================================================ -- Complex Expression Tests -- ============================================================ @@ -5807,6 +5932,41 @@ DROP FUNCTION rpr_srf_inline(int); DROP TABLE rpr_planner; +-- A DEFINE clause reading a compound GROUP BY expression. After grouping +-- only the expression itself exists, so make_window_input_target() has to +-- take it whole and stop: asking for the Vars underneath would ask the +-- grouping step for columns it cannot produce. "((a + b))" alone on the +-- Output lines, with no bare a or b anywhere above the HashAggregate, is the +-- assertion. +CREATE TABLE rpr_gexp (a int, b int); +INSERT INTO rpr_gexp VALUES (1, 1), (2, 2), (3, 3), (4, 4); + +SELECT a + b AS ab, count(*) OVER w AS c +FROM rpr_gexp +GROUP BY a + b +WINDOW w AS (ORDER BY a + b + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (X+) DEFINE X AS a + b > 2); + +EXPLAIN (VERBOSE, COSTS OFF) +SELECT a + b AS ab, count(*) OVER w AS c +FROM rpr_gexp +GROUP BY a + b +WINDOW w AS (ORDER BY a + b + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (X+) DEFINE X AS a + b > 2); + +-- Reaching below the grouping expression is rejected, as it would be in any +-- other clause evaluated after grouping. +SELECT a + b AS ab, count(*) OVER w AS c +FROM rpr_gexp +GROUP BY a + b +WINDOW w AS (ORDER BY a + b + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (X+) DEFINE X AS a > 2); + +DROP TABLE rpr_gexp; + -- ============================================================ -- Stress Tests -- ============================================================ diff --git a/src/test/regress/sql/rpr_integration.sql b/src/test/regress/sql/rpr_integration.sql index 0ae15bafbd3..5e895a27e40 100644 --- a/src/test/regress/sql/rpr_integration.sql +++ b/src/test/regress/sql/rpr_integration.sql @@ -34,7 +34,7 @@ -- B8. RPR + Incremental sort -- B9. RPR + Volatile function in DEFINE -- B10. RPR + Correlated subquery in WHERE --- B11. RPR + Junk targetlist pruning +-- B11. RPR + DEFINE-only column pruning -- B12. RPR + Correlated navigation offsets -- B13. RPR + DEFINE-only parameter caching -- B14. RPR + Multiple window definitions @@ -390,9 +390,10 @@ SELECT count(*) FROM ( -- The same column has to survive at the top level, where -- remove_unused_subquery_outputs() never runs at all: "val" is referenced only --- by DEFINE, so the parser's resjunk targetlist entry is the only thing --- carrying it into the WindowAgg's input. The trailing "val" on the --- WindowAgg's Output line is the assertion. +-- by DEFINE, so build_base_rel_tlists() marking it needed is the only thing +-- carrying it up the join tree, and make_window_input_target() is what asks +-- for it again. "val" on the Sort and Seq Scan Output lines, below a +-- WindowAgg that does not output it, is the assertion. EXPLAIN (VERBOSE, COSTS OFF) SELECT id, count(*) OVER w AS cnt FROM rpr_integ @@ -543,6 +544,37 @@ SELECT c FROM ( DEFINE B AS val > PREV(val)) ) t; +-- The same shape with the window function one level down, inside an +-- expression. The pre-pass that settles which winrefs are still live only +-- replaces an entry whose top-level node is a WindowFunc, so this one is +-- still standing when the live set is read and w2 is reported live; the loop +-- after it replaces the entry all the same and w2 goes inactive anyway. +-- "val" is therefore kept for a window that never runs, where the bare case +-- above drops it. Not a wrong answer -- the direction is over-retention -- +-- but the two cases should agree, and making the live set exact is left to +-- its own commit. The difference between this plan and the one above is the +-- assertion. +EXPLAIN (VERBOSE, COSTS OFF) +SELECT c FROM ( + SELECT count(*) OVER w1 AS c, (count(*) OVER w2) + 1 AS unread, val + FROM rpr_integ + WINDOW w1 AS (ORDER BY id), + w2 AS (ORDER BY id + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A B+) + DEFINE B AS val > PREV(val)) +) t; + +SELECT c FROM ( + SELECT count(*) OVER w1 AS c, (count(*) OVER w2) + 1 AS unread, val + FROM rpr_integ + WINDOW w1 AS (ORDER BY id), + w2 AS (ORDER BY id + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A B+) + DEFINE B AS val > PREV(val)) +) t; + CREATE TABLE rpr_integ_two (id int, v1 int, v2 int); INSERT INTO rpr_integ_two SELECT i, i * 10, i * 100 FROM generate_series(1, 5) i; @@ -593,9 +625,9 @@ SELECT sum(c) FROM ( -- It still reaches a DEFINE clause without being written there: pulling up a -- subquery substitutes that subquery's output expressions into defineClause, --- and one of them can be 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 +-- and one of them can be a whole-row Var (attribute number 0). The window +-- input target takes it 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. @@ -1153,8 +1185,8 @@ 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. +-- Only varlevelsup separates them, so the window input target has to take y +-- 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); @@ -1328,10 +1360,10 @@ FROM rpr_integ o ORDER BY o.id; -- ============================================================ --- B11. RPR + Junk targetlist pruning +-- B11. RPR + DEFINE-only column pruning -- ============================================================ --- Verify that the junk targetlist entry planted for a DEFINE-only --- column does not keep an unrelated column alive. DEFINE references +-- Verify that carrying a DEFINE-only column to the WindowAgg's input +-- 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); diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 49e576ade3f..8cf7d5c8a00 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -666,7 +666,6 @@ DefElemAction DefaultACLInfo DefineMetadataContext DefinePhase -DefinePlantCtx DefineStmt DefineWalkCtx DefnDumperPtr -- 2.54.0 (Apple Git-157)