Re: UNION ALL on partitioned tables won't use indices. - Mailing list pgsql-hackers

Hello. Attached is the 2nd version of 'pushdown in UNION ALL on
partitioned tables' patch type 1 - fix in equiv-member version.

As far as I can see, I have found no problem on the original
Tom's patch. I have no annoyance of modifying inh flag and so
with this.  

At Tue, 04 Mar 2014 18:57:56 +0900, Kyotaro HORIGUCHI wrote
me> Mmm. That's motifying but you seems to be right :) Equipping this
me> with some regression tests become my work from now.

And I took an advantage of using Noah's regression test after
some modifications. After all, this patch consists of work of you
all. Thanks for all you did to me.

I simplified the query for regression tests so as to clarify the
objective and getting rid of confisions of readers. Using only
the first column seems to be enough to also make sure of pushing
down and ordering.

Any comments?


At Wed, 05 Mar 2014 13:59:45 -0500, Tom Lane wrote
tgl> >> ec_relids has never included child relids.
> > |    Relids  ec_relids;   /* all relids appearing in ec_members */    
> > ...
> > |    Relids  em_relids;   /* all relids appearing in em_expr */   
> 
> Ah.  Those comments could use improvement, I guess.

The revised comment makes it clear. Thank you.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 03be7b1..5777cb2 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -1021,10 +1021,15 @@ get_cheapest_parameterized_child_path(PlannerInfo *root, RelOptInfo *rel, *
accumulate_append_subpath*        Add a subpath to the list being built for an Append or MergeAppend *
 
- * It's possible that the child is itself an Append path, in which case
- * we can "cut out the middleman" and just add its child paths to our
- * own list.  (We don't try to do this earlier because we need to
- * apply both levels of transformation to the quals.)
+ * It's possible that the child is itself an Append or MergeAppend path, in
+ * which case we can "cut out the middleman" and just add its child paths to
+ * our own list.  (We don't try to do this earlier because we need to apply
+ * both levels of transformation to the quals.)
+ *
+ * Note that if we omit a child MergeAppend in this way, we are effectively
+ * omitting a sort step, which seems fine: if the parent is to be an Append,
+ * its result would be unsorted anyway, while if the parent is to be a
+ * MergeAppend, there's no point in a separate sort on a child. */static List *accumulate_append_subpath(List
*subpaths,Path *path)
 
@@ -1036,6 +1041,13 @@ accumulate_append_subpath(List *subpaths, Path *path)        /* list_copy is important here to
avoidsharing list substructure */        return list_concat(subpaths, list_copy(apath->subpaths));    }
 
