]> granicus.if.org Git - postgresql/commitdiff
Back-patch "Fix EquivalenceClass processing for nested append relations".
authorTom Lane <tgl@sss.pgh.pa.us>
Thu, 26 Jun 2014 17:41:10 +0000 (10:41 -0700)
committerTom Lane <tgl@sss.pgh.pa.us>
Thu, 26 Jun 2014 17:42:08 +0000 (10:42 -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 715f44675f8ae86a3a10555ad771d94d2c320d36..59b7233916392364d4111cb0069254d6458549b4 100644 (file)
@@ -635,10 +635,15 @@ set_append_rel_pathlist(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)
@@ -650,6 +655,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 2a3826cfa5e2cadb7890c0fe576ee9b98971fa88..d89feb34b32713f2d743b609385e87fa53e45b4e 100644 (file)
@@ -1850,16 +1850,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_child)
-                               continue;               /* ignore children here */
+                       if (cur_em->em_is_const)
+                               continue;               /* ignore consts here */
 
                        /* Does it reference (only) parent_rel? */
                        if (bms_equal(cur_em->em_relids, parent_rel->relids))
index c246468c929fb5960205f91c8b82f915cfa2c593..682217d1503509946c2c8434e4279a7c4fe54bf6 100644 (file)
@@ -700,7 +700,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 2c30e485c822933bac29637d03e702f79a8cc50f..9b6eb8a26ab49dc425dc97d788efc231217f8b13 100644 (file)
@@ -500,9 +500,37 @@ explain (costs off)
                Index Cond: (ab = 'ab'::text)
 (6 rows)
 
+--
+-- Test that ORDER BY for UNION ALL can be pushed down to inheritance
+-- children.
+--
 reset enable_seqscan;
 reset enable_indexscan;
 reset enable_bitmapscan;
+create table events (event_id int primary key);
+NOTICE:  CREATE TABLE / PRIMARY KEY will create implicit index "events_pkey" for table "events"
+create table other_events (event_id int primary key);
+NOTICE:  CREATE TABLE / PRIMARY KEY will create implicit index "other_events_pkey" for table "other_events"
+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                           
+----------------------------------------------------------------
+ Result
+   ->  Merge Append
+         Sort Key: public.events.event_id
+         ->  Index Scan using events_pkey on events
+         ->  Sort
+               Sort Key: public.events.event_id
+               ->  Seq Scan on events_child events
+         ->  Index Scan using other_events_pkey on other_events
+(8 rows)
+
+drop table events_child, events, other_events;
 -- Test constraint exclusion of UNION ALL subqueries
 explain (costs off)
  SELECT * FROM
index f3c9d113827eb240d055bfc32a31a60d89038dbc..86207d25b098e6e67dbddcbd0a684fcf730bc7de 100644 (file)
@@ -196,10 +196,28 @@ explain (costs off)
   SELECT * FROM t2) t
  WHERE ab = 'ab';
 
+--
+-- Test that ORDER BY for UNION ALL can be pushed down to inheritance
+-- children.
+--
+
 reset enable_seqscan;
 reset enable_indexscan;
 reset enable_bitmapscan;
 
+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;
+
 -- Test constraint exclusion of UNION ALL subqueries
 explain (costs off)
  SELECT * FROM