create_unique_plan() should not discard existing output columns of the

subplan it starts with, as they may be needed at upper join levels.
See comments added to code for the non-obvious reason why.  Per bug report
from Robert Creager.

diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 7529b7c0c7f3bef38d8ae513d5c3f5a6e6eacc18..c77f7d405393fd6a22b701d9de573348e75bcc76 100644 (file)
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v 1.151 2003/08/04 02:40:00 momjian Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v 1.152 2003/08/07 19:20:22 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -504,52 +504,87 @@ create_unique_plan(Query *root, UniquePath * best_path)
 {
        Plan       *plan;
        Plan       *subplan;
-       List       *sub_targetlist;
+       List       *uniq_exprs;
+       int                     numGroupCols;
+       AttrNumber *groupColIdx;
+       int                     groupColPos;
+       List       *newtlist;
+       int                     nextresno;
+       bool            newitems;
        List       *my_tlist;
        List       *l;
 
        subplan = create_plan(root, best_path->subpath);
 
        /*
-        * If the subplan came from an IN subselect (currently always the
-        * case), we need to instantiate the correct output targetlist for the
-        * subselect, rather than using the flattened tlist.
+        * As constructed, the subplan has a "flat" tlist containing just the
+        * Vars needed here and at upper levels.  The values we are supposed
+        * to unique-ify may be expressions in these variables.  We have to
+        * add any such expressions to the subplan's tlist.  We then build
+        * control information showing which subplan output columns are to be
+        * examined by the grouping step.  (Since we do not remove any existing
+        * subplan outputs, not all the output columns may be used for grouping.)
+        *
+        * Note: the reason we don't remove any subplan outputs is that there
+        * are scenarios where a Var is needed at higher levels even though it
+        * is not one of the nominal outputs of an IN clause.  Consider
+        *              WHERE x IN (SELECT y FROM t1,t2 WHERE y = z)
+        * Implied equality deduction will generate an "x = z" clause, which may
+        * get used instead of "x = y" in the upper join step.  Therefore the
+        * sub-select had better deliver both y and z in its targetlist.  It is
+        * sufficient to unique-ify on y, however.
+        *
+        * To find the correct list of values to unique-ify, we look in the
+        * information saved for IN expressions.  If this code is ever used in
+        * other scenarios, some other way of finding what to unique-ify will
+        * be needed.
         */
-       sub_targetlist = NIL;
+       uniq_exprs = NIL;                       /* just to keep compiler quiet */
        foreach(l, root->in_info_list)
        {
                InClauseInfo *ininfo = (InClauseInfo *) lfirst(l);
 
                if (bms_equal(ininfo->righthand, best_path->path.parent->relids))
                {
-                       sub_targetlist = ininfo->sub_targetlist;
+                       uniq_exprs = ininfo->sub_targetlist;
                        break;
                }
        }
-
-       if (sub_targetlist)
+       if (l == NIL)                           /* fell out of loop? */
+               elog(ERROR, "could not find UniquePath in in_info_list");
+
+       /* set up to record positions of unique columns */
+       numGroupCols = length(uniq_exprs);
+       groupColIdx = (AttrNumber *) palloc(numGroupCols * sizeof(AttrNumber));
+       groupColPos = 0;
+       /* not sure if tlist might be shared with other nodes, so copy */
+       newtlist = copyObject(subplan->targetlist);
+       nextresno = length(newtlist) + 1;
+       newitems = false;
+
+       foreach(l, uniq_exprs)
        {
-               /*
-                * Transform list of plain Vars into targetlist
-                */
-               List       *newtlist = NIL;
-               int                     resno = 1;
+               Node       *uniqexpr = lfirst(l);
+               TargetEntry *tle;
 
-               foreach(l, sub_targetlist)
+               tle = tlistentry_member(uniqexpr, newtlist);
+               if (!tle)
                {
-                       Node       *tlexpr = lfirst(l);
-                       TargetEntry *tle;
-
-                       tle = makeTargetEntry(makeResdom(resno,
-                                                                                        exprType(tlexpr),
-                                                                                        exprTypmod(tlexpr),
+                       tle = makeTargetEntry(makeResdom(nextresno,
+                                                                                        exprType(uniqexpr),
+                                                                                        exprTypmod(uniqexpr),
                                                                                         NULL,
                                                                                         false),
-                                                                 (Expr *) tlexpr);
+                                                                 (Expr *) uniqexpr);
                        newtlist = lappend(newtlist, tle);
-                       resno++;
+                       nextresno++;
+                       newitems = true;
                }
+               groupColIdx[groupColPos++] = tle->resdom->resno;
+       }
 
+       if (newitems)
+       {
                /*
                 * If the top plan node can't do projections, we need to add a
                 * Result node to help it along.
@@ -563,21 +598,15 @@ create_unique_plan(Query *root, UniquePath * best_path)
                        subplan->targetlist = newtlist;
        }
 
+       /* Copy tlist again to make one we can put sorting labels on */
        my_tlist = copyObject(subplan->targetlist);
 
        if (best_path->use_hash)
        {
-               int                     numGroupCols = length(my_tlist);
                long            numGroups;
-               AttrNumber *groupColIdx;
-               int                     i;
 
                numGroups = (long) Min(best_path->rows, (double) LONG_MAX);
 
-               groupColIdx = (AttrNumber *) palloc(numGroupCols * sizeof(AttrNumber));
-               for (i = 0; i < numGroupCols; i++)
-                       groupColIdx[i] = i + 1;
-
                plan = (Plan *) make_agg(root,
                                                                 my_tlist,
                                                                 NIL,
@@ -590,9 +619,17 @@ create_unique_plan(Query *root, UniquePath * best_path)
        }
        else
        {
-               List       *sortList;
+               List       *sortList = NIL;
 
-               sortList = addAllTargetsToSortList(NULL, NIL, my_tlist, false);
+               for (groupColPos = 0; groupColPos < numGroupCols; groupColPos++)
+               {
+                       TargetEntry *tle;
+
+                       tle = nth(groupColIdx[groupColPos] - 1, my_tlist);
+                       Assert(tle->resdom->resno == groupColIdx[groupColPos]);
+                       sortList = addTargetToSortList(NULL, tle, sortList,
+                                                                                  my_tlist, NIL, false);
+               }
                plan = (Plan *) make_sort_from_sortclauses(root, my_tlist,
                                                                                                   subplan, sortList);
                plan = (Plan *) make_unique(my_tlist, plan, sortList);

diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index f56e06136e84af72f3a28e2c571e4ee331d0de17..ebc3ed23eecf5d8dcbe5223a9c7fa5b2cf0d1bbd 100644 (file)
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/parser/parse_clause.c,v 1.120 2003/08/04 02:40:01 momjian Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/parser/parse_clause.c,v 1.121 2003/08/07 19:20:22 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -59,9 +59,6 @@ static Node *buildMergedJoinVar(ParseState *pstate, JoinType jointype,
                                   Var *l_colvar, Var *r_colvar);
 static TargetEntry *findTargetlistEntry(ParseState *pstate, Node *node,
                                        List *tlist, int clause);
-static List *addTargetToSortList(ParseState *pstate, TargetEntry *tle,
-                                       List *sortlist, List *targetlist,
-                                       List *opname, bool resolveUnknown);
 
 
 /*
@@ -1478,7 +1475,7 @@ addAllTargetsToSortList(ParseState *pstate, List *sortlist,
  *
  * Returns the updated ORDER BY list.
  */
-static List *
+List *
 addTargetToSortList(ParseState *pstate, TargetEntry *tle,
                                        List *sortlist, List *targetlist,
                                        List *opname, bool resolveUnknown)

diff --git a/src/include/parser/parse_clause.h b/src/include/parser/parse_clause.h
index e806b4c86b8fd9dcc497e2cb464fde63758b6fa9..670b72cfbac0e0bb108d075cd7f758b13e349886 100644 (file)
--- a/src/include/parser/parse_clause.h
+++ b/src/include/parser/parse_clause.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: parse_clause.h,v 1.35 2003/08/04 02:40:14 momjian Exp $
+ * $Id: parse_clause.h,v 1.36 2003/08/07 19:20:23 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -35,6 +35,9 @@ extern List *transformDistinctClause(ParseState *pstate, List *distinctlist,
 extern List *addAllTargetsToSortList(ParseState *pstate,
                                                List *sortlist, List *targetlist,
                                                bool resolveUnknown);
+extern List *addTargetToSortList(ParseState *pstate, TargetEntry *tle,
+                                       List *sortlist, List *targetlist,
+                                       List *opname, bool resolveUnknown);
 extern Index assignSortGroupRef(TargetEntry *tle, List *tlist);
 extern bool targetIsInSortList(TargetEntry *tle, List *sortList);
 

diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 6ee15172acd0a38168bf519be0968b674a3d587a..2ce1d7b177f5da5cd736b0ae7df2f0ef58bd28fc 100644 (file)
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -2127,6 +2127,18 @@ on (x1 = xx1) where (xx2 is not null);
   4 | 44 |  4 |     |   4 |  44
 (3 rows)
 
+--
+-- regression test: check for bug with propagation of implied equality
+-- to outside an IN
+--
+select count(*) from tenk1 a where unique1 in
+  (select unique1 from tenk1 b join tenk1 c using (unique1)
+   where b.unique2 = 42);
+ count 
+-------
+     1
+(1 row)
+
 --
 -- Clean up
 --

diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index e4ccfa45828340d09fa8f73acd356291d40791c7..9a286345b1450f02eff0a79c5fcde27214c6ad4b 100644 (file)
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -330,6 +330,14 @@ on (x1 = xx1) where (y2 is not null);
 select * from (x left join y on (x1 = y1)) left join x xx(xx1,xx2)
 on (x1 = xx1) where (xx2 is not null);
 
+--
+-- regression test: check for bug with propagation of implied equality
+-- to outside an IN
+--
+select count(*) from tenk1 a where unique1 in
+  (select unique1 from tenk1 b join tenk1 c using (unique1)
+   where b.unique2 = 42);
+
 
 --
 -- Clean up