Skip to content

Commit 0cf1668

Browse files
tglsfdctglsfdc
committed
Back-patch "Fix EquivalenceClass processing for nested append relations".
When we committed a87c729, we somehow failed to notice that it didn't merely improve plan quality for expression indexes; there were very closely related cases that failed outright with "could not find pathkey item to sort". The failing cases seem to be those where the planner was already capable of selecting a MergeAppend plan, and there was inheritance involved: the lack of appropriate eclass child members would prevent prepare_sort_from_pathkeys() from succeeding on the MergeAppend's child plan nodes for inheritance child tables. Accordingly, back-patch into 9.1 through 9.3, along with an extra regression test case covering the problem. Per trouble report from Michael Glaesemann.
1 parent 4ee4594 commit 0cf1668

File tree

5 files changed

+76
-9
lines changed

5 files changed

+76
-9
lines changed

‎src/backend/optimizer/path/allpaths.c

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1011,10 +1011,15 @@ get_cheapest_parameterized_child_path(PlannerInfo *root, RelOptInfo *rel,
10111011
* accumulate_append_subpath
10121012
* Add a subpath to the list being built for an Append or MergeAppend
10131013
*
1014-
* It's possible that the child is itself an Append path, in which case
1015-
* we can "cut out the middleman" and just add its child paths to our
1016-
* own list. (We don't try to do this earlier because we need to
1017-
* apply both levels of transformation to the quals.)
1014+
* It's possible that the child is itself an Append or MergeAppend path, in
1015+
* which case we can "cut out the middleman" and just add its child paths to
1016+
* our own list. (We don't try to do this earlier because we need to apply
1017+
* both levels of transformation to the quals.)
1018+
*
1019+
* Note that if we omit a child MergeAppend in this way, we are effectively
1020+
* omitting a sort step, which seems fine: if the parent is to be an Append,
1021+
* its result would be unsorted anyway, while if the parent is to be a
1022+
* MergeAppend, there's no point in a separate sort on a child.
10181023
*/
10191024
static List *
10201025
accumulate_append_subpath(List *subpaths, Path *path)
@@ -1026,6 +1031,13 @@ accumulate_append_subpath(List *subpaths, Path *path)
10261031
/* list_copy is important here to avoid sharing list substructure */
10271032
return list_concat(subpaths, list_copy(apath->subpaths));
10281033
}
1034+
else if (IsA(path, MergeAppendPath))
1035+
{
1036+
MergeAppendPath *mpath = (MergeAppendPath *) path;
1037+
1038+
/* list_copy is important here to avoid sharing list substructure */
1039+
return list_concat(subpaths, list_copy(mpath->subpaths));
1040+
}
10291041
else
10301042
return lappend(subpaths, path);
10311043
}

‎src/backend/optimizer/path/equivclass.c

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1937,16 +1937,20 @@ add_child_rel_equivalences(PlannerInfo *root,
19371937
if (cur_ec->ec_has_volatile)
19381938
continue;
19391939

1940-
/* No point in searching if parent rel not mentioned in eclass */
1941-
if (!bms_is_subset(parent_rel->relids, cur_ec->ec_relids))
1940+
/*
1941+
* No point in searching if parent rel not mentioned in eclass; but
1942+
* we can't tell that for sure if parent rel is itself a child.
1943+
*/
1944+
if (parent_rel->reloptkind == RELOPT_BASEREL &&
1945+
!bms_is_subset(parent_rel->relids, cur_ec->ec_relids))
19421946
continue;
19431947

19441948
foreach(lc2, cur_ec->ec_members)
19451949
{
19461950
EquivalenceMember *cur_em = (EquivalenceMember *) lfirst(lc2);
19471951

1948-
if (cur_em->em_is_const || cur_em->em_is_child)
1949-
continue; /* ignore consts and children here */
1952+
if (cur_em->em_is_const)
1953+
continue; /* ignore consts here */
19501954

19511955
/* Does it reference parent_rel? */
19521956
if (bms_overlap(cur_em->em_relids, parent_rel->relids))

‎src/backend/optimizer/plan/createplan.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -731,7 +731,7 @@ create_merge_append_plan(PlannerInfo *root, MergeAppendPath *best_path)
731731

732732
/* Compute sort column info, and adjust MergeAppend's tlist as needed */
733733
(void) prepare_sort_from_pathkeys(root, plan, pathkeys,
734-
NULL,
734+
best_path->path.parent->relids,
735735
NULL,
736736
true,
737737
&node->numCols,

‎src/test/regress/expected/union.out

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -500,9 +500,39 @@ explain (costs off)
500500
Index Cond: (ab = 'ab'::text)
501501
(6 rows)
502502

503+
--
504+
-- Test that ORDER BY for UNION ALL can be pushed down to inheritance
505+
-- children.
506+
--
503507
reset enable_seqscan;
504508
reset enable_indexscan;
505509
reset enable_bitmapscan;
510+
set enable_indexonlyscan = off;
511+
create table events (event_id int primary key);
512+
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "events_pkey" for table "events"
513+
create table other_events (event_id int primary key);
514+
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "other_events_pkey" for table "other_events"
515+
create table events_child () inherits (events);
516+
explain (costs off)
517+
select event_id
518+
from (select event_id from events
519+
union all
520+
select event_id from other_events) ss
521+
order by event_id;
522+
QUERY PLAN
523+
----------------------------------------------------------------
524+
Result
525+
-> Merge Append
526+
Sort Key: public.events.event_id
527+
-> Index Scan using events_pkey on events
528+
-> Sort
529+
Sort Key: public.events.event_id
530+
-> Seq Scan on events_child events
531+
-> Index Scan using other_events_pkey on other_events
532+
(8 rows)
533+
534+
drop table events_child, events, other_events;
535+
reset enable_indexonlyscan;
506536
-- Test constraint exclusion of UNION ALL subqueries
507537
explain (costs off)
508538
SELECT * FROM

‎src/test/regress/sql/union.sql

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,9 +196,30 @@ explain (costs off)
196196
SELECT * FROM t2) t
197197
WHERE ab = 'ab';
198198

199+
--
200+
-- Test that ORDER BY for UNION ALL can be pushed down to inheritance
201+
-- children.
202+
--
203+
199204
reset enable_seqscan;
200205
reset enable_indexscan;
201206
reset enable_bitmapscan;
207+
set enable_indexonlyscan = off;
208+
209+
create table events (event_id int primary key);
210+
create table other_events (event_id int primary key);
211+
create table events_child () inherits (events);
212+
213+
explain (costs off)
214+
select event_id
215+
from (select event_id from events
216+
union all
217+
select event_id from other_events) ss
218+
order by event_id;
219+
220+
drop table events_child, events, other_events;
221+
222+
reset enable_indexonlyscan;
202223

203224
-- Test constraint exclusion of UNION ALL subqueries
204225
explain (costs off)

0 commit comments

Comments
 (0)