From 0a7fe768d8b06bccfeff404ddc38c21a7cbaeda8 Mon Sep 17 00:00:00 2001 From: zengxx Date: Thu, 24 Sep 2026 08:51:28 +0800 Subject: [PATCH v3 1/2] Move unique-index GROUP BY matching into indxpath Extract the unique-index/GROUP BY column matcher used by remove_useless_groupby_columns() into indxpath.c. The new relation_removable_groupby_columns() returns the heap attribute numbers that a suitable immediate unique index makes redundant. Keep the matcher data structures private to indxpath.c. This also clarifies that notnullattnums contains heap attribute numbers. Shared matching lets the NOT NULL, NULLS NOT DISTINCT, opfamily, and collation checks have one implementation. This is a refactoring step and does not change planner behavior. --- src/backend/optimizer/path/indxpath.c | 182 ++++++++++++++++++++++++- src/backend/optimizer/plan/initsplan.c | 164 ++++------------------ src/include/nodes/pathnodes.h | 8 +- src/include/optimizer/paths.h | 3 + 4 files changed, 214 insertions(+), 143 deletions(-) diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index 3f5d4fa318..ddac5594a5 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -74,7 +74,6 @@ typedef struct int indexcol; /* index column we want to match to */ } ec_member_matches_arg; - static void consider_index_join_clauses(PlannerInfo *root, RelOptInfo *rel, IndexOptInfo *index, IndexClauseSet *rclauseset, @@ -4285,6 +4284,187 @@ relation_has_unique_index_for(PlannerInfo *root, RelOptInfo *rel, return false; } +/* + * The internal shape of one simple GROUP BY Var used when matching a unique + * index against GROUP BY keys. This is not exposed outside indxpath.c. + */ +typedef struct GroupByColInfo +{ + AttrNumber attno; /* var->varattno */ + List *eq_opfamilies; /* mergejoin opfamilies of sgc->eqop */ + Oid coll; /* var->varcollid */ +} GroupByColInfo; + +/* + * build_groupby_col_infos + * Collect the simple Vars from groupClause that belong to rel. + * + * Other GROUP BY items may refine the grouping, but they cannot help an index + * prove uniqueness of the underlying relation. If groupbyattnos isn't NULL, + * it is set to the heap attribute numbers of the collected Vars. + */ +static List * +build_groupby_col_infos(RelOptInfo *rel, List *groupClause, List *targetList, + Bitmapset **groupbyattnos) +{ + List *infos = NIL; + ListCell *lc; + + if (groupbyattnos) + *groupbyattnos = NULL; + + foreach(lc, groupClause) + { + SortGroupClause *sgc = lfirst_node(SortGroupClause, lc); + TargetEntry *tle = get_sortgroupclause_tle(sgc, targetList); + Var *var; + GroupByColInfo *info; + + if (tle == NULL) + continue; + + if (!IsA(tle->expr, Var)) + continue; + + var = (Var *) tle->expr; + if (var->varlevelsup != 0 || var->varattno <= 0 || + var->varno != rel->relid) + continue; + + info = palloc_object(GroupByColInfo); + info->attno = var->varattno; + info->eq_opfamilies = get_mergejoin_opfamilies(sgc->eqop); + info->coll = var->varcollid; + infos = lappend(infos, info); + + if (groupbyattnos) + *groupbyattnos = bms_add_member(*groupbyattnos, + var->varattno - + FirstLowInvalidHeapAttributeNumber); + } + + return infos; +} + +/* + * unique_index_keys_match_groupby_cols + * Test whether an immediate unique index proves uniqueness under the + * equality semantics of the given GROUP BY columns. + * + * For each index key column, there must be a GROUP BY Var on the same column + * whose mergejoin opfamilies include the index opfamily and whose collation + * agrees on equality. A NULLS DISTINCT index additionally requires every key + * column to be NOT NULL. + * + * If index_attnos isn't NULL, it is set to the heap attribute numbers of the + * matched index key columns. On a false return it may describe a partial + * match; callers must ignore it unless the function returns true. + */ +static bool +unique_index_keys_match_groupby_cols(IndexOptInfo *index, RelOptInfo *rel, + List *groupbycols, + Bitmapset **index_attnos) +{ + if (index_attnos) + *index_attnos = NULL; + + /* + * Only an immediate, unconditional unique index proves that the input is + * unique. Expression and partial indexes cannot prove whole-relation + * uniqueness. Skip hypothetical indexes because they do not prove a + * property of the physical relation. + */ + if (!index->unique || !index->immediate || index->indpred != NIL || + index->indexprs != NIL || index->hypothetical) + return false; + + for (int i = 0; i < index->nkeycolumns; i++) + { + AttrNumber indkey = index->indexkeys[i]; + ListCell *lc; + + if (indkey <= 0 || + (!index->nullsnotdistinct && + !bms_is_member(indkey, rel->notnullattnums))) + return false; + + foreach(lc, groupbycols) + { + GroupByColInfo *info = (GroupByColInfo *) lfirst(lc); + + if (info->attno == indkey && + list_member_oid(info->eq_opfamilies, index->opfamily[i]) && + collations_agree_on_equality(index->indexcollations[i], + info->coll)) + break; + } + if (lc == NULL) + return false; + + if (index_attnos) + *index_attnos = bms_add_member(*index_attnos, + indkey - + FirstLowInvalidHeapAttributeNumber); + } + + return true; +} + +/* + * relation_removable_groupby_columns + * Return the GROUP BY Vars made redundant by a unique index. + * + * The returned bitmap contains heap attribute numbers from rel. A unique + * index key must be a proper subset of rel's simple GROUP BY Vars: equal keys + * cannot remove any columns. The caller owns the returned bitmap. + */ +Bitmapset * +relation_removable_groupby_columns(RelOptInfo *rel, List *groupClause, + List *targetList) +{ + List *groupbycols; + Bitmapset *groupbyattnos; + Bitmapset *best_keycolumns = NULL; + IndexOptInfo *best_index = NULL; + ListCell *lc; + + groupbycols = build_groupby_col_infos(rel, groupClause, targetList, + &groupbyattnos); + + if (list_length(groupbycols) < 2) + return NULL; + + foreach(lc, rel->indexlist) + { + IndexOptInfo *index = lfirst_node(IndexOptInfo, lc); + Bitmapset *index_attnos; + + if (!unique_index_keys_match_groupby_cols(index, rel, groupbycols, + &index_attnos)) + continue; + + /* + * Only a proper subset of the GROUP BY keys can identify columns to + * remove. Prefer the index with the fewest key columns, which + * removes the largest number of GROUP BY columns. + */ + if (bms_subset_compare(index_attnos, groupbyattnos) != BMS_SUBSET1) + continue; + + if (best_index == NULL || + index->nkeycolumns < best_index->nkeycolumns) + { + best_index = index; + best_keycolumns = index_attnos; + } + } + + if (best_index == NULL) + return NULL; + + return bms_difference(groupbyattnos, best_keycolumns); +} + /* * indexcol_is_bool_constant_for_query * diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c index 8893e37c8f..c5db9d1d70 100644 --- a/src/backend/optimizer/plan/initsplan.c +++ b/src/backend/optimizer/plan/initsplan.c @@ -81,19 +81,6 @@ typedef struct JoinTreeItem * lateral references */ } JoinTreeItem; -/* - * Compatibility info for one GROUP BY item, precomputed for use by - * remove_useless_groupby_columns() when matching unique-index columns against - * GROUP BY items. - */ -typedef struct GroupByColInfo -{ - AttrNumber attno; /* var->varattno */ - List *eq_opfamilies; /* mergejoin opfamilies of sgc->eqop */ - Oid coll; /* var->varcollid */ -} GroupByColInfo; - - static bool is_partial_agg_memory_risky(PlannerInfo *root); static void create_agg_clause_infos(PlannerInfo *root); static void create_grouping_expr_infos(PlannerInfo *root); @@ -372,7 +359,6 @@ remove_useless_groupby_columns(PlannerInfo *root) { Query *parse = root->parse; Bitmapset **groupbyattnos; - List **groupbycols; Bitmapset **surplusvars; bool tryremove = false; ListCell *lc; @@ -389,20 +375,14 @@ remove_useless_groupby_columns(PlannerInfo *root) /* * Scan the GROUP BY clause to find GROUP BY items that are simple Vars. * Fill groupbyattnos[k] with a bitmapset of the column attnos of RTE k - * that are GROUP BY items, and groupbycols[k] with a parallel list of - * GroupByColInfo records. We need the latter so that, when checking a - * unique index against this rel's GROUP BY items, we can verify that the - * index's notion of equality agrees with at least one GROUP BY item per - * index column. + * that are GROUP BY items. */ groupbyattnos = palloc0_array(Bitmapset *, list_length(parse->rtable) + 1); - groupbycols = palloc0_array(List *, list_length(parse->rtable) + 1); foreach(lc, root->processed_groupClause) { SortGroupClause *sgc = lfirst_node(SortGroupClause, lc); TargetEntry *tle = get_sortgroupclause_tle(sgc, parse->targetList); - Var *var = (Var *) tle->expr; - GroupByColInfo *info; + Var *var; /* * Ignore non-Vars and Vars from other query levels. @@ -412,8 +392,11 @@ remove_useless_groupby_columns(PlannerInfo *root) * BY items. But it's not clear that such cases occur often enough to * be worth troubling over. */ - if (!IsA(var, Var) || - var->varlevelsup > 0) + if (tle == NULL || !IsA(tle->expr, Var)) + continue; + + var = (Var *) tle->expr; + if (var->varlevelsup > 0) continue; /* OK, remember we have this Var */ @@ -427,18 +410,13 @@ remove_useless_groupby_columns(PlannerInfo *root) */ tryremove |= !bms_is_empty(groupbyattnos[relid]); groupbyattnos[relid] = bms_add_member(groupbyattnos[relid], - var->varattno - FirstLowInvalidHeapAttributeNumber); - - info = palloc_object(GroupByColInfo); - info->attno = var->varattno; - info->eq_opfamilies = get_mergejoin_opfamilies(sgc->eqop); - info->coll = var->varcollid; - groupbycols[relid] = lappend(groupbycols[relid], info); + var->varattno - + FirstLowInvalidHeapAttributeNumber); } /* - * No Vars or didn't find multiple Vars for any relation in the GROUP BY? - * If so, nothing can be removed, so don't waste more effort trying. + * No relation has multiple Vars in GROUP BY? If so, nothing can be + * removed, so don't waste more effort trying. */ if (!tryremove) return; @@ -456,8 +434,7 @@ remove_useless_groupby_columns(PlannerInfo *root) RangeTblEntry *rte = lfirst_node(RangeTblEntry, lc); RelOptInfo *rel; Bitmapset *relattnos; - Bitmapset *best_keycolumns = NULL; - int32 best_nkeycolumns = PG_INT32_MAX; + Bitmapset *best_keycolumns; relid++; @@ -479,105 +456,9 @@ remove_useless_groupby_columns(PlannerInfo *root) continue; rel = root->simple_rel_array[relid]; - - /* - * Now check each index for this relation to see if there are any with - * columns which are a proper subset of the grouping columns for this - * relation. - */ - foreach_node(IndexOptInfo, index, rel->indexlist) - { - Bitmapset *ind_attnos; - bool index_check_ok; - - /* - * Skip any non-unique and deferrable indexes. Predicate indexes - * have not been checked yet, so we must skip those too as the - * predOK check that's done later might fail. - */ - if (!index->unique || !index->immediate || index->indpred != NIL) - continue; - - /* For simplicity, we currently don't support expression indexes */ - if (index->indexprs != NIL) - continue; - - ind_attnos = NULL; - index_check_ok = true; - for (int i = 0; i < index->nkeycolumns; i++) - { - AttrNumber indkey_attno = index->indexkeys[i]; - Oid indkey_opfamily = index->opfamily[i]; - Oid indkey_coll = index->indexcollations[i]; - ListCell *lc2; - - /* - * We must insist that the index columns are all defined NOT - * NULL otherwise duplicate NULLs could exist. However, we - * can relax this check when the index is defined with NULLS - * NOT DISTINCT as there can only be 1 NULL row, therefore - * functional dependency on the unique columns is maintained, - * despite the NULL. - */ - if (!index->nullsnotdistinct && - !bms_is_member(indkey_attno, rel->notnullattnums)) - { - index_check_ok = false; - break; - } - - /* - * The index proves uniqueness only under its own opfamily and - * collation. Require some GROUP BY item on this column to - * use a compatible eqop and collation, the same check - * relation_has_unique_index_for() applies to join clauses. - */ - foreach(lc2, groupbycols[relid]) - { - GroupByColInfo *info = (GroupByColInfo *) lfirst(lc2); - - if (info->attno != indkey_attno) - continue; - if (list_member_oid(info->eq_opfamilies, indkey_opfamily) && - collations_agree_on_equality(indkey_coll, info->coll)) - break; - } - if (lc2 == NULL) - { - index_check_ok = false; - break; - } - - ind_attnos = - bms_add_member(ind_attnos, - indkey_attno - - FirstLowInvalidHeapAttributeNumber); - } - - if (!index_check_ok) - continue; - - /* - * Skip any indexes where the indexed columns aren't a proper - * subset of the GROUP BY. - */ - if (bms_subset_compare(ind_attnos, relattnos) != BMS_SUBSET1) - continue; - - /* - * Record the attribute numbers from the index with the fewest - * columns. This allows the largest number of columns to be - * removed from the GROUP BY clause. In the future, we may wish - * to consider using the narrowest set of columns and looking at - * pg_statistic.stawidth as it might be better to use an index - * with, say two INT4s, rather than, say, one long varlena column. - */ - if (index->nkeycolumns < best_nkeycolumns) - { - best_keycolumns = ind_attnos; - best_nkeycolumns = index->nkeycolumns; - } - } + best_keycolumns = relation_removable_groupby_columns(rel, + root->processed_groupClause, + parse->targetList); /* Did we find a suitable index? */ if (!bms_is_empty(best_keycolumns)) @@ -590,7 +471,7 @@ remove_useless_groupby_columns(PlannerInfo *root) surplusvars = palloc0_array(Bitmapset *, list_length(parse->rtable) + 1); /* Remember the attnos of the removable columns */ - surplusvars[relid] = bms_difference(relattnos, best_keycolumns); + surplusvars[relid] = best_keycolumns; } } @@ -607,14 +488,21 @@ remove_useless_groupby_columns(PlannerInfo *root) { SortGroupClause *sgc = lfirst_node(SortGroupClause, lc); TargetEntry *tle = get_sortgroupclause_tle(sgc, parse->targetList); - Var *var = (Var *) tle->expr; + Var *var; + + if (tle == NULL || !IsA(tle->expr, Var)) + { + new_groupby = lappend(new_groupby, sgc); + continue; + } + + var = (Var *) tle->expr; /* * New list must include non-Vars, outer Vars, and anything not * marked as surplus. */ - if (!IsA(var, Var) || - var->varlevelsup > 0 || + if (var->varlevelsup > 0 || !bms_is_member(var->varattno - FirstLowInvalidHeapAttributeNumber, surplusvars[var->varno])) new_groupby = lappend(new_groupby, sgc); diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index 1c6d1fe3d0..02e6cc29ea 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -827,9 +827,9 @@ typedef struct PartitionSchemeData *PartitionScheme; * the attribute is needed as part of final targetlist * attr_widths - cache space for per-attribute width estimates; * zero means not computed yet - * notnullattnums - zero-based set containing attnums of NOT NULL - * columns (not populated for rels corresponding to - * non-partitioned inh==true RTEs) + * notnullattnums - set of heap attnums for columns with valid NOT + * NULL constraints (not populated for rels + * corresponding to non-partitioned inh==true RTEs) * nulling_relids - relids of outer joins that can null this rel * lateral_vars - lateral cross-references of rel, if any (list of * Vars and PlaceHolderVars) @@ -1079,7 +1079,7 @@ typedef struct RelOptInfo Relids *attr_needed pg_node_attr(read_write_ignore); /* array indexed [min_attr .. max_attr] */ int32 *attr_widths pg_node_attr(read_write_ignore); - /* zero-based set containing attnums of NOT NULL columns */ + /* set of heap attnums for columns with valid NOT NULL constraints */ Bitmapset *notnullattnums; /* relids of outer joins that can null this baserel */ Relids nulling_relids; diff --git a/src/include/optimizer/paths.h b/src/include/optimizer/paths.h index d3853d1c07..d09f57eb0a 100644 --- a/src/include/optimizer/paths.h +++ b/src/include/optimizer/paths.h @@ -83,6 +83,9 @@ extern void create_index_paths(PlannerInfo *root, RelOptInfo *rel); extern bool relation_has_unique_index_for(PlannerInfo *root, RelOptInfo *rel, List *restrictlist, List **extra_clauses); +extern Bitmapset *relation_removable_groupby_columns(RelOptInfo *rel, + List *groupClause, + List *targetList); extern bool indexcol_is_bool_constant_for_query(PlannerInfo *root, IndexOptInfo *index, int indexcol); -- 2.43.0