Skip to content

Commit c1ebef3

Browse files
committed
Compare collations before merging UNION operations.
In the dim past we figured it was okay to ignore collations when combining UNION set-operation nodes into a single N-way UNION operation. I believe that was fine at the time, but it stopped being fine when we added nondeterministic collations: the semantics of distinct-ness are affected by those. v17 made it even less fine by allowing per-child sorting operations to be merged via MergeAppend, although I think we accidentally avoided any live bug from that. Add a check that collations match before deciding that two UNION nodes are equivalent. I also failed to resist the temptation to comment plan_union_children() a little better. Back-patch to all supported branches (v13 now), since they all have nondeterministic collations. Discussion: https://postgr.es/m/3605568.1731970579@sss.pgh.pa.us
1 parent 6304632 commit c1ebef3

File tree

1 file changed

+12
-12
lines changed

1 file changed

+12
-12
lines changed

src/backend/optimizer/prep/prepunion.c

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -719,9 +719,9 @@ generate_union_paths(SetOperationStmt *op, PlannerInfo *root,
719719

720720
/*
721721
* If any of my children are identical UNION nodes (same op, all-flag, and
722-
* colTypes) then they can be merged into this node so that we generate
723-
* only one Append/MergeAppend and unique-ification for the lot. Recurse
724-
* to find such nodes.
722+
* colTypes/colCollations) then they can be merged into this node so that
723+
* we generate only one Append/MergeAppend and unique-ification for the
724+
* lot. Recurse to find such nodes.
725725
*/
726726
rellist = plan_union_children(root,
727727
op,
@@ -1193,17 +1193,16 @@ generate_nonunion_paths(SetOperationStmt *op, PlannerInfo *root,
11931193
}
11941194

11951195
/*
1196-
* Pull up children of a UNION node that are identically-propertied UNIONs.
1196+
* Pull up children of a UNION node that are identically-propertied UNIONs,
1197+
* and perform planning of the queries underneath the N-way UNION.
1198+
*
1199+
* The result is a list of RelOptInfos containing Paths for sub-nodes, with
1200+
* one entry for each descendant that is a leaf query or non-identical setop.
1201+
* We also return parallel lists of the childrens' targetlists and
1202+
* is-trivial-tlist flags.
11971203
*
11981204
* NOTE: we can also pull a UNION ALL up into a UNION, since the distinct
11991205
* output rows will be lost anyway.
1200-
*
1201-
* NOTE: currently, we ignore collations while determining if a child has
1202-
* the same properties. This is semantically sound only so long as all
1203-
* collations have the same notion of equality. It is valid from an
1204-
* implementation standpoint because we don't care about the ordering of
1205-
* a UNION child's result: UNION ALL results are always unordered, and
1206-
* generate_union_paths will force a fresh sort if the top level is a UNION.
12071206
*/
12081207
static List *
12091208
plan_union_children(PlannerInfo *root,
@@ -1232,7 +1231,8 @@ plan_union_children(PlannerInfo *root,
12321231

12331232
if (op->op == top_union->op &&
12341233
(op->all == top_union->all || op->all) &&
1235-
equal(op->colTypes, top_union->colTypes))
1234+
equal(op->colTypes, top_union->colTypes) &&
1235+
equal(op->colCollations, top_union->colCollations))
12361236
{
12371237
/* Same UNION, so fold children into parent */
12381238
pending_rels = lcons(op->rarg, pending_rels);

0 commit comments

Comments
 (0)