Back-patch "Fix EquivalenceClass processing for nested append relations".
authorTom Lane <tgl@sss.pgh.pa.us>
Thu, 26 Jun 2014 17:41:01 +0000 (10:41 -0700)
committerTom Lane <tgl@sss.pgh.pa.us>
Thu, 26 Jun 2014 17:41:59 +0000 (10:41 -0700)
When we committed a87c729153e372f3731689a7be007bc2b53f1410, 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.

src/backend/optimizer/path/allpaths.c
src/backend/optimizer/path/equivclass.c
src/backend/optimizer/plan/createplan.c
src/test/regress/expected/union.out
src/test/regress/sql/union.sql

index 5d7c6abad94144ca3f849e262ed82ebff391688e..2009ca4acf50e4129ba845229e15fb808bb897cc 100644 (file)
@@ -1031,10 +1031,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)
@@ -1046,6 +1051,13 @@ accumulate_append_subpath(List *subpaths, Path *path)
        /* list_copy is important here to avoid sharing 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);
 }
index 257563ce6d129ab5a7b21f5d3a4cd4ddc0b0f0f6..93ac033ab8b62f82f7faf42e06e7696833f15dab 100644 (file)
@@ -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))
index 46df0daf2808df9bfe89958a531949881b4e1f60..3a54cb14652806400e2c304acbd75087ffca69dd 100644 (file)
@@ -753,7 +753,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,
index ae690cf9689ebc8e3e2cda052df2f0bb144f2938..c900588c5d901ba6f570a5b3fe6023b5c5558a0b 100644 (file)
@@ -501,9 +501,78 @@ explain (costs off)
                Index Cond: (ab = 'ab'::text)
 (6 rows)
 
+--
+-- Test that ORDER BY for UNION ALL can be pushed down to inheritance
+-- children.
+--
+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;
+-- This simpler variant of the above test has been observed to fail differently
+create table events (event_id int primary key);
+create table other_events (event_id int primary key);
+create table events_child () inherits (events);
+explain (costs off)
+select event_id
+ from (select event_id from events
+       union all
+       select event_id from other_events) ss
+ order by event_id;
+                        QUERY PLAN                        
+----------------------------------------------------------
+ Merge Append
+   Sort Key: events.event_id
+   ->  Index Scan using events_pkey on events
+   ->  Sort
+         Sort Key: events_child.event_id
+         ->  Seq Scan on events_child
+   ->  Index Scan using other_events_pkey on other_events
+(7 rows)
+
+drop table events_child, events, other_events;
+reset enable_indexonlyscan;
 -- Test constraint exclusion of UNION ALL subqueries
 explain (costs off)
  SELECT * FROM
index d567cf148191870756825fde711784a2704e60e6..9ff1551e5de4087e2ee46e44d45a227fc41c1f2d 100644 (file)
@@ -198,10 +198,55 @@ explain (costs off)
   SELECT * FROM t2) t
  WHERE ab = 'ab';
 
+--
+-- Test that ORDER BY for UNION ALL can be pushed down to inheritance
+-- children.
+--
+
+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;
 
+-- This simpler variant of the above test has been observed to fail differently
+
+create table events (event_id int primary key);
+create table other_events (event_id int primary key);
+create table events_child () inherits (events);
+
+explain (costs off)
+select event_id
+ from (select event_id from events
+       union all
+       select event_id from other_events) ss
+ order by event_id;
+
+drop table events_child, events, other_events;
+
+reset enable_indexonlyscan;
+
 -- Test constraint exclusion of UNION ALL subqueries
 explain (costs off)
  SELECT * FROM