Skip to content

Commit a2db7b7

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 131ec00 commit a2db7b7

File tree

5 files changed

+139
-9
lines changed

5 files changed

+139
-9
lines changed

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

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1031,10 +1031,15 @@ get_cheapest_parameterized_child_path(PlannerInfo *root, RelOptInfo *rel,
10311031
* accumulate_append_subpath
10321032
* Add a subpath to the list being built for an Append or MergeAppend
10331033
*
1034-
* It's possible that the child is itself an Append path, in which case
1035-
* we can "cut out the middleman" and just add its child paths to our
1036-
* own list. (We don't try to do this earlier because we need to
1037-
* apply both levels of transformation to the quals.)
1034+
* It's possible that the child is itself an Append or MergeAppend path, in
1035+
* which case we can "cut out the middleman" and just add its child paths to
1036+
* our own list. (We don't try to do this earlier because we need to apply
1037+
* both levels of transformation to the quals.)
1038+
*
1039+
* Note that if we omit a child MergeAppend in this way, we are effectively
1040+
* omitting a sort step, which seems fine: if the parent is to be an Append,
1041+
* its result would be unsorted anyway, while if the parent is to be a
1042+
* MergeAppend, there's no point in a separate sort on a child.
10381043
*/
10391044
static List *
10401045
accumulate_append_subpath(List *subpaths, Path *path)
@@ -1046,6 +1051,13 @@ accumulate_append_subpath(List *subpaths, Path *path)
10461051
/* list_copy is important here to avoid sharing list substructure */
10471052
return list_concat(subpaths, list_copy(apath->subpaths));
10481053
}
1054+
else if (IsA(path, MergeAppendPath))
1055+
{
1056+
MergeAppendPath *mpath = (MergeAppendPath *) path;
1057+
1058+
/* list_copy is important here to avoid sharing list substructure */
1059+
return list_concat(subpaths, list_copy(mpath->subpaths));
1060+
}
10491061
else
10501062
return lappend(subpaths, path);
10511063
}

‎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
@@ -753,7 +753,7 @@ create_merge_append_plan(PlannerInfo *root, MergeAppendPath *best_path)
753753

754754
/* Compute sort column info, and adjust MergeAppend's tlist as needed */
755755
(void) prepare_sort_from_pathkeys(root, plan, pathkeys,
756-
NULL,
756+
best_path->path.parent->relids,
757757
NULL,
758758
true,
759759
&node->numCols,

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

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

504+
--
505+
-- Test that ORDER BY for UNION ALL can be pushed down to inheritance
506+
-- children.
507+
--
508+
CREATE TEMP TABLE t1c (b text, a text);
509+
ALTER TABLE t1c INHERIT t1;
510+
CREATE TEMP TABLE t2c (primary key (ab)) INHERITS (t2);
511+
INSERT INTO t1c VALUES ('v', 'w'), ('c', 'd'), ('m', 'n'), ('e', 'f');
512+
INSERT INTO t2c VALUES ('vw'), ('cd'), ('mn'), ('ef');
513+
CREATE INDEX t1c_ab_idx on t1c ((a || b));
514+
set enable_seqscan = on;
515+
set enable_indexonlyscan = off;
516+
explain (costs off)
517+
SELECT * FROM
518+
(SELECT a || b AS ab FROM t1
519+
UNION ALL
520+
SELECT ab FROM t2) t
521+
ORDER BY 1 LIMIT 8;
522+
QUERY PLAN
523+
------------------------------------------------
524+
Limit
525+
-> Merge Append
526+
Sort Key: ((t1.a || t1.b))
527+
-> Index Scan using t1_ab_idx on t1
528+
-> Index Scan using t1c_ab_idx on t1c
529+
-> Index Scan using t2_pkey on t2
530+
-> Index Scan using t2c_pkey on t2c
531+
(7 rows)
532+
533+
SELECT * FROM
534+
(SELECT a || b AS ab FROM t1
535+
UNION ALL
536+
SELECT ab FROM t2) t
537+
ORDER BY 1 LIMIT 8;
538+
ab
539+
----
540+
ab
541+
ab
542+
cd
543+
dc
544+
ef
545+
fe
546+
mn
547+
nm
548+
(8 rows)
549+
504550
reset enable_seqscan;
505551
reset enable_indexscan;
506552
reset enable_bitmapscan;
553+
-- This simpler variant of the above test has been observed to fail differently
554+
create table events (event_id int primary key);
555+
create table other_events (event_id int primary key);
556+
create table events_child () inherits (events);
557+
explain (costs off)
558+
select event_id
559+
from (select event_id from events
560+
union all
561+
select event_id from other_events) ss
562+
order by event_id;
563+
QUERY PLAN
564+
----------------------------------------------------------
565+
Merge Append
566+
Sort Key: events.event_id
567+
-> Index Scan using events_pkey on events
568+
-> Sort
569+
Sort Key: events_child.event_id
570+
-> Seq Scan on events_child
571+
-> Index Scan using other_events_pkey on other_events
572+
(7 rows)
573+
574+
drop table events_child, events, other_events;
575+
reset enable_indexonlyscan;
507576
-- Test constraint exclusion of UNION ALL subqueries
508577
explain (costs off)
509578
SELECT * FROM

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

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,10 +198,55 @@ explain (costs off)
198198
SELECT * FROM t2) t
199199
WHERE ab = 'ab';
200200

201+
--
202+
-- Test that ORDER BY for UNION ALL can be pushed down to inheritance
203+
-- children.
204+
--
205+
206+
CREATE TEMP TABLE t1c (b text, a text);
207+
ALTER TABLE t1c INHERIT t1;
208+
CREATE TEMP TABLE t2c (primary key (ab)) INHERITS (t2);
209+
INSERT INTO t1c VALUES ('v', 'w'), ('c', 'd'), ('m', 'n'), ('e', 'f');
210+
INSERT INTO t2c VALUES ('vw'), ('cd'), ('mn'), ('ef');
211+
CREATE INDEX t1c_ab_idx on t1c ((a || b));
212+
213+
set enable_seqscan = on;
214+
set enable_indexonlyscan = off;
215+
216+
explain (costs off)
217+
SELECT * FROM
218+
(SELECT a || b AS ab FROM t1
219+
UNION ALL
220+
SELECT ab FROM t2) t
221+
ORDER BY 1 LIMIT 8;
222+
223+
SELECT * FROM
224+
(SELECT a || b AS ab FROM t1
225+
UNION ALL
226+
SELECT ab FROM t2) t
227+
ORDER BY 1 LIMIT 8;
228+
201229
reset enable_seqscan;
202230
reset enable_indexscan;
203231
reset enable_bitmapscan;
204232

233+
-- This simpler variant of the above test has been observed to fail differently
234+
235+
create table events (event_id int primary key);
236+
create table other_events (event_id int primary key);
237+
create table events_child () inherits (events);
238+
239+
explain (costs off)
240+
select event_id
241+
from (select event_id from events
242+
union all
243+
select event_id from other_events) ss
244+
order by event_id;
245+
246+
drop table events_child, events, other_events;
247+
248+
reset enable_indexonlyscan;
249+
205250
-- Test constraint exclusion of UNION ALL subqueries
206251
explain (costs off)
207252
SELECT * FROM

0 commit comments

Comments
 (0)