Thread

1. ERROR: MergeAppend child's targetlist doesn't match MergeAppend

   Teodor Sigaev <teodor@sigaev.ru> — 2011-11-08T17:49:28Z

   Hi!
   
   I ran into a problem with PG 9.1 and bug is observed even in master. After 
   simplifying a query (original was 9Kb long!) it's possible to reproduce it easily:
   
   CREATE TABLE wow (t1 text, t2 text);
   CREATE INDEX idx ON wow (t1,t2);
   
   SET enable_seqscan=off;
   SET enable_bitmapscan=off;
   
   EXPLAIN
   SELECT
        t1, t2
   FROM (
        SELECT t1, t2 FROM wow
        UNION ALL
        SELECT 'a', 'a' FROM wow
   ) t
   ORDER BY t1, t2;
   
   if second 'a' constant is changed to something else then it works fine.
   
   The root of problem is that tlist_member() (called in 
   create_merge_append_plan()) for second constant returns TargetEntry for first 
   constant because they are equal. And the same problem is observed if second 
   select is replaced by  "SELECT t1, t1 FROM wow".
   
   It's seems to me that check in create_merge_append_plan() is too restrictive:
            if (memcmp(sortColIdx, node->sortColIdx,
                       numsortkeys * sizeof(AttrNumber)) != 0)
                elog(ERROR, "MergeAppend child's targetlist doesn't match 
   MergeAppend");
   
   Although I think, that more accurate check will repeat work done in 
   prepare_sort_from_pathkeys().
   
   -- 
   Teodor Sigaev                                   E-mail: teodor@sigaev.ru
                                                       WWW: http://www.sigaev.ru/
   
   

2. Re: ERROR: MergeAppend child's targetlist doesn't match MergeAppend

   Tom Lane <tgl@sss.pgh.pa.us> — 2011-11-08T19:27:02Z

   Teodor Sigaev <teodor@sigaev.ru> writes:
   > SELECT
   >      t1, t2
   > FROM (
   >      SELECT t1, t2 FROM wow
   >      UNION ALL
   >      SELECT 'a', 'a' FROM wow
   > ) t
   > ORDER BY t1, t2;
   
   Hmm, interesting.
   
   > It's seems to me that check in create_merge_append_plan() is too restrictive:
   >          if (memcmp(sortColIdx, node->sortColIdx,
   >                     numsortkeys * sizeof(AttrNumber)) != 0)
   >              elog(ERROR, "MergeAppend child's targetlist doesn't match 
   > MergeAppend");
   
   No, it isn't.  That code is fine; the problem is that
   add_child_rel_equivalences is generating an invalid state of the
   EquivalenceClass structures by adding equal items to two different
   EquivalenceClasses.  We need to rethink what that routine is doing.
   It's definitely wrong for it to add constant items; here, that would
   imply injecting t1 = 'a' and t2 = 'a' conditions, which is not correct.
   
   > And the same problem is observed if second 
   > select is replaced by  "SELECT t1, t1 FROM wow".
   
   And this one is a bit nasty too, since it would still add equal items
   to two different ECs, leading to the conclusion that they should be
   merged, ie t1 = t2, which is likewise wrong.
   
   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.
   
   			regards, tom lane
   
   

3. Re: ERROR: MergeAppend child's targetlist doesn't match MergeAppend

   Tom Lane <tgl@sss.pgh.pa.us> — 2011-11-08T23:28:10Z

   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
   
   

4. Re: ERROR: MergeAppend child's targetlist doesn't match MergeAppend

   Teodor Sigaev <teodor@sigaev.ru> — 2011-11-09T07:38:52Z

   > 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.
   
   Thank you, your patch fixes original query too.
   
   -- 
   Teodor Sigaev                                   E-mail: teodor@sigaev.ru
                                                       WWW: http://www.sigaev.ru/