On Sat, Jan 4, 2025 at 2:58 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I'm thinking at this point that the bug boils down to trying to
> push pathkeys into the subplan without regard for the type
> conversion that occurs at the set-operation level. Once we've
> done that, the lower level will generate this incorrectly-sorted
> Path, and that probably wins the add_path tournament on the basis
> of being better sorted and fuzzily the same cost as the unsorted
> path. So that's how come that path gets chosen even though
> the sort is useless in context.
I've reached the same conclusion. I'm thinking about whether we
should refrain from pushing pathkeys into the subplan when type
conversion occurs at the set-operation level. Maybe we can do this
check in generate_setop_child_grouplist, like below.
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -8091,6 +8091,9 @@ generate_setop_child_grouplist(SetOperationStmt
*op, List *targetlist)
{
TargetEntry *tle = (TargetEntry *) lfirst(lt);
SortGroupClause *sgc;
+ Oid opfamily,
+ opcintype;
+ int16 strategy;
/* resjunk columns could have sortgrouprefs. Leave these alone */
if (tle->resjunk)
@@ -8101,6 +8104,18 @@ generate_setop_child_grouplist(SetOperationStmt
*op, List *targetlist)
sgc = (SortGroupClause *) lfirst(lg);
lg = lnext(grouplist, lg);
+ if (!OidIsValid(sgc->sortop))
+ return NIL;
+
+ /* Find the operator in pg_amop --- failure shouldn't happen */
+ if (!get_ordering_op_properties(sgc->sortop,
+ &opfamily, &opcintype, &strategy))
+ elog(ERROR, "operator %u is not a valid ordering operator",
+ sgc->sortop);
+
+ if (exprType((Node *) tle->expr) != opcintype)
+ return NIL;
+
/* assign a tleSortGroupRef, or reuse the existing one */
sgc->tleSortGroupRef = assignSortGroupRef(tle, targetlist);
}
Thanks
Richard
