Re: ERROR: MergeAppend child's targetlist doesn't match MergeAppend - Mailing list pgsql-hackers

I wrote:
> Not immediately sure what to do about this.  The quick-and-dirty fix
> would be to only apply add_child_rel_equivalences to simple inheritance
> child relations, for which the added items must be Vars and must be
> different (which is what that code is expecting).  Seems like a bit of a
> shame to lobotomize planning for UNION cases, though.  Maybe we need a
> more complicated representation of child EquivalenceClass members.

After some thought and experimentation, the best fix seems to be to
eliminate the ambiguity by wrapping subquery outputs in PlaceHolderVars
whenever there is a risk of confusion.  The attached crude patch fixes
both test cases.  It needs work (more comments and a regression test
case would be good), but barring objection I'll push forward with doing
it this way.

            regards, tom lane

diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index aeaae8c8d87637b62e02e54b0c50509f98cd0138..5a9605d764ce268466c95fb73f219657a5781c25 100644
*** a/src/backend/optimizer/prep/prepjointree.c
--- b/src/backend/optimizer/prep/prepjointree.c
*************** pull_up_simple_subquery(PlannerInfo *roo
*** 784,805 ****
      parse->havingQual = pullup_replace_vars(parse->havingQual, &rvcontext);

      /*
!      * Replace references in the translated_vars lists of appendrels. When
!      * pulling up an appendrel member, we do not need PHVs in the list of the
!      * parent appendrel --- there isn't any outer join between. Elsewhere, use
!      * PHVs for safety.  (This analysis could be made tighter but it seems
!      * unlikely to be worth much trouble.)
       */
      foreach(lc, root->append_rel_list)
      {
          AppendRelInfo *appinfo = (AppendRelInfo *) lfirst(lc);
-         bool        save_need_phvs = rvcontext.need_phvs;

-         if (appinfo == containing_appendrel)
-             rvcontext.need_phvs = false;
          appinfo->translated_vars = (List *)
              pullup_replace_vars((Node *) appinfo->translated_vars, &rvcontext);
-         rvcontext.need_phvs = save_need_phvs;
      }

      /*
--- 784,799 ----
      parse->havingQual = pullup_replace_vars(parse->havingQual, &rvcontext);

      /*
!      * Replace references in the translated_vars lists of appendrels, too.
!      * We must preserve the original AppendRelInfo structs, so we have to do
!      * it this way.
       */
      foreach(lc, root->append_rel_list)
      {
          AppendRelInfo *appinfo = (AppendRelInfo *) lfirst(lc);

          appinfo->translated_vars = (List *)
              pullup_replace_vars((Node *) appinfo->translated_vars, &rvcontext);
      }

      /*
*************** pullup_replace_vars_callback(Var *var,
*** 1407,1420 ****
              if (newnode && IsA(newnode, Var) &&
                  ((Var *) newnode)->varlevelsup == 0)
              {
!                 /* Simple Vars always escape being wrapped */
!                 wrap = false;
              }
              else if (newnode && IsA(newnode, PlaceHolderVar) &&
                       ((PlaceHolderVar *) newnode)->phlevelsup == 0)
              {
                  /* No need to wrap a PlaceHolderVar with another one, either */
!                 wrap = false;
              }
              else if (rcon->wrap_non_vars)
              {
--- 1401,1416 ----
              if (newnode && IsA(newnode, Var) &&
                  ((Var *) newnode)->varlevelsup == 0)
              {
!                 /* Simple Vars always escape being wrapped, unless dup */
!                 wrap = (rcon->wrap_non_vars &&
!                         tlist_member(newnode, rcon->targetlist) != tle);
              }
              else if (newnode && IsA(newnode, PlaceHolderVar) &&
                       ((PlaceHolderVar *) newnode)->phlevelsup == 0)
              {
                  /* No need to wrap a PlaceHolderVar with another one, either */
!                 wrap = (rcon->wrap_non_vars &&
!                         tlist_member(newnode, rcon->targetlist) != tle);
              }
              else if (rcon->wrap_non_vars)
              {
diff --git a/src/backend/optimizer/util/placeholder.c b/src/backend/optimizer/util/placeholder.c
index 8b65245b5107595da7e15d09d009446ca0d701c7..a5d35f0783cc5b53094ed4c5790cdb48aad7069e 100644
*** a/src/backend/optimizer/util/placeholder.c
--- b/src/backend/optimizer/util/placeholder.c
*************** find_placeholders_in_jointree(PlannerInf
*** 120,125 ****
--- 120,128 ----
          Assert(root->parse->jointree != NULL &&
                 IsA(root->parse->jointree, FromExpr));
          (void) find_placeholders_recurse(root, (Node *) root->parse->jointree);
+
+         /* Also set up PlaceHolderInfos for PHVs in append_rel_list */
+         mark_placeholders_in_expr(root, (Node *) root->append_rel_list, NULL);
      }
  }