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

Commit a87c729

Browse files
committed
Fix EquivalenceClass processing for nested append relations.
The original coding of EquivalenceClasses didn't foresee that appendrel child relations might themselves be appendrels; but this is possible for example when a UNION ALL subquery scans a table with inheritance children. The oversight led to failure to optimize ordering-related issues very well for the grandchild tables. After some false starts involving explicitly flattening the appendrel representation, we found that this could be fixed easily by removing a few implicit assumptions about appendrel parent rels not being children themselves. Kyotaro Horiguchi and Tom Lane, reviewed by Noah Misch
1 parent b777be0 commit a87c729

File tree

5 files changed

+101
-9
lines changed

5 files changed

+101
-9
lines changed

src/backend/optimizer/path/allpaths.c

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1021,10 +1021,15 @@ get_cheapest_parameterized_child_path(PlannerInfo *root, RelOptInfo *rel,
10211021
* accumulate_append_subpath
10221022
* Add a subpath to the list being built for an Append or MergeAppend
10231023
*
1024-
* It's possible that the child is itself an Append path, in which case
1025-
* we can "cut out the middleman" and just add its child paths to our
1026-
* own list. (We don't try to do this earlier because we need to
1027-
* apply both levels of transformation to the quals.)
1024+
* It's possible that the child is itself an Append or MergeAppend path, in
1025+
* which case we can "cut out the middleman" and just add its child paths to
1026+
* our own list. (We don't try to do this earlier because we need to apply
1027+
* both levels of transformation to the quals.)
1028+
*
1029+
* Note that if we omit a child MergeAppend in this way, we are effectively
1030+
* omitting a sort step, which seems fine: if the parent is to be an Append,
1031+
* its result would be unsorted anyway, while if the parent is to be a
1032+
* MergeAppend, there's no point in a separate sort on a child.
10281033
*/
10291034
static List *
10301035
accumulate_append_subpath(List *subpaths, Path *path)
@@ -1036,6 +1041,13 @@ accumulate_append_subpath(List *subpaths, Path *path)
10361041
/* list_copy is important here to avoid sharing list substructure */
10371042
return list_concat(subpaths, list_copy(apath->subpaths));
10381043
}
1044+
else if (IsA(path, MergeAppendPath))
1045+
{
1046+
MergeAppendPath *mpath = (MergeAppendPath *) path;
1047+
1048+
/* list_copy is important here to avoid sharing list substructure */
1049+
return list_concat(subpaths, list_copy(mpath->subpaths));
1050+
}
10391051
else
10401052
return lappend(subpaths, path);
10411053
}

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
@@ -751,7 +751,7 @@ create_merge_append_plan(PlannerInfo *root, MergeAppendPath *best_path)
751751

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

src/test/regress/expected/union.out

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -502,9 +502,56 @@ explain (costs off)
502502
Index Cond: (ab = 'ab'::text)
503503
(7 rows)
504504

505+
--
506+
-- Test that ORDER BY for UNION ALL can be pushed down to inheritance
507+
-- children.
508+
--
509+
CREATE TEMP TABLE t1c (b text, a text);
510+
ALTER TABLE t1c INHERIT t1;
511+
CREATE TEMP TABLE t2c (primary key (ab)) INHERITS (t2);
512+
INSERT INTO t1c VALUES ('v', 'w'), ('c', 'd'), ('m', 'n'), ('e', 'f');
513+
INSERT INTO t2c VALUES ('vw'), ('cd'), ('mn'), ('ef');
514+
CREATE INDEX t1c_ab_idx on t1c ((a || b));
515+
set enable_seqscan = on;
516+
set enable_indexonlyscan = off;
517+
explain (costs off)
518+
SELECT * FROM
519+
(SELECT a || b AS ab FROM t1
520+
UNION ALL
521+
SELECT ab FROM t2) t
522+
ORDER BY 1 LIMIT 8;
523+
QUERY PLAN
524+
------------------------------------------------
525+
Limit
526+
-> Merge Append
527+
Sort Key: ((t1.a || t1.b))
528+
-> Index Scan using t1_ab_idx on t1
529+
-> Index Scan using t1c_ab_idx on t1c
530+
-> Index Scan using t2_pkey on t2
531+
-> Index Scan using t2c_pkey on t2c
532+
(7 rows)
533+
534+
SELECT * FROM
535+
(SELECT a || b AS ab FROM t1
536+
UNION ALL
537+
SELECT ab FROM t2) t
538+
ORDER BY 1 LIMIT 8;
539+
ab
540+
----
541+
ab
542+
ab
543+
cd
544+
dc
545+
ef
546+
fe
547+
mn
548+
nm
549+
(8 rows)
550+
505551
reset enable_seqscan;
506552
reset enable_indexscan;
507553
reset enable_bitmapscan;
554+
reset enable_indexonlyscan;
508555
-- Test constraint exclusion of UNION ALL subqueries
509556
explain (costs off)
510557
SELECT * FROM

src/test/regress/sql/union.sql

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,9 +198,38 @@ 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;
232+
reset enable_indexonlyscan;
204233

205234
-- Test constraint exclusion of UNION ALL subqueries
206235
explain (costs off)

0 commit comments

Comments
 (0)