From 568e0ee99311fef7b5e12e62c668ff419c0044bf Mon Sep 17 00:00:00 2001 From: Haibo Yan Date: Fri, 9 Oct 2026 19:30:44 -0700 Subject: [PATCH] Fix missing nullification for variable-free grouping expressions in grouping sets When a grouping expression simplifies to a constant or is variable-free, mark_nullable_by_grouping() wraps it in a PlaceHolderVar so it can carry the grouping step's nullingrels. However, mark_nullable_by_grouping() was calling make_placeholder_expr() anew for each reference to the grouping expression in the query tree, assigning a distinct phid to each instance. Because PlaceHolderVars are matched strictly by phid in setrefs.c (search_indexed_tlist_for_phv), non-grouping target entries referencing the grouping expression failed to match the grouping column in the subplan targetlist. As a fallback, setrefs.c reduced such PlaceHolderVars to their underlying constant expression, evaluating the constant directly in the upper plan and bypassing the Agg node's grouping-column nullification. Consequently, ROLLUP queries with expressions like (1 + 1) * 10 evaluated using the constant 2 instead of NULL on subtotal and grand-total rows. Fix this by caching PlaceHolderVars created for variable-free grouping expressions in PlannerInfo (group_phvars), indexed by grouping column varattno. References to the same grouping column share the same PHV ID, ensuring consistent identity across projections, HAVING quals, subqueries, and SortGroupClauses, while keeping distinct grouping columns with identical constant values separate. --- src/backend/optimizer/util/var.c | 33 ++++++- src/include/nodes/pathnodes.h | 7 ++ src/test/regress/expected/groupingsets.out | 110 +++++++++++++++++++++ src/test/regress/sql/groupingsets.sql | 55 +++++++++++ 4 files changed, 200 insertions(+), 5 deletions(-) diff --git a/src/backend/optimizer/util/var.c b/src/backend/optimizer/util/var.c index 337e0306752..eeea571c15a 100644 --- a/src/backend/optimizer/util/var.c +++ b/src/backend/optimizer/util/var.c @@ -1218,13 +1218,36 @@ mark_nullable_by_grouping(PlannerInfo *root, Node *newnode, Var *oldvar) !expression_returns_set(newnode)) { PlaceHolderVar *newphv; - Relids phrels; - phrels = get_relids_in_jointree((Node *) root->parse->jointree, - true, false); - Assert(!bms_is_empty(phrels)); + if (!root) + return newnode; - newphv = make_placeholder_expr(root, (Expr *) newnode, phrels); + if (root->group_phvars == NULL) + { + RangeTblEntry *rte = rt_fetch(oldvar->varno, root->parse->rtable); + int num_exprs = list_length(rte->groupexprs); + + root->group_phvars = (PlaceHolderVar **) + palloc0((num_exprs + 1) * sizeof(PlaceHolderVar *)); + root->num_group_phvars = num_exprs; + } + + Assert(oldvar->varattno > 0 && + oldvar->varattno <= root->num_group_phvars); + + if (root->group_phvars[oldvar->varattno] == NULL) + { + Relids phrels; + + phrels = get_relids_in_jointree((Node *) root->parse->jointree, + true, false); + Assert(!bms_is_empty(phrels)); + + newphv = make_placeholder_expr(root, (Expr *) newnode, phrels); + root->group_phvars[oldvar->varattno] = newphv; + } + + newphv = copyObject(root->group_phvars[oldvar->varattno]); /* newphv has zero phlevelsup and NULL phnullingrels; fix it */ newphv->phlevelsup = oldvar->varlevelsup; newphv->phnullingrels = bms_copy(oldvar->varnullingrels); diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index adb9066c1c3..e97fb444bec 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -665,6 +665,13 @@ struct PlannerInfo */ int group_rtindex; + /* + * PlaceHolderVars generated for variable-free grouping expressions, + * indexed by grouping Var attno (1-based). + */ + struct PlaceHolderVar **group_phvars pg_node_attr(read_write_ignore); + int num_group_phvars pg_node_attr(read_write_ignore); + /* * Information about aggregates. Filled by preprocess_aggrefs(). */ diff --git a/src/test/regress/expected/groupingsets.out b/src/test/regress/expected/groupingsets.out index 2b46212e8a0..6cf5828ed77 100644 --- a/src/test/regress/expected/groupingsets.out +++ b/src/test/regress/expected/groupingsets.out @@ -2835,4 +2835,114 @@ group by email is not null; (1 row) drop table gs_users; +-- test constant / variable-free grouping expressions with ROLLUP / grouping sets +select (1 + 1) * 10 +from generate_series(1, 10) +group by rollup (1 + 1) +order by 1; + ?column? +---------- + 20 + +(2 rows) + +explain (verbose, costs off) +select (1 + 1) * 10 +from generate_series(1, 10) +group by rollup (1 + 1) +order by 1; + QUERY PLAN +--------------------------------------------------------- + Sort + Output: (((2) * 10)), (2) + Sort Key: (((2) * 10)) + -> MixedAggregate + Output: ((2) * 10), (2) + Hash Key: 2 + Group Key: () + -> Function Scan on pg_catalog.generate_series + Output: 2 + Function Call: generate_series(1, 10) +(10 rows) + +-- correlated subquery +select (select (1 + 1) * 10) +from generate_series(1, 10) +group by rollup (1 + 1) +order by 1; + ?column? +---------- + 20 + +(2 rows) + +-- multiple expressions simplifying to identical constants +select (1 + 1) * 10 as a, (2 * 1) * 100 as b +from generate_series(1, 2) +group by rollup (1 + 1, 2 * 1) +order by 1, 2; + a | b +----+----- + 20 | 200 + 20 | + | +(3 rows) + +-- HAVING filter on constant grouping expression +select (1 + 1) * 10 +from generate_series(1, 10) +group by rollup (1 + 1) +having (1 + 1) is not null +order by 1; + ?column? +---------- + 20 +(1 row) + +select (1 + 1) * 10 +from generate_series(1, 10) +group by rollup (1 + 1) +having (1 + 1) is null +order by 1; + ?column? +---------- + +(1 row) + +-- CUBE with constant grouping expressions +select 1 + 1 as a, 2 + 2 as b, count(*) +from generate_series(1, 2) +group by cube (1 + 1, 2 + 2) +order by 1, 2; + a | b | count +---+---+------- + 2 | 4 | 2 + 2 | | 2 + | 4 | 2 + | | 2 +(4 rows) + +-- window function over grouping set output with constant expression +select (1 + 1) * 10 as val, + count(*) over (partition by (1 + 1)) as cnt +from generate_series(1, 2) +group by rollup (1 + 1) +order by 1; + val | cnt +-----+----- + 20 | 1 + | 1 +(2 rows) + +-- volatile expression +select count(*) +from generate_series(1, 2) +group by rollup (random()); + count +------- + 2 + 1 + 1 +(3 rows) + -- end diff --git a/src/test/regress/sql/groupingsets.sql b/src/test/regress/sql/groupingsets.sql index 9cf861f6316..923886162b9 100644 --- a/src/test/regress/sql/groupingsets.sql +++ b/src/test/regress/sql/groupingsets.sql @@ -846,4 +846,59 @@ group by email is not null; drop table gs_users; +-- test constant / variable-free grouping expressions with ROLLUP / grouping sets +select (1 + 1) * 10 +from generate_series(1, 10) +group by rollup (1 + 1) +order by 1; + +explain (verbose, costs off) +select (1 + 1) * 10 +from generate_series(1, 10) +group by rollup (1 + 1) +order by 1; + +-- correlated subquery +select (select (1 + 1) * 10) +from generate_series(1, 10) +group by rollup (1 + 1) +order by 1; + +-- multiple expressions simplifying to identical constants +select (1 + 1) * 10 as a, (2 * 1) * 100 as b +from generate_series(1, 2) +group by rollup (1 + 1, 2 * 1) +order by 1, 2; + +-- HAVING filter on constant grouping expression +select (1 + 1) * 10 +from generate_series(1, 10) +group by rollup (1 + 1) +having (1 + 1) is not null +order by 1; + +select (1 + 1) * 10 +from generate_series(1, 10) +group by rollup (1 + 1) +having (1 + 1) is null +order by 1; + +-- CUBE with constant grouping expressions +select 1 + 1 as a, 2 + 2 as b, count(*) +from generate_series(1, 2) +group by cube (1 + 1, 2 + 2) +order by 1, 2; + +-- window function over grouping set output with constant expression +select (1 + 1) * 10 as val, + count(*) over (partition by (1 + 1)) as cnt +from generate_series(1, 2) +group by rollup (1 + 1) +order by 1; + +-- volatile expression +select count(*) +from generate_series(1, 2) +group by rollup (random()); + -- end -- 2.54.0