Fix oversight in planning of GROUP queries: when an expression is used
authorTom Lane
Tue, 9 Jan 2001 03:48:51 +0000 (03:48 +0000)
committerTom Lane
Tue, 9 Jan 2001 03:48:51 +0000 (03:48 +0000)
as both a GROUP BY item and an output expression, the top-level Group
node should just copy up the evaluated expression value from its input,
rather than re-evaluating the expression.  Aside from any performance
benefit this might offer, this avoids a crash when there is a sub-SELECT
in said expression.

src/backend/optimizer/plan/setrefs.c
src/backend/optimizer/plan/subselect.c
src/backend/optimizer/util/tlist.c
src/include/optimizer/tlist.h

index deb020e2565b5451a84e8c4f65041801a9ba4acd..5a54c95691274cd21342ff987377288801296076 100644 (file)
@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/optimizer/plan/setrefs.c,v 1.68 2000/10/26 21:36:09 tgl Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/optimizer/plan/setrefs.c,v 1.69 2001/01/09 03:48:51 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -17,6 +17,7 @@
 
 #include "postgres.h"
 
+#include "nodes/makefuncs.h"
 #include "nodes/nodeFuncs.h"
 #include "optimizer/clauses.h"
 #include "optimizer/planmain.h"
@@ -270,22 +271,75 @@ set_join_references(Join *join)
  *   to refer to the tuples returned by its lefttree subplan.
  *
  * This is used for single-input plan types like Agg, Group, Result.
+ *
+ * In most cases, we have to match up individual Vars in the tlist and
+ * qual expressions with elements of the subplan's tlist (which was
+ * generated by flatten_tlist() from these selfsame expressions, so it
+ * should have all the required variables).  There is an important exception,
+ * however: a GROUP BY expression that is also an output expression will
+ * have been pushed into the subplan tlist unflattened.  We want to detect
+ * this case and reference the subplan output directly.  Therefore, check
+ * for equality of the whole tlist expression to any subplan element before
+ * we resort to picking the expression apart for individual Vars.
  */
 static void
 set_uppernode_references(Plan *plan, Index subvarno)
 {
    Plan       *subplan = plan->lefttree;
-   List       *subplanTargetList;
+   List       *subplanTargetList,
+              *outputTargetList,
+              *l;
 
    if (subplan != NULL)
        subplanTargetList = subplan->targetlist;
    else
        subplanTargetList = NIL;
 
-   plan->targetlist = (List *)
-       replace_vars_with_subplan_refs((Node *) plan->targetlist,
-                                      subvarno,
-                                      subplanTargetList);
+   outputTargetList = NIL;
+   foreach (l, plan->targetlist)
+   {
+       TargetEntry *tle = (TargetEntry *) lfirst(l);
+       TargetEntry *subplantle;
+       Node       *newexpr;
+
+       subplantle = tlistentry_member(tle->expr, subplanTargetList);
+       if (subplantle)
+       {
+           /* Found a matching subplan output expression */
+           Resdom *resdom = subplantle->resdom;
+           Var    *newvar;
+
+           newvar = makeVar(subvarno,
+                            resdom->resno,
+                            resdom->restype,
+                            resdom->restypmod,
+                            0);
+           /* If we're just copying a simple Var, copy up original info */
+           if (subplantle->expr && IsA(subplantle->expr, Var))
+           {
+               Var    *subvar = (Var *) subplantle->expr;
+
+               newvar->varnoold = subvar->varnoold;
+               newvar->varoattno = subvar->varoattno;
+           }
+           else
+           {
+               newvar->varnoold = 0;
+               newvar->varoattno = 0;
+           }
+           newexpr = (Node *) newvar;
+       }
+       else
+       {
+           /* No matching expression, so replace individual Vars */
+           newexpr = replace_vars_with_subplan_refs(tle->expr,
+                                                    subvarno,
+                                                    subplanTargetList);
+       }
+       outputTargetList = lappend(outputTargetList,
+                                  makeTargetEntry(tle->resdom, newexpr));
+   }
+   plan->targetlist = outputTargetList;
 
    plan->qual = (List *)
        replace_vars_with_subplan_refs((Node *) plan->qual,
index 4ebabc1e8753f47746d1dafb300d7e6fe8883ac7..61dd2baa7fd6436bd2c7f1bc16366a5ae7447242 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/optimizer/plan/subselect.c,v 1.46 2000/11/21 00:17:59 tgl Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/optimizer/plan/subselect.c,v 1.47 2001/01/09 03:48:51 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -125,6 +125,7 @@ make_subplan(SubLink *slink)
 {
    SubPlan    *node = makeNode(SubPlan);
    Query      *subquery = (Query *) (slink->subselect);
+   Oid         result_type = exprType((Node *) slink);
    double      tuple_fraction;
    Plan       *plan;
    List       *lst;
@@ -368,7 +369,7 @@ make_subplan(SubLink *slink)
        /*
         * Make expression of SUBPLAN type
         */
-       expr->typeOid = BOOLOID;/* bogus, but we don't really care */
+       expr->typeOid = result_type;
        expr->opType = SUBPLAN_EXPR;
        expr->oper = (Node *) node;
 
index 93b3fc6f568e87b33e82dd4af13cf68cfa2370c9..fa542a8828a906d80b17cb97e3b8aa7d29c8ec4e 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/optimizer/util/tlist.c,v 1.47 2000/08/08 15:41:53 tgl Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/optimizer/util/tlist.c,v 1.48 2001/01/09 03:48:50 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -18,7 +18,6 @@
 #include "optimizer/tlist.h"
 #include "optimizer/var.h"
 
-static TargetEntry *tlistentry_member(Node *node, List *targetlist);
 
 /*****************************************************************************
  * ---------- RELATION node target list routines ----------
@@ -29,7 +28,7 @@ static TargetEntry *tlistentry_member(Node *node, List *targetlist);
  *   Finds the (first) member of the given tlist whose expression is
  *   equal() to the given expression.  Result is NULL if no such member.
  */
-static TargetEntry *
+TargetEntry *
 tlistentry_member(Node *node, List *targetlist)
 {
    List       *temp;
index 8d3eb20d0244fc530e1d5992254266a65af8f46b..2159cbe2b8bb2a42b41730460ae12255cb6a8dab 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2000, PostgreSQL, Inc
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: tlist.h,v 1.26 2000/06/08 22:37:51 momjian Exp $
+ * $Id: tlist.h,v 1.27 2001/01/09 03:48:50 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -16,6 +16,7 @@
 
 #include "nodes/relation.h"
 
+extern TargetEntry *tlistentry_member(Node *node, List *targetlist);
 extern Resdom *tlist_member(Node *node, List *targetlist);
 
 extern void add_var_to_tlist(RelOptInfo *rel, Var *var);