From 368dc9daf8038b5fc267c59048b1dd354e0f2fda Mon Sep 17 00:00:00 2001 From: William Bernbaum Date: Wed, 26 Aug 2026 07:43:42 -0700 Subject: [PATCH v1 02/16] Name the grouping clause for eager aggregation Since the clause we group on will not always be the query's GROUP BY, record it in PlannerInfo and read it from there. --- src/backend/optimizer/plan/initsplan.c | 90 ++++++++++++++------------ src/backend/optimizer/plan/planmain.c | 1 + src/backend/optimizer/util/relnode.c | 2 +- src/include/nodes/pathnodes.h | 3 + 4 files changed, 54 insertions(+), 42 deletions(-) diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c index f08a918146c..85fc5651cb1 100644 --- a/src/backend/optimizer/plan/initsplan.c +++ b/src/backend/optimizer/plan/initsplan.c @@ -97,6 +97,7 @@ typedef struct GroupByColInfo static bool is_partial_agg_memory_risky(PlannerInfo *root); static void create_agg_clause_infos(PlannerInfo *root); +static bool grouping_key_usable(Expr *expr); static void create_grouping_expr_infos(PlannerInfo *root); static EquivalenceClass *get_eclass_for_sortgroupclause(PlannerInfo *root, SortGroupClause *sgc, @@ -706,6 +707,8 @@ setup_eager_aggregation(PlannerInfo *root) if (!root->processed_groupClause) return; + root->eager_group_clause = root->processed_groupClause; + /* * For now we don't try to support grouping sets. */ @@ -920,6 +923,50 @@ create_agg_clause_infos(PlannerInfo *root) } } +/* + * grouping_key_usable + * Can the given expression serve as a grouping key? + * + * For now we only support plain Vars. Beyond that, equality must imply image + * equality, or else merging two keys could lose information an upper qual + * needs. NUMERIC is the standard counterexample: 0 and 0.0 are equal to the + * equality operator but do not have the same byte image. + */ +static bool +grouping_key_usable(Expr *expr) +{ + TypeCacheEntry *tce; + Oid equalimageproc; + + if (!IsA(expr, Var)) + return false; + + tce = lookup_type_cache(exprType((Node *) expr), + TYPECACHE_BTREE_OPFAMILY); + if (!OidIsValid(tce->btree_opf) || + !OidIsValid(tce->btree_opintype)) + return false; + + equalimageproc = get_opfamily_proc(tce->btree_opf, + tce->btree_opintype, + tce->btree_opintype, + BTEQUALIMAGE_PROC); + + /* + * If there is no BTEQUALIMAGE_PROC, eager aggregation is assumed to be + * unsafe. Otherwise, we call the procedure to check. We must be careful + * to pass the expression's actual collation, rather than the data type's + * default collation, to ensure that non-deterministic collations are + * correctly handled. + */ + if (!OidIsValid(equalimageproc)) + return false; + + return DatumGetBool(OidFunctionCall1Coll(equalimageproc, + exprCollation((Node *) expr), + ObjectIdGetDatum(tce->btree_opintype))); +} + /* * create_grouping_expr_infos * Create a GroupingExprInfo for each expression usable as grouping key. @@ -940,53 +987,14 @@ create_grouping_expr_infos(PlannerInfo *root) Assert(root->group_expr_list == NIL); - foreach(lc, root->processed_groupClause) + foreach(lc, root->eager_group_clause) { SortGroupClause *sgc = lfirst_node(SortGroupClause, lc); TargetEntry *tle = get_sortgroupclause_tle(sgc, root->processed_tlist); - TypeCacheEntry *tce; - Oid equalimageproc; Assert(tle->ressortgroupref > 0); - /* - * For now we only support plain Vars as grouping expressions. - */ - if (!IsA(tle->expr, Var)) - return; - - /* - * Eager aggregation is only possible if equality implies image - * equality for each grouping key. Otherwise, placing keys with - * different byte images into the same group may result in the loss of - * information that could be necessary to evaluate upper qual clauses. - * - * For instance, the NUMERIC data type is not supported, as values - * that are considered equal by the equality operator (e.g., 0 and - * 0.0) can have different scales. - */ - tce = lookup_type_cache(exprType((Node *) tle->expr), - TYPECACHE_BTREE_OPFAMILY); - if (!OidIsValid(tce->btree_opf) || - !OidIsValid(tce->btree_opintype)) - return; - - equalimageproc = get_opfamily_proc(tce->btree_opf, - tce->btree_opintype, - tce->btree_opintype, - BTEQUALIMAGE_PROC); - - /* - * If there is no BTEQUALIMAGE_PROC, eager aggregation is assumed to - * be unsafe. Otherwise, we call the procedure to check. We must be - * careful to pass the expression's actual collation, rather than the - * data type's default collation, to ensure that non-deterministic - * collations are correctly handled. - */ - if (!OidIsValid(equalimageproc) || - !DatumGetBool(OidFunctionCall1Coll(equalimageproc, - exprCollation((Node *) tle->expr), - ObjectIdGetDatum(tce->btree_opintype)))) + if (!grouping_key_usable(tle->expr)) return; exprs = lappend(exprs, tle->expr); diff --git a/src/backend/optimizer/plan/planmain.c b/src/backend/optimizer/plan/planmain.c index 02495e22e24..407b89219e1 100644 --- a/src/backend/optimizer/plan/planmain.c +++ b/src/backend/optimizer/plan/planmain.c @@ -78,6 +78,7 @@ query_planner(PlannerInfo *root, root->placeholder_array_size = 0; root->agg_clause_list = NIL; root->group_expr_list = NIL; + root->eager_group_clause = NIL; root->tlist_vars = NIL; root->fkey_list = NIL; root->initial_rels = NIL; diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c index 4fd569d21b4..2d7b8f85fdf 100644 --- a/src/backend/optimizer/util/relnode.c +++ b/src/backend/optimizer/util/relnode.c @@ -2991,7 +2991,7 @@ init_grouping_targets(PlannerInfo *root, RelOptInfo *rel, SortGroupClause *sgc; /* Find the matching SortGroupClause */ - sgc = get_sortgroupref_clause(sortgroupref, root->processed_groupClause); + sgc = get_sortgroupref_clause(sortgroupref, root->eager_group_clause); Assert(sgc->tleSortGroupRef <= maxSortGroupRef); /* diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index c48e656ce80..b0b210bd07e 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -501,6 +501,9 @@ struct PlannerInfo /* list of GroupingExprInfos */ List *group_expr_list; + /* the SortGroupClauses the grouping expressions were derived from */ + List *eager_group_clause; + /* list of plain Vars contained in targetlist and havingQual */ List *tlist_vars;