Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit c372671

Browse files
committed
Fix generate_union_paths for non-sortable types.
The previous logic would fail to set groupList when grouping_is_sortable() returned false, which could cause queries to return wrong answers when some columns were not sortable. David Rowley, reviewed by Heikki Linnakangas and Álvaro Herrera. Reported by Hubert "depesz" Lubaczewski. Discussion: http://postgr.es/m/Zktzf926vslR35Fv@depesz.com Discussion: http://postgr.es/m/CAApHDvra=c8_zZT0J-TftByWN2Y+OJfnjNJFg4Dfdi2s+QzmqA@mail.gmail.com
1 parent 12933dc commit c372671

File tree

3 files changed

+31
-9
lines changed

3 files changed

+31
-9
lines changed

src/backend/optimizer/prep/prepunion.c

+12-9
Original file line numberDiff line numberDiff line change
@@ -498,7 +498,7 @@ generate_recursion_path(SetOperationStmt *setOp, PlannerInfo *root,
498498
* interesting_pathkeys: if not NIL, also include paths that suit these
499499
* pathkeys, sorting any unsorted paths as required.
500500
* *pNumGroups: if not NULL, we estimate the number of distinct groups
501-
* in the result, and store it there
501+
* in the result, and store it there.
502502
*/
503503
static void
504504
build_setop_child_paths(PlannerInfo *root, RelOptInfo *rel,
@@ -714,7 +714,7 @@ generate_union_paths(SetOperationStmt *op, PlannerInfo *root,
714714
List *groupList = NIL;
715715
Path *apath;
716716
Path *gpath = NULL;
717-
bool try_sorted;
717+
bool try_sorted = false;
718718
List *union_pathkeys = NIL;
719719

720720
/*
@@ -740,18 +740,21 @@ generate_union_paths(SetOperationStmt *op, PlannerInfo *root,
740740
tlist_list, refnames_tlist);
741741
*pTargetList = tlist;
742742

743-
/* For for UNIONs (not UNION ALL), try sorting, if sorting is possible */
744-
try_sorted = !op->all && grouping_is_sortable(op->groupClauses);
745-
746-
if (try_sorted)
743+
/* For UNIONs (not UNION ALL), try sorting, if sorting is possible */
744+
if (!op->all)
747745
{
748746
/* Identify the grouping semantics */
749747
groupList = generate_setop_grouplist(op, tlist);
750748

751-
/* Determine the pathkeys for sorting by the whole target list */
752-
union_pathkeys = make_pathkeys_for_sortclauses(root, groupList, tlist);
749+
if (grouping_is_sortable(op->groupClauses))
750+
{
751+
try_sorted = true;
752+
/* Determine the pathkeys for sorting by the whole target list */
753+
union_pathkeys = make_pathkeys_for_sortclauses(root, groupList,
754+
tlist);
753755

754-
root->query_pathkeys = union_pathkeys;
756+
root->query_pathkeys = union_pathkeys;
757+
}
755758
}
756759

757760
/*

src/test/regress/expected/union.out

+13
Original file line numberDiff line numberDiff line change
@@ -815,6 +815,19 @@ select x from (values (row(1, 2)), (row(1, 3))) _(x) except select x from (value
815815
(1,3)
816816
(1 row)
817817

818+
-- non-sortable type
819+
-- Ensure we get a HashAggregate plan. Keep enable_hashagg=off to ensure
820+
-- there's no chance of a sort.
821+
explain (costs off) select '123'::xid union select '123'::xid;
822+
QUERY PLAN
823+
---------------------------
824+
HashAggregate
825+
Group Key: ('123'::xid)
826+
-> Append
827+
-> Result
828+
-> Result
829+
(5 rows)
830+
818831
reset enable_hashagg;
819832
--
820833
-- Mixed types

src/test/regress/sql/union.sql

+6
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,12 @@ explain (costs off)
244244
select x from (values (row(1, 2)), (row(1, 3))) _(x) except select x from (values (row(1, 2)), (row(1, 4))) _(x);
245245
select x from (values (row(1, 2)), (row(1, 3))) _(x) except select x from (values (row(1, 2)), (row(1, 4))) _(x);
246246

247+
-- non-sortable type
248+
249+
-- Ensure we get a HashAggregate plan. Keep enable_hashagg=off to ensure
250+
-- there's no chance of a sort.
251+
explain (costs off) select '123'::xid union select '123'::xid;
252+
247253
reset enable_hashagg;
248254

249255
--

0 commit comments

Comments
 (0)