Fix problems with subselects used in GROUP BY expressions, per gripe

from Philip Warner.  Side effect of change is that GROUP BY expressions
will not be re-evaluated at multiple plan levels anymore, whereas this
sometimes happened with old code.

1 files changed, 78 insertions, 59 deletions

diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index e7f8361b9a7..652e7c294a0 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/optimizer/plan/setrefs.c,v 1.71 2001/03/22 03:59:37 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/optimizer/plan/setrefs.c,v 1.72 2001/10/30 19:58:58 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -33,7 +33,8 @@ typedef struct
 typedef struct
 {
 	Index		subvarno;
-	List	   *subplanTargetList;
+	List	   *subplan_targetlist;
+	bool		tlist_has_non_vars;
 } replace_vars_with_subplan_refs_context;
 
 static void fix_expr_references(Plan *plan, Node *node);
@@ -43,7 +44,8 @@ static Node *join_references_mutator(Node *node,
 						join_references_context *context);
 static Node *replace_vars_with_subplan_refs(Node *node,
 							   Index subvarno,
-							   List *subplanTargetList);
+							   List *subplan_targetlist,
+							   bool tlist_has_non_vars);
 static Node *replace_vars_with_subplan_refs_mutator(Node *node,
 						replace_vars_with_subplan_refs_context *context);
 static bool fix_opids_walker(Node *node, void *context);
@@ -278,75 +280,62 @@ set_join_references(Join *join)
  * 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.
+ * however: GROUP BY and ORDER BY expressions will have been pushed into the
+ * subplan tlist unflattened.  If these values are also needed in the output
+ * then we want to reference the subplan tlist element rather than recomputing
+ * the expression.
  */
 static void
 set_uppernode_references(Plan *plan, Index subvarno)
 {
 	Plan	   *subplan = plan->lefttree;
-	List	   *subplanTargetList,
-			   *outputTargetList,
+	List	   *subplan_targetlist,
+			   *output_targetlist,
 			   *l;
+	bool		tlist_has_non_vars;
 
 	if (subplan != NULL)
-		subplanTargetList = subplan->targetlist;
+		subplan_targetlist = subplan->targetlist;
 	else
-		subplanTargetList = NIL;
+		subplan_targetlist = NIL;
 
-	outputTargetList = NIL;
-	foreach(l, plan->targetlist)
+	/*
+	 * Detect whether subplan tlist has any non-Vars (typically it won't
+	 * because it's been flattened).  This allows us to save comparisons
+	 * in common cases.
+	 */
+	tlist_has_non_vars = false;
+	foreach(l, subplan_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
+		if (tle->expr && ! IsA(tle->expr, Var))
 		{
-			/* No matching expression, so replace individual Vars */
-			newexpr = replace_vars_with_subplan_refs(tle->expr,
-													 subvarno,
-													 subplanTargetList);
+			tlist_has_non_vars = true;
+			break;
 		}
-		outputTargetList = lappend(outputTargetList,
-								   makeTargetEntry(tle->resdom, newexpr));
 	}
-	plan->targetlist = outputTargetList;
+
+	output_targetlist = NIL;
+	foreach(l, plan->targetlist)
+	{
+		TargetEntry *tle = (TargetEntry *) lfirst(l);
+		Node	   *newexpr;
+
+		newexpr = replace_vars_with_subplan_refs(tle->expr,
+												 subvarno,
+												 subplan_targetlist,
+												 tlist_has_non_vars);
+		output_targetlist = lappend(output_targetlist,
+									makeTargetEntry(tle->resdom, newexpr));
+	}
+	plan->targetlist = output_targetlist;
 
 	plan->qual = (List *)
 		replace_vars_with_subplan_refs((Node *) plan->qual,
 									   subvarno,
-									   subplanTargetList);
+									   subplan_targetlist,
+									   tlist_has_non_vars);
 }
 
 /*
@@ -439,9 +428,16 @@ join_references_mutator(Node *node,
  * --- so this routine should only be applied to nodes whose subplans'
  * targetlists were generated via flatten_tlist() or some such method.
  *
- * 'node': the tree to be fixed (a targetlist or qual list)
+ * If tlist_has_non_vars is true, then we try to match whole subexpressions
+ * against elements of the subplan tlist, so that we can avoid recomputing
+ * expressions that were already computed by the subplan.  (This is relatively
+ * expensive, so we don't want to try it in the common case where the
+ * subplan tlist is just a flattened list of Vars.)
+ *
+ * 'node': the tree to be fixed (a target item or qual)
  * 'subvarno': varno to be assigned to all Vars
- * 'subplanTargetList': target list for subplan
+ * 'subplan_targetlist': target list for subplan
+ * 'tlist_has_non_vars': true if subplan_targetlist contains non-Var exprs
  *
  * The resulting tree is a copy of the original in which all Var nodes have
  * varno = subvarno, varattno = resno of corresponding subplan target.
@@ -450,12 +446,14 @@ join_references_mutator(Node *node,
 static Node *
 replace_vars_with_subplan_refs(Node *node,
 							   Index subvarno,
-							   List *subplanTargetList)
+							   List *subplan_targetlist,
+							   bool tlist_has_non_vars)
 {
 	replace_vars_with_subplan_refs_context context;
 
 	context.subvarno = subvarno;
-	context.subplanTargetList = subplanTargetList;
+	context.subplan_targetlist = subplan_targetlist;
+	context.tlist_has_non_vars = tlist_has_non_vars;
 	return replace_vars_with_subplan_refs_mutator(node, &context);
 }
 
@@ -468,17 +466,38 @@ replace_vars_with_subplan_refs_mutator(Node *node,
 	if (IsA(node, Var))
 	{
 		Var		   *var = (Var *) node;
-		Var		   *newvar = (Var *) copyObject(var);
 		Resdom	   *resdom;
+		Var		   *newvar;
 
-		resdom = tlist_member((Node *) var, context->subplanTargetList);
+		resdom = tlist_member((Node *) var, context->subplan_targetlist);
 		if (!resdom)
 			elog(ERROR, "replace_vars_with_subplan_refs: variable not in subplan target list");
-
+		newvar = (Var *) copyObject(var);
 		newvar->varno = context->subvarno;
 		newvar->varattno = resdom->resno;
 		return (Node *) newvar;
 	}
+	/* Try matching more complex expressions too, if tlist has any */
+	if (context->tlist_has_non_vars)
+	{
+		Resdom	   *resdom;
+
+		resdom = tlist_member(node, context->subplan_targetlist);
+		if (resdom)
+		{
+			/* Found a matching subplan output expression */
+			Var		   *newvar;
+
+			newvar = makeVar(context->subvarno,
+							 resdom->resno,
+							 resdom->restype,
+							 resdom->restypmod,
+							 0);
+			newvar->varnoold = 0; /* wasn't ever a plain Var */
+			newvar->varoattno = 0;
+			return (Node *) newvar;
+		}
+	}
 	return expression_tree_mutator(node,
 								   replace_vars_with_subplan_refs_mutator,
 								   (void *) context);