From fecc2760105107e5e1c167a7a23f5263d85f64df Mon Sep 17 00:00:00 2001 From: Hannu Krosing Date: Wed, 16 Sep 2026 13:12:52 +0000 Subject: [PATCH v3 1/5] Refactor set operation group clauses to decouple from targetlist positions Previously, generate_setop_child_grouplist, generate_setop_grouplist, and add_setop_child_rel_equivalences assumed that groupClauses had a strict 1:1 positional correspondence with all non-junk targetlist columns. This patch decouples set operation group clauses from targetlist positions by matching them through SortGroupClause.tleSortGroupRef. This prepares the planner and equivalence class machinery for set operations that group on a subset of columns. --- src/backend/optimizer/path/equivclass.c | 35 +++++++------- src/backend/optimizer/plan/planner.c | 58 ++++++++++++++---------- src/backend/optimizer/prep/prepunion.c | 24 +++------- src/backend/parser/analyze.c | 9 +++- src/backend/rewrite/rewriteSearchCycle.c | 21 ++++++--- 5 files changed, 80 insertions(+), 67 deletions(-) diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c index 553d9d13dc5..0001c64817b 100644 --- a/src/backend/optimizer/path/equivclass.c +++ b/src/backend/optimizer/path/equivclass.c @@ -3139,30 +3139,31 @@ add_setop_child_rel_equivalences(PlannerInfo *root, RelOptInfo *child_rel, List *child_tlist, List *setop_pathkeys) { ListCell *lc; - ListCell *lc2 = list_head(setop_pathkeys); - foreach(lc, child_tlist) + foreach(lc, setop_pathkeys) { - TargetEntry *tle = lfirst_node(TargetEntry, lc); + PathKey *pk = lfirst_node(PathKey, lc); + Index ref = pk->pk_eclass->ec_sortref; + TargetEntry *tle = NULL; + ListCell *lc2; EquivalenceMember *parent_em; - PathKey *pk; - if (tle->resjunk) - continue; + /* Find matching non-resjunk tle in child_tlist */ + foreach(lc2, child_tlist) + { + TargetEntry *cur_tle = lfirst_node(TargetEntry, lc2); + if (cur_tle->ressortgroupref == ref && !cur_tle->resjunk) + { + tle = cur_tle; + break; + } + } - if (lc2 == NULL) - elog(ERROR, "too few pathkeys for set operation"); + if (tle == NULL) + elog(ERROR, "could not find target entry for setop pathkey ref %d", ref); - pk = lfirst_node(PathKey, lc2); parent_em = linitial(pk->pk_eclass->ec_members); - /* - * We can safely pass the parent member as the first member in the - * ec_members list as this is added first in generate_union_paths, - * likewise, the JoinDomain can be that of the initial member of the - * Pathkey's EquivalenceClass. We pass -1 for ec_index since we - * maintain the eclass_indexes for the child_rel after the loop. - */ add_child_eq_member(root, pk->pk_eclass, -1, @@ -3172,8 +3173,6 @@ add_setop_child_rel_equivalences(PlannerInfo *root, RelOptInfo *child_rel, parent_em, exprType((Node *) tle->expr), child_rel->relid); - - lc2 = lnext(setop_pathkeys, lc2); } /* diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index edaa42d713a..c09d356d528 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -8646,49 +8646,59 @@ group_by_has_partkey(RelOptInfo *input_rel, * then we return an empty list. This may leave some TLEs with unreferenced * ressortgroupref markings, but that's harmless. */ +static TargetEntry * +get_nth_nonjunk_tle(List *tlist, int n) +{ + ListCell *lc; + int count = 0; + + foreach(lc, tlist) + { + TargetEntry *tle = lfirst_node(TargetEntry, lc); + + if (!tle->resjunk) + { + count++; + if (count == n) + return tle; + } + } + return NULL; +} + static List * generate_setop_child_grouplist(SetOperationStmt *op, List *targetlist) { List *grouplist = copyObject(op->groupClauses); ListCell *lg; - ListCell *lt; - ListCell *ct; - lg = list_head(grouplist); - ct = list_head(op->colTypes); - foreach(lt, targetlist) + foreach(lg, grouplist) { - TargetEntry *tle = (TargetEntry *) lfirst(lt); - SortGroupClause *sgc; + SortGroupClause *sgc = (SortGroupClause *) lfirst(lg); + Index ref = sgc->tleSortGroupRef; + TargetEntry *tle; Oid coltype; - /* resjunk columns could have sortgrouprefs. Leave these alone */ - if (tle->resjunk) - continue; + /* If tleSortGroupRef is not set, we can't map it. */ + if (ref == 0) + elog(ERROR, "missing tleSortGroupRef in setop groupClause"); - /* - * We expect every non-resjunk target to have a SortGroupClause and - * colTypes. - */ - Assert(lg != NULL); - Assert(ct != NULL); - sgc = (SortGroupClause *) lfirst(lg); - coltype = lfirst_oid(ct); + tle = get_nth_nonjunk_tle(targetlist, ref); + if (tle == NULL) + elog(ERROR, "missing target entry for setop groupClause ref %d", ref); + + /* We also need to get the type from op->colTypes */ + Assert(ref <= list_length(op->colTypes)); + coltype = list_nth_oid(op->colTypes, ref - 1); /* reject if target type isn't the same as the setop target type */ if (coltype != exprType((Node *) tle->expr)) return NIL; - lg = lnext(grouplist, lg); - ct = lnext(op->colTypes, ct); - /* assign a tleSortGroupRef, or reuse the existing one */ sgc->tleSortGroupRef = assignSortGroupRef(tle, targetlist); } - Assert(lg == NULL); - Assert(ct == NULL); - return grouplist; } diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c index 02ac58be211..efbaf134627 100644 --- a/src/backend/optimizer/prep/prepunion.c +++ b/src/backend/optimizer/prep/prepunion.c @@ -1735,28 +1735,18 @@ generate_setop_grouplist(SetOperationStmt *op, List *targetlist) { List *grouplist = copyObject(op->groupClauses); ListCell *lg; - ListCell *lt; - lg = list_head(grouplist); - foreach(lt, targetlist) + foreach(lg, grouplist) { - TargetEntry *tle = (TargetEntry *) lfirst(lt); - SortGroupClause *sgc; + SortGroupClause *sgc = (SortGroupClause *) lfirst(lg); + Index ref = sgc->tleSortGroupRef; + TargetEntry *tle; + Assert(ref > 0 && ref <= list_length(targetlist)); + tle = list_nth(targetlist, ref - 1); Assert(!tle->resjunk); - - /* non-resjunk columns should have sortgroupref = resno */ - Assert(tle->ressortgroupref == tle->resno); - - /* non-resjunk columns should have grouping clauses */ - Assert(lg != NULL); - sgc = (SortGroupClause *) lfirst(lg); - lg = lnext(grouplist, lg); - Assert(sgc->tleSortGroupRef == 0); - - sgc->tleSortGroupRef = tle->ressortgroupref; + Assert(tle->ressortgroupref == ref); } - Assert(lg == NULL); return grouplist; } diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index 2dc81a8d3b2..505f0114bfe 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -2284,6 +2284,7 @@ constructSetOpTargetlist(ParseState *pstate, SetOperationStmt *op, { ListCell *ltl; ListCell *rtl; + int resno = 1; /* * Verify that the two children have the same number of non-junk columns, @@ -2403,13 +2404,15 @@ constructSetOpTargetlist(ParseState *pstate, SetOperationStmt *op, if (op->op != SETOP_UNION || !op->all) { ParseCallbackState pcbstate; + SortGroupClause *grpcl; setup_parser_errposition_callback(&pcbstate, pstate, bestlocation); /* If it's a recursive union, we need to require hashing support. */ - op->groupClauses = lappend(op->groupClauses, - makeSortGroupClauseForSetOp(rescoltype, recursive)); + grpcl = makeSortGroupClauseForSetOp(rescoltype, recursive); + grpcl->tleSortGroupRef = resno; + op->groupClauses = lappend(op->groupClauses, grpcl); cancel_parser_errposition_callback(&pcbstate); } @@ -2434,6 +2437,8 @@ constructSetOpTargetlist(ParseState *pstate, SetOperationStmt *op, false); *targetlist = lappend(*targetlist, restle); } + + resno++; } } diff --git a/src/backend/rewrite/rewriteSearchCycle.c b/src/backend/rewrite/rewriteSearchCycle.c index 75943072817..524a265a4c9 100644 --- a/src/backend/rewrite/rewriteSearchCycle.c +++ b/src/backend/rewrite/rewriteSearchCycle.c @@ -606,8 +606,11 @@ rewriteSearchAndCycle(CommonTableExpr *cte) sos->colTypmods = lappend_int(sos->colTypmods, -1); sos->colCollations = lappend_oid(sos->colCollations, InvalidOid); if (!sos->all) - sos->groupClauses = lappend(sos->groupClauses, - makeSortGroupClauseForSetOp(search_seq_type, true)); + { + SortGroupClause *sgc = makeSortGroupClauseForSetOp(search_seq_type, true); + sgc->tleSortGroupRef = list_length(sos->colTypes); + sos->groupClauses = lappend(sos->groupClauses, sgc); + } } if (cte->cycle_clause) { @@ -615,15 +618,21 @@ rewriteSearchAndCycle(CommonTableExpr *cte) sos->colTypmods = lappend_int(sos->colTypmods, cte->cycle_clause->cycle_mark_typmod); sos->colCollations = lappend_oid(sos->colCollations, cte->cycle_clause->cycle_mark_collation); if (!sos->all) - sos->groupClauses = lappend(sos->groupClauses, - makeSortGroupClauseForSetOp(cte->cycle_clause->cycle_mark_type, true)); + { + SortGroupClause *sgc = makeSortGroupClauseForSetOp(cte->cycle_clause->cycle_mark_type, true); + sgc->tleSortGroupRef = list_length(sos->colTypes); + sos->groupClauses = lappend(sos->groupClauses, sgc); + } sos->colTypes = lappend_oid(sos->colTypes, RECORDARRAYOID); sos->colTypmods = lappend_int(sos->colTypmods, -1); sos->colCollations = lappend_oid(sos->colCollations, InvalidOid); if (!sos->all) - sos->groupClauses = lappend(sos->groupClauses, - makeSortGroupClauseForSetOp(RECORDARRAYOID, true)); + { + SortGroupClause *sgc = makeSortGroupClauseForSetOp(RECORDARRAYOID, true); + sgc->tleSortGroupRef = list_length(sos->colTypes); + sos->groupClauses = lappend(sos->groupClauses, sgc); + } } /* -- 2.56.0.176.g3575e65682-goog