Re: UNION ALL on partitioned tables won't use indices.

Noah Misch <noah@leadboat.com> writes:
> If you are convinced that a separate flattening pass is best, that suffices
> for me at this stage.  Please submit the patch you have in mind, incorporating
> any improvements from the v7 patch that are relevant to both approaches.

I went back and re-read the original message, and this time it struck me
that really the issue here is that add_child_rel_equivalences() doesn't
think it has to deal with the case of a "parent" rel being itself a child.
That's not inherent though; it's just that it didn't occur to me at the
time that such a situation could arise.  It takes only very small changes
to allow that to happen.

If you do that, as in the attached changes to equivclass.c, you get
"could not find pathkey item to sort" errors from createplan.c; but
that's just because create_merge_append_plan() is likewise not expecting
the mergeappend's parent rel to be itself a child.  Fix for that is
a one-liner, ie, pass down relids.

That gets you to a point where the code generates a valid plan, but it's
got nested MergeAppends, which are unnecessary expense.  Kyotaro-san's
original fix for that was overcomplicated.  It's sufficient to teach
accumulate_append_subpath() to flatten nested MergeAppendPaths.

In short, the attached patch seems to fix it, for a lot less added
complication than anything else that's been discussed on this thread.
I've only lightly tested it (it could use a new regression test case),
but unless someone can find a flaw I think we should use this approach.

            regards, tom lane

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
*************** get_cheapest_parameterized_child_path(Pl
*** 1021,1030 ****
   * 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.)
   */
  static List *
  accumulate_append_subpath(List *subpaths, Path *path)
--- 1021,1035 ----
   * 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 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)
*************** accumulate_append_subpath(List *subpaths
*** 1036,1041 ****
--- 1041,1053 ----
          /* 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);
  }
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
*************** add_child_rel_equivalences(PlannerInfo *
*** 1937,1952 ****
          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))
              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 */

              /* Does it reference parent_rel? */
              if (bms_overlap(cur_em->em_relids, parent_rel->relids))
--- 1937,1956 ----
          if (cur_ec->ec_has_volatile)
              continue;

!         /*
!          * 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)
!                 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
*************** create_merge_append_plan(PlannerInfo *ro
*** 751,757 ****

      /* Compute sort column info, and adjust MergeAppend's tlist as needed */
      (void) prepare_sort_from_pathkeys(root, plan, pathkeys,
!                                       NULL,
                                        NULL,
                                        true,
                                        &node->numCols,
--- 751,757 ----

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