Assign sort keys properly when there are duplicate entries in

pathkey list --- corrects misbehavior seen with multiple mergejoin clauses
mentioning same variable.

diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 17f98389a8e2f6920da117876773e0fdf83199d1..3310bf5f925f2423920e20616bcc2e6ef9d8fbac 100644 (file)
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v 1.71 1999/08/16 02:17:53 tgl Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v 1.72 1999/08/16 23:07:20 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -28,7 +28,7 @@
 
 
 static List *switch_outer(List *clauses);
-static List *set_tlist_sort_info(List *tlist, List *pathkeys);
+static int set_tlist_sort_info(List *tlist, List *pathkeys);
 static Scan *create_scan_node(Path *best_path, List *tlist);
 static Join *create_join_node(JoinPath *best_path, List *tlist);
 static SeqScan *create_seqscan_node(Path *best_path, List *tlist,
@@ -804,40 +804,65 @@ switch_outer(List *clauses)
  *   Sets the reskey and reskeyop fields of resdom nodes in a target list
  *   for a sort node.
  *
- * 'tlist' is the target list
+ * 'tlist' is the target list (which is modified in-place).
+ *         tlist's reskey fields must be clear to start with.
  * 'pathkeys' is the desired pathkeys for the sort.  NIL means no sort.
  *
- * Returns the modified-in-place target list.
+ * Returns the number of sort keys assigned (which might be less than
+ * length(pathkeys)!)
  */
-static List *
+static int
 set_tlist_sort_info(List *tlist, List *pathkeys)
 {
-   int         keyno = 1;
+   int         keysassigned = 0;
    List       *i;
 
    foreach(i, pathkeys)
    {
        List           *keysublist = (List *) lfirst(i);
-       PathKeyItem    *pathkey;
-       Resdom         *resdom;
+       PathKeyItem    *pathkey = NULL;
+       Resdom         *resdom = NULL;
+       List           *j;
 
        /*
         * We can sort by any one of the sort key items listed in this
-        * sublist.  For now, we always take the first one --- is there
-        * any way of figuring out which might be cheapest to execute?
-        * (For example, int4lt is likely much cheaper to execute than
-        * numericlt, but both might appear in the same pathkey sublist...)
+        * sublist.  For now, we take the first one that corresponds to
+        * an available Var in the tlist.
+        *
+        * XXX if we have a choice, is there any way of figuring out which
+        * might be cheapest to execute?  (For example, int4lt is likely
+        * much cheaper to execute than numericlt, but both might appear in
+        * the same pathkey sublist...)  Not clear that we ever will have
+        * a choice in practice, so it may not matter.
         */
-       pathkey = lfirst(keysublist);
-       Assert(IsA(pathkey, PathKeyItem));
-       resdom = tlist_member((Var *) pathkey->key, tlist);
+       foreach(j, keysublist)
+       {
+           pathkey = lfirst(j);
+           Assert(IsA(pathkey, PathKeyItem));
+           resdom = tlist_member((Var *) pathkey->key, tlist);
+           if (resdom)
+               break;
+       }
        if (!resdom)
            elog(ERROR, "set_tlist_sort_info: cannot find tlist item to sort");
-       resdom->reskey = keyno;
-       resdom->reskeyop = get_opcode(pathkey->sortop);
-       keyno++;
+
+       /*
+        * The resdom might be already marked as a sort key, if the pathkeys
+        * contain duplicate entries.  (This can happen in scenarios where
+        * multiple mergejoinable clauses mention the same var, for example.)
+        * In that case the current pathkey is essentially a no-op, because
+        * only one value can be seen within any subgroup where it would be
+        * consulted.  We can ignore it.
+        */
+       if (resdom->reskey == 0)
+       {
+           /* OK, mark it as a sort key and set the sort operator regproc */
+           resdom->reskey = ++keysassigned;
+           resdom->reskeyop = get_opcode(pathkey->sortop);
+       }
    }
-   return tlist;
+
+   return keysassigned;
 }
 
 /*
@@ -885,34 +910,37 @@ make_noname(List *tlist,
            Plan *plan_node)
 {
    List       *noname_tlist;
+   int         numsortkeys;
+   Plan       *tmpplan;
    Noname     *retval;
 
    /* Create a new target list for the noname, with sort keys set. */
-   noname_tlist = set_tlist_sort_info(new_unsorted_tlist(tlist),
-                                      pathkeys);
+   noname_tlist = new_unsorted_tlist(tlist);
+   numsortkeys = set_tlist_sort_info(noname_tlist, pathkeys);
 
-   if (pathkeys != NIL)
+   if (numsortkeys > 0)
    {
        /* need to sort */
-       retval = (Noname *) make_seqscan(tlist,
-                                        NIL,
-                                        _NONAME_RELATION_ID_,
-                                        (Plan *) make_sort(noname_tlist,
-                                                   _NONAME_RELATION_ID_,
-                                                           plan_node,
-                                                           length(pathkeys)));
+       tmpplan = (Plan *) make_sort(noname_tlist,
+                                    _NONAME_RELATION_ID_,
+                                    plan_node,
+                                    numsortkeys);
    }
    else
    {
        /* no sort */
-       retval = (Noname *) make_seqscan(tlist,
-                                        NIL,
+       tmpplan = (Plan *) make_material(noname_tlist,
                                         _NONAME_RELATION_ID_,
-                                        (Plan *) make_material(noname_tlist,
-                                                   _NONAME_RELATION_ID_,
-                                                               plan_node,
-                                                               0));
+                                        plan_node,
+                                        0);
    }
+
+   /* Return a seqscan using the original tlist */
+   retval = (Noname *) make_seqscan(tlist,
+                                    NIL,
+                                    _NONAME_RELATION_ID_,
+                                    tmpplan);
+
    return retval;
 }