From d3d59b36e04fbca3ce511c8da53e92d30b30b87e Mon Sep 17 00:00:00 2001 From: shihao zhong Date: Mon, 5 Oct 2026 21:46:34 -0400 Subject: [PATCH v1 5/5] Keep duplicate grouping expressions apart with grouping sets Preprocessing can simplify two grouping expressions to the same expression, such as a and COALESCE(a, 0) for a NOT NULL column. With grouping sets, expressions above the grouping step then read the wrong one. Wrap such a duplicate in a PlaceHolderVar, also when the two differ only by a binary-compatible cast. --- src/backend/optimizer/plan/planner.c | 108 +++++++++++++++++++++ src/test/regress/expected/groupingsets.out | 80 +++++++++++++++ src/test/regress/sql/groupingsets.sql | 38 ++++++++ 3 files changed, 226 insertions(+) diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index edaa42d713a..96ada9d9457 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -48,6 +48,7 @@ #include "optimizer/plancat.h" #include "optimizer/planmain.h" #include "optimizer/planner.h" +#include "optimizer/placeholder.h" #include "optimizer/prep.h" #include "optimizer/subselect.h" #include "optimizer/tlist.h" @@ -156,6 +157,8 @@ typedef struct static Node *preprocess_expression(PlannerInfo *root, Node *expr, int kind); static void preprocess_qual_conditions(PlannerInfo *root, Node *jtnode); static Bitmapset *find_having_conflicts(Query *parse, Index group_rtindex); +static void wrap_duplicate_group_exprs(PlannerInfo *root); +static bool wrap_srf_argument_walker(Node *node, PlannerInfo *root); static Oid having_var_grouping_eqop(Var *var, void *context); static Oid group_var_eqop(Query *parse, Var *var); static void preprocess_subquery_phvs(PlannerInfo *root, Node *node); @@ -1221,6 +1224,10 @@ subquery_planner(PlannerGlobal *glob, Query *parse, char *plan_name, */ if (parse->hasGroupRTE) { + /* With grouping sets, duplicate grouping expressions must stay apart */ + if (parse->groupingSets) + wrap_duplicate_group_exprs(root); + parse->targetList = (List *) flatten_group_exprs(root, root->parse, (Node *) parse->targetList); parse->havingQual = @@ -1541,6 +1548,107 @@ preprocess_qual_conditions(PlannerInfo *root, Node *jtnode) (int) nodeTag(jtnode)); } +/* + * wrap_duplicate_group_exprs + * Wrap a grouping expression that duplicates an earlier one in a + * PlaceHolderVar. + * + * The parser gives equal grouping expressions a single entry, so duplicates + * only appear when preprocessing simplifies two different expressions to the + * same thing, such as "a" and "COALESCE(a, 0)" for a NOT NULL column. With + * grouping sets the two are nulled separately, and expressions above the + * grouping step could not tell which one they reference. + * + * Expressions that differ only by binary relabeling count as duplicates too, + * since equivalence classes do not distinguish them. + * + * A set-returning function cannot go inside a PlaceHolderVar, so for such an + * expression we wrap one argument of the function instead. That is enough to + * make the expression distinct. + */ +static void +wrap_duplicate_group_exprs(PlannerInfo *root) +{ + RangeTblEntry *rte = rt_fetch(root->group_rtindex, root->parse->rtable); + Relids phrels = NULL; + ListCell *lc; + + foreach(lc, rte->groupexprs) + { + Node *expr = (Node *) lfirst(lc); + Node *bare = expr; + bool duplicate = false; + ListCell *lc2; + + while (IsA(bare, RelabelType)) + bare = (Node *) ((RelabelType *) bare)->arg; + + foreach(lc2, rte->groupexprs) + { + Node *other = (Node *) lfirst(lc2); + + if (lc2 == lc) + break; + while (IsA(other, RelabelType)) + other = (Node *) ((RelabelType *) other)->arg; + if (equal(bare, other)) + { + duplicate = true; + break; + } + } + + if (!duplicate) + continue; + + if (expression_returns_set(expr)) + { + (void) wrap_srf_argument_walker(expr, root); + continue; + } + + if (phrels == NULL) + phrels = get_relids_in_jointree((Node *) root->parse->jointree, + true, false); + lfirst(lc) = make_placeholder_expr(root, (Expr *) expr, phrels); + } +} + +/* + * Wrap the first argument that does not return a set, of the first + * set-returning function found in the expression, in a PlaceHolderVar. + */ +static bool +wrap_srf_argument_walker(Node *node, PlannerInfo *root) +{ + List *args = NIL; + ListCell *lc; + + if (node == NULL) + return false; + if (IsA(node, FuncExpr) && ((FuncExpr *) node)->funcretset) + args = ((FuncExpr *) node)->args; + else if (IsA(node, OpExpr) && ((OpExpr *) node)->opretset) + args = ((OpExpr *) node)->args; + + foreach(lc, args) + { + Node *arg = (Node *) lfirst(lc); + + if (!expression_returns_set(arg)) + { + Relids phrels; + + phrels = get_relids_in_jointree((Node *) root->parse->jointree, + true, false); + lfirst(lc) = make_placeholder_expr(root, (Expr *) arg, phrels); + return true; + } + } + + return expression_tree_walker(node, wrap_srf_argument_walker, root); +} + /* * find_having_conflicts * Identify HAVING clauses that must not be moved to WHERE because they diff --git a/src/test/regress/expected/groupingsets.out b/src/test/regress/expected/groupingsets.out index 08d5ce3156d..b4c91e74363 100644 --- a/src/test/regress/expected/groupingsets.out +++ b/src/test/regress/expected/groupingsets.out @@ -2762,4 +2762,84 @@ order by 1, 2; 2 | (6 rows) +-- test that grouping expressions simplified to the same expression are +-- still nulled separately +create temp table gstest_dup (a int not null); +insert into gstest_dup values (10), (20); +select a, coalesce(a, 0) as ca, coalesce(a, 0) is null as ca_is_null +from gstest_dup +group by grouping sets ((a), (coalesce(a, 0))) +order by 1, 2; + a | ca | ca_is_null +----+----+------------ + 10 | | t + 20 | | t + | 10 | f + | 20 | f +(4 rows) + +select a, coalesce(a, 0) as ca, count(*) +from gstest_dup +group by grouping sets ((a), (coalesce(a, 0))) +having coalesce(a, 0) is null +order by 1, 2; + a | ca | count +----+----+------- + 10 | | 1 + 20 | | 1 +(2 rows) + +-- likewise for grouping expressions that differ only by a binary-compatible +-- cast +select v, v::text as vt +from (values ('m'::varchar), ('c'), ('x'), ('a'), ('q'), ('f'), ('k'), ('b')) as t (v) +group by grouping sets ((v), (v::text)) +order by 1, 2; + v | vt +---+---- + a | + b | + c | + f | + k | + m | + q | + x | + | a + | b + | c + | f + | k + | m + | q + | x +(16 rows) + +-- and for volatile grouping expressions +select (random() > 2) as r1, coalesce(random() > 2) as r2, + coalesce(random() > 2) is null as r2_is_null +from gstest_dup +group by grouping sets ((random() > 2), (coalesce(random() > 2))) +order by 1, 2; + r1 | r2 | r2_is_null +----+----+------------ + f | | t + | f | f +(2 rows) + +-- and for set-returning ones +select generate_series(a, a) as s1, generate_series(a, coalesce(a, 0)) as s2, + generate_series(a, coalesce(a, 0)) is null as s2_is_null +from gstest_dup +group by grouping sets ((generate_series(a, a)), + (generate_series(a, coalesce(a, 0)))) +order by 1, 2; + s1 | s2 | s2_is_null +----+----+------------ + 10 | | t + 20 | | t + | 10 | f + | 20 | f +(4 rows) + -- end diff --git a/src/test/regress/sql/groupingsets.sql b/src/test/regress/sql/groupingsets.sql index c5d4ed2eb57..0745ea491c8 100644 --- a/src/test/regress/sql/groupingsets.sql +++ b/src/test/regress/sql/groupingsets.sql @@ -803,4 +803,42 @@ from (values (1, 1), (2, 2)) as t (a, b) group by rollup(a, ab) order by 1, 2; +-- test that grouping expressions simplified to the same expression are +-- still nulled separately +create temp table gstest_dup (a int not null); +insert into gstest_dup values (10), (20); + +select a, coalesce(a, 0) as ca, coalesce(a, 0) is null as ca_is_null +from gstest_dup +group by grouping sets ((a), (coalesce(a, 0))) +order by 1, 2; + +select a, coalesce(a, 0) as ca, count(*) +from gstest_dup +group by grouping sets ((a), (coalesce(a, 0))) +having coalesce(a, 0) is null +order by 1, 2; + +-- likewise for grouping expressions that differ only by a binary-compatible +-- cast +select v, v::text as vt +from (values ('m'::varchar), ('c'), ('x'), ('a'), ('q'), ('f'), ('k'), ('b')) as t (v) +group by grouping sets ((v), (v::text)) +order by 1, 2; + +-- and for volatile grouping expressions +select (random() > 2) as r1, coalesce(random() > 2) as r2, + coalesce(random() > 2) is null as r2_is_null +from gstest_dup +group by grouping sets ((random() > 2), (coalesce(random() > 2))) +order by 1, 2; + +-- and for set-returning ones +select generate_series(a, a) as s1, generate_series(a, coalesce(a, 0)) as s2, + generate_series(a, coalesce(a, 0)) is null as s2_is_null +from gstest_dup +group by grouping sets ((generate_series(a, a)), + (generate_series(a, coalesce(a, 0)))) +order by 1, 2; + -- end -- 2.37.1 (Apple Git-137.1)