+    else if (IsA(path, MergeAppendPath))
+    {
+        MergeAppendPath *mpath = (MergeAppendPath *) path;
+
+        /* list_copy is important here to avoid sharing list substructure */
+        return list_concat(subpaths, list_copy(mpath->subpaths));
+    }    else        return lappend(subpaths, path);}
diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c
index 35d2a83..ac12f84 100644
--- a/src/backend/optimizer/path/equivclass.c
+++ b/src/backend/optimizer/path/equivclass.c
@@ -1937,16 +1937,20 @@ add_child_rel_equivalences(PlannerInfo *root,        if (cur_ec->ec_has_volatile)
continue;
-        /* No point in searching if parent rel not mentioned in eclass */
-        if (!bms_is_subset(parent_rel->relids, cur_ec->ec_relids))
+        /*
+         * No point in searching if parent rel not mentioned in eclass; but
+         * we can't tell that for sure if parent rel is itself a child.
+         */
+        if (parent_rel->reloptkind == RELOPT_BASEREL &&
+            !bms_is_subset(parent_rel->relids, cur_ec->ec_relids))            continue;        foreach(lc2,
cur_ec->ec_members)       {            EquivalenceMember *cur_em = (EquivalenceMember *) lfirst(lc2);
 
-            if (cur_em->em_is_const || cur_em->em_is_child)
-                continue;        /* ignore consts and children here */
+            if (cur_em->em_is_const)
+                continue;        /* ignore consts here */            /* Does it reference parent_rel? */            if
(bms_overlap(cur_em->em_relids,parent_rel->relids))
 
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 184d37a..784805f 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -751,7 +751,7 @@ create_merge_append_plan(PlannerInfo *root, MergeAppendPath *best_path)    /* Compute sort column
info,and adjust MergeAppend's tlist as needed */    (void) prepare_sort_from_pathkeys(root, plan, pathkeys,
 
-                                      NULL,
+                                      best_path->path.parent->relids,                                      NULL,
                              true,                                      &node->numCols,
 
diff --git a/src/test/regress/expected/union.out b/src/test/regress/expected/union.out
index 6f9ee5e..68643d8 100644
--- a/src/test/regress/expected/union.out
+++ b/src/test/regress/expected/union.out
@@ -502,9 +502,56 @@ explain (costs off)               Index Cond: (ab = 'ab'::text)(7 rows)
+--
+-- Test that ORDER BY for UNION ALL can be pushed down on inheritance
+-- tables.
+--
+CREATE TEMP TABLE t1c (b text, a text);
+ALTER TABLE t1c INHERIT t1;
+CREATE TEMP TABLE t2c (primary key (ab)) INHERITS (t2);
+INSERT INTO t1c VALUES ('v', 'w'), ('c', 'd'), ('m', 'n'), ('e', 'f');
+INSERT INTO t2c VALUES ('vw'), ('cd'), ('mn'), ('ef');
+CREATE INDEX t1c_ab_idx on t1c ((a || b));
+set enable_seqscan = on;
+set enable_indexonlyscan = off;
+explain (costs off)
+  SELECT * FROM
+  (SELECT a || b AS ab FROM t1
+   UNION ALL
+   SELECT ab FROM t2) t
+  ORDER BY 1 LIMIT 8;
+                   QUERY PLAN                   
+------------------------------------------------
+ Limit
+   ->  Merge Append
+         Sort Key: ((t1.a || t1.b))
+         ->  Index Scan using t1_ab_idx on t1
+         ->  Index Scan using t1c_ab_idx on t1c
+         ->  Index Scan using t2_pkey on t2
+         ->  Index Scan using t2c_pkey on t2c
+(7 rows)
+
+  SELECT * FROM
+  (SELECT a || b AS ab FROM t1
+   UNION ALL
+   SELECT ab FROM t2) t
+  ORDER BY 1  LIMIT 8;
+ ab 
+----
+ ab
+ ab
+ cd
+ dc
+ ef
+ fe
+ mn
+ nm
+(8 rows)
+reset enable_seqscan;reset enable_indexscan;reset enable_bitmapscan;
+reset enable_indexonlyscan;-- Test constraint exclusion of UNION ALL subqueriesexplain (costs off) SELECT * FROM
diff --git a/src/test/regress/sql/union.sql b/src/test/regress/sql/union.sql
index d567cf1..aa3ef7e 100644
--- a/src/test/regress/sql/union.sql
+++ b/src/test/regress/sql/union.sql
@@ -198,9 +198,37 @@ explain (costs off)  SELECT * FROM t2) t WHERE ab = 'ab';
+--
+-- Test that ORDER BY for UNION ALL can be pushed down on inheritance
+-- tables.
+--
+
+CREATE TEMP TABLE t1c (b text, a text);
+ALTER TABLE t1c INHERIT t1;
+CREATE TEMP TABLE t2c (primary key (ab)) INHERITS (t2);
+INSERT INTO t1c VALUES ('v', 'w'), ('c', 'd'), ('m', 'n'), ('e', 'f');
+INSERT INTO t2c VALUES ('vw'), ('cd'), ('mn'), ('ef');
+CREATE INDEX t1c_ab_idx on t1c ((a || b));
+set enable_seqscan = on;
+set enable_indexonlyscan = off;
+
+explain (costs off)
+  SELECT * FROM
+  (SELECT a || b AS ab FROM t1
+   UNION ALL
+   SELECT ab FROM t2) t
+  ORDER BY 1 LIMIT 8;
+
+  SELECT * FROM
+  (SELECT a || b AS ab FROM t1
+   UNION ALL
+   SELECT ab FROM t2) t
+  ORDER BY 1  LIMIT 8;
+reset enable_seqscan;reset enable_indexscan;reset enable_bitmapscan;
+reset enable_indexonlyscan;-- Test constraint exclusion of UNION ALL subqueriesexplain (costs off)