From 6a078b3fdd5c52030ddc679d02a29b7d05ac5d0b Mon Sep 17 00:00:00 2001 From: David Rowley Date: Tue, 21 May 2024 12:32:05 +1200 Subject: [PATCH v2 2/2] Fix UNION planner bug and add regression test --- src/backend/optimizer/prep/prepunion.c | 17 ++++++++++------- src/test/regress/expected/union.out | 13 +++++++++++++ src/test/regress/sql/union.sql | 6 ++++++ 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c index 30068c27a1..e3ba0d17cf 100644 --- a/src/backend/optimizer/prep/prepunion.c +++ b/src/backend/optimizer/prep/prepunion.c @@ -714,7 +714,7 @@ generate_union_paths(SetOperationStmt *op, PlannerInfo *root, List *groupList = NIL; Path *apath; Path *gpath = NULL; - bool try_sorted; + bool try_sorted = false; List *union_pathkeys = NIL; /* @@ -741,17 +741,20 @@ generate_union_paths(SetOperationStmt *op, PlannerInfo *root, *pTargetList = tlist; /* For for UNIONs (not UNION ALL), try sorting, if sorting is possible */ - try_sorted = !op->all && grouping_is_sortable(op->groupClauses); - - if (try_sorted) + if (!op->all) { /* Identify the grouping semantics */ groupList = generate_setop_grouplist(op, tlist); - /* Determine the pathkeys for sorting by the whole target list */ - union_pathkeys = make_pathkeys_for_sortclauses(root, groupList, tlist); + if (grouping_is_sortable(op->groupClauses)) + { + try_sorted = true; + /* Determine the pathkeys for sorting by the whole target list */ + union_pathkeys = make_pathkeys_for_sortclauses(root, groupList, + tlist); - root->query_pathkeys = union_pathkeys; + root->query_pathkeys = union_pathkeys; + } } /* diff --git a/src/test/regress/expected/union.out b/src/test/regress/expected/union.out index 26b718e903..0fd0e1c38b 100644 --- a/src/test/regress/expected/union.out +++ b/src/test/regress/expected/union.out @@ -815,6 +815,19 @@ select x from (values (row(1, 2)), (row(1, 3))) _(x) except select x from (value (1,3) (1 row) +-- non-sortable type +-- Ensure we get a HashAggregate plan. Keep enable_hashagg=off to ensure +-- there's no chance of a sort. +explain (costs off) select '123'::xid union select '123'::xid; + QUERY PLAN +--------------------------- + HashAggregate + Group Key: ('123'::xid) + -> Append + -> Result + -> Result +(5 rows) + reset enable_hashagg; -- -- Mixed types diff --git a/src/test/regress/sql/union.sql b/src/test/regress/sql/union.sql index 8afc580c63..f8826514e4 100644 --- a/src/test/regress/sql/union.sql +++ b/src/test/regress/sql/union.sql @@ -244,6 +244,12 @@ explain (costs off) select x from (values (row(1, 2)), (row(1, 3))) _(x) except select x from (values (row(1, 2)), (row(1, 4))) _(x); select x from (values (row(1, 2)), (row(1, 3))) _(x) except select x from (values (row(1, 2)), (row(1, 4))) _(x); +-- non-sortable type + +-- Ensure we get a HashAggregate plan. Keep enable_hashagg=off to ensure +-- there's no chance of a sort. +explain (costs off) select '123'::xid union select '123'::xid; + reset enable_hashagg; -- -- 2.34.1