From 35583d77e798b4bead63b00c9f9d5ff414b932d8 Mon Sep 17 00:00:00 2001 From: Henson Choi Date: Fri, 18 Sep 2026 12:04:44 +0900 Subject: [PATCH] Keep a row null test whole in a DEFINE clause A DEFINE expression is planted in the targetlist one bare Var at a time, which holds only as long as the expression keeps the shape it was planted with. It does not 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 not splitting that test for a DEFINE clause. A DEFINE condition is evaluated once per row inside the pattern matcher rather than driving an index or a join, so it gains nothing from a split the rest of the planner wants for its own reasons. Add EXPRKIND_RPR_DEFINE for the defineClause preprocessing pass, and eval_const_expressions_keep_row_nulltest() for it to call in place of eval_const_expressions(). Both shapes the split reached this way are covered: a composite ORDER BY key read by DEFINE, and a pulled-up composite partition key read by DEFINE through a subquery and through a view. --- src/backend/optimizer/plan/planner.c | 18 +++++- src/backend/optimizer/util/clauses.c | 27 +++++++- src/include/optimizer/optimizer.h | 1 + src/test/regress/expected/rpr_base.out | 88 ++++++++++++++++++++++++++ src/test/regress/sql/rpr_base.sql | 42 ++++++++++++ 5 files changed, 172 insertions(+), 4 deletions(-) diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 825a4ef1e3e..9de05968d25 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -98,6 +98,7 @@ create_upper_paths_hook_type create_upper_paths_hook = NULL; #define EXPRKIND_TABLEFUNC 11 #define EXPRKIND_TABLEFUNC_LATERAL 12 #define EXPRKIND_GROUPEXPR 13 +#define EXPRKIND_RPR_DEFINE 14 /* * Data specific to grouping sets @@ -1064,7 +1065,7 @@ subquery_planner(PlannerGlobal *glob, Query *parse, char *plan_name, EXPRKIND_LIMIT); wc->defineClause = (List *) preprocess_expression(root, (Node *) wc->defineClause, - EXPRKIND_TARGET); + EXPRKIND_RPR_DEFINE); /* * Reject volatile expressions in an RPR DEFINE clause. This is done @@ -1491,7 +1492,17 @@ preprocess_expression(PlannerInfo *root, Node *expr, int kind) * careful to maintain AND/OR flatness --- that is, do not generate a tree * with AND directly under AND, nor OR directly under OR. */ - if (kind != EXPRKIND_RTFUNC) + if (kind == EXPRKIND_RPR_DEFINE) + { + /* + * Don't split a ROW(...) IS [NOT] NULL in DEFINE into per-field + * tests: the fields it would split into are never planted (see + * transformDefineClause()), and DEFINE gets no benefit from the split + * anyway since it's evaluated per row, not via an index. + */ + expr = eval_const_expressions_keep_row_nulltest(root, expr); + } + else if (kind != EXPRKIND_RTFUNC) expr = eval_const_expressions(root, expr); /* @@ -1512,7 +1523,8 @@ preprocess_expression(PlannerInfo *root, Node *expr, int kind) * hashfuncid of any that might execute more quickly by using hash lookups * instead of a linear search. */ - if (kind == EXPRKIND_QUAL || kind == EXPRKIND_TARGET) + if (kind == EXPRKIND_QUAL || kind == EXPRKIND_TARGET || + kind == EXPRKIND_RPR_DEFINE) { convert_saop_to_hashed_saop(expr); } diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index 55cebe4a74b..b65454d6795 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -71,6 +71,7 @@ typedef struct List *active_fns; Node *case_val; bool estimate; + bool keep_row_nulltest; /* don't split ROW(...) IS [NOT] NULL */ } eval_const_expressions_context; typedef struct @@ -2626,6 +2627,29 @@ eval_const_expressions(PlannerInfo *root, Node *node) context.active_fns = NIL; /* nothing being recursively simplified */ context.case_val = NULL; /* no CASE being examined */ context.estimate = false; /* safe transformations only */ + context.keep_row_nulltest = false; + return eval_const_expressions_mutator(node, &context); +} + +/* + * eval_const_expressions_keep_row_nulltest + * As eval_const_expressions(), but keeps a ROW(...) IS [NOT] NULL + * test whole instead of splitting it into one test per field. + */ +Node * +eval_const_expressions_keep_row_nulltest(PlannerInfo *root, Node *node) +{ + eval_const_expressions_context context; + + if (root) + context.boundParams = root->glob->boundParams; + else + context.boundParams = NULL; + context.root = root; + context.active_fns = NIL; + context.case_val = NULL; + context.estimate = false; + context.keep_row_nulltest = true; return eval_const_expressions_mutator(node, &context); } @@ -3956,7 +3980,8 @@ eval_const_expressions_mutator(Node *node, arg = eval_const_expressions_mutator((Node *) ntest->arg, context); - if (ntest->argisrow && arg && IsA(arg, RowExpr)) + if (ntest->argisrow && arg && IsA(arg, RowExpr) && + !context->keep_row_nulltest) { /* * We break ROW(...) IS [NOT] NULL into separate tests on diff --git a/src/include/optimizer/optimizer.h b/src/include/optimizer/optimizer.h index cb6241e2bdd..5a7d5c7f301 100644 --- a/src/include/optimizer/optimizer.h +++ b/src/include/optimizer/optimizer.h @@ -145,6 +145,7 @@ extern bool contain_volatile_functions_after_planning(Expr *expr); extern bool contain_volatile_functions_not_nextval(Node *clause); extern Node *eval_const_expressions(PlannerInfo *root, Node *node); +extern Node *eval_const_expressions_keep_row_nulltest(PlannerInfo *root, Node *node); extern void convert_saop_to_hashed_saop(Node *node); diff --git a/src/test/regress/expected/rpr_base.out b/src/test/regress/expected/rpr_base.out index 539a4fb4bc7..bda37b4fc5f 100644 --- a/src/test/regress/expected/rpr_base.out +++ b/src/test/regress/expected/rpr_base.out @@ -5184,6 +5184,94 @@ 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 -- too late for anything to have planted +-- its fields. Keeping the DEFINE side unsplit avoids needing them at all. +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 diff --git a/src/test/regress/sql/rpr_base.sql b/src/test/regress/sql/rpr_base.sql index db6afdcc257..60625f6e6eb 100644 --- a/src/test/regress/sql/rpr_base.sql +++ b/src/test/regress/sql/rpr_base.sql @@ -3313,9 +3313,51 @@ 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 -- too late for anything to have planted +-- its fields. Keeping the DEFINE side unsplit avoids needing them at all. +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 -- 2.54.0 (Apple Git-157)