From 389af07cf0bf80655b4f8f4eb0add8859d2154f8 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 1 Oct 1999 04:08:24 +0000
Subject: [PATCH] Clean up rewriter routines to use expression_tree_walker and
 expression_tree_mutator rather than ad-hoc tree walking code.  This shortens
 the code materially and fixes a fair number of sins of omission.  Also,
 change modifyAggrefQual to *not* recurse into subselects, since its mission
 is satisfied if it removes aggregate functions from the top level of a WHERE
 clause.  This cures problems with queries of the form SELECT ... WHERE x IN
 (SELECT ... HAVING something-using-an-aggregate), which would formerly get
 mucked up by modifyAggrefQual.  The routine is still fundamentally broken, of
 course, but I don't think there's any way to get rid of it before we
 implement subselects in FROM ...

---
 src/backend/rewrite/locks.c          |  137 +-
 src/backend/rewrite/rewriteHandler.c | 2062 +++++++-------------------
 src/backend/rewrite/rewriteManip.c   | 1212 +++++----------
 src/include/rewrite/rewriteManip.h   |   25 +-
 4 files changed, 979 insertions(+), 2457 deletions(-)

diff --git a/src/backend/rewrite/locks.c b/src/backend/rewrite/locks.c
index ef96b85210c..789e05b2246 100644
--- a/src/backend/rewrite/locks.c
+++ b/src/backend/rewrite/locks.c
@@ -6,7 +6,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/rewrite/Attic/locks.c,v 1.22 1999/09/18 19:07:18 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/rewrite/Attic/locks.c,v 1.23 1999/10/01 04:08:24 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -14,6 +14,7 @@
 
 #include "access/heapam.h"
 #include "catalog/pg_shadow.h"
+#include "optimizer/clauses.h"
 #include "rewrite/locks.h"
 #include "utils/acl.h"
 #include "utils/builtins.h"
@@ -22,102 +23,86 @@
 
 
 /*
- * ThisLockWasTriggered
+ * thisLockWasTriggered
  *
  * walk the tree, if there we find a varnode,
  * we check the varattno against the attnum
  * if we find at least one such match, we return true
  * otherwise, we return false
+ *
+ * XXX this should be unified with attribute_used()
  */
+
+typedef struct {
+	int			varno;
+	int			attnum;
+	int			sublevels_up;
+} thisLockWasTriggered_context;
+
 static bool
-nodeThisLockWasTriggered(Node *node, int varno, AttrNumber attnum,
-						 int sublevels_up)
+thisLockWasTriggered_walker (Node *node,
+							 thisLockWasTriggered_context *context)
 {
 	if (node == NULL)
-		return FALSE;
-	switch (nodeTag(node))
+		return false;
+	if (IsA(node, Var))
 	{
-		case T_Var:
-			{
-				Var		   *var = (Var *) node;
+		Var		   *var = (Var *) node;
 
-				if (varno == var->varno &&
-					(attnum == var->varattno || attnum == -1))
-					return TRUE;
-			}
-			break;
-		case T_Expr:
-			{
-				Expr	   *expr = (Expr *) node;
-
-				return nodeThisLockWasTriggered((Node *) expr->args, varno,
-												attnum, sublevels_up);
-			}
-			break;
-		case T_TargetEntry:
-			{
-				TargetEntry *tle = (TargetEntry *) node;
-
-				return nodeThisLockWasTriggered(tle->expr, varno, attnum,
-												sublevels_up);
-			}
-			break;
-		case T_Aggref:
-			{
-				Aggref	   *aggref = (Aggref *) node;
-
-				return nodeThisLockWasTriggered(aggref->target, varno, attnum,
-												sublevels_up);
-			}
-			break;
-		case T_List:
-			{
-				List	   *l;
-
-				foreach(l, (List *) node)
-				{
-					if (nodeThisLockWasTriggered(lfirst(l), varno, attnum,
-												 sublevels_up))
-						return TRUE;
-				}
-				return FALSE;
-			}
-			break;
-		case T_SubLink:
-			{
-				SubLink    *sublink = (SubLink *) node;
-				Query	   *query = (Query *) sublink->subselect;
+		if (var->varlevelsup == context->sublevels_up &&
+			var->varno == context->varno &&
+			(var->varattno == context->attnum || context->attnum == -1))
+			return true;
+		return false;
+	}
+	if (IsA(node, SubLink))
+	{
+		/*
+		 * Standard expression_tree_walker will not recurse into subselect,
+		 * but here we must do so.
+		 */
+		SubLink    *sub = (SubLink *) node;
 
-				return nodeThisLockWasTriggered(query->qual, varno, attnum,
-												sublevels_up + 1);
-			}
-			break;
-		default:
-			break;
+		if (thisLockWasTriggered_walker((Node *) (sub->lefthand), context))
+			return true;
+		context->sublevels_up++;
+		if (thisLockWasTriggered_walker((Node *) (sub->subselect), context))
+		{
+			context->sublevels_up--; /* not really necessary */
+			return true;
+		}
+		context->sublevels_up--;
+		return false;
+	}
+	if (IsA(node, Query))
+	{
+		/* Reach here after recursing down into subselect above... */
+		Query	   *qry = (Query *) node;
+
+		if (thisLockWasTriggered_walker((Node *) (qry->targetList), context))
+			return true;
+		if (thisLockWasTriggered_walker((Node *) (qry->qual), context))
+			return true;
+		if (thisLockWasTriggered_walker((Node *) (qry->havingQual), context))
+			return true;
+		return false;
 	}
-	return FALSE;
+	return expression_tree_walker(node, thisLockWasTriggered_walker,
+								  (void *) context);
 }
 
-/*
- * thisLockWasTriggered -
- *	   walk the tree, if there we find a varnode, we check the varattno
- *	   against the attnum if we find at least one such match, we return true
- *	   otherwise, we return false
- */
 static bool
 thisLockWasTriggered(int varno,
-					 AttrNumber attnum,
+					 int attnum,
 					 Query *parsetree)
 {
+	thisLockWasTriggered_context context;
 
-	if (nodeThisLockWasTriggered(parsetree->qual, varno, attnum, 0))
-		return true;
-
-	if (nodeThisLockWasTriggered((Node *) parsetree->targetList, varno, attnum, 0))
-		return true;
-
-	return false;
+	context.varno = varno;
+	context.attnum = attnum;
+	context.sublevels_up = 0;
 
+	return thisLockWasTriggered_walker((Node *) parsetree, &context);
 }
 
 /*
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index a76121aa823..42e87244e18 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -6,7 +6,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteHandler.c,v 1.57 1999/09/19 17:20:58 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteHandler.c,v 1.58 1999/10/01 04:08:24 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -32,6 +32,19 @@
 #include "utils/lsyscache.h"
 
 
+extern void CheckSelectForUpdate(Query *rule_action);	/* in analyze.c */
+
+
+/* macros borrowed from expression_tree_mutator */
+
+#define FLATCOPY(newnode, node, nodetype)  \
+	( (newnode) = makeNode(nodetype), \
+	  memcpy((newnode), (node), sizeof(nodetype)) )
+
+#define MUTATE(newfield, oldfield, fieldtype, mutator, context)  \
+		( (newfield) = (fieldtype) mutator((Node *) (oldfield), (context)) )
+
+
 static RewriteInfo *gatherRewriteMeta(Query *parsetree,
 				  Query *rule_action,
 				  Node *rule_qual,
@@ -39,12 +52,14 @@ static RewriteInfo *gatherRewriteMeta(Query *parsetree,
 				  CmdType event,
 				  bool *instead_flag);
 static bool rangeTableEntry_used(Node *node, int rt_index, int sublevels_up);
-static bool attribute_used(Node *node, int rt_index, int attno, int sublevels_up);
-static void modifyAggrefUplevel(Node *node);
-static void modifyAggrefChangeVarnodes(Node **nodePtr, int rt_index, int new_index, int sublevels_up);
-static void modifyAggrefDropQual(Node **nodePtr, Node *orignode, Expr *expr);
+static bool attribute_used(Node *node, int rt_index, int attno,
+						   int sublevels_up);
+static bool modifyAggrefUplevel(Node *node, void *context);
+static bool modifyAggrefChangeVarnodes(Node *node, int rt_index, int new_index,
+									   int sublevels_up);
+static Node *modifyAggrefDropQual(Node *node, Node *targetNode);
 static SubLink *modifyAggrefMakeSublink(Expr *origexp, Query *parsetree);
-static void modifyAggrefQual(Node **nodePtr, Query *parsetree);
+static Node *modifyAggrefQual(Node *node, Query *parsetree);
 static bool checkQueryHasAggs(Node *node);
 static bool checkQueryHasAggs_walker(Node *node, void *context);
 static bool checkQueryHasSubLink(Node *node);
@@ -135,221 +150,68 @@ gatherRewriteMeta(Query *parsetree,
  *	we need to process a RTE for RIR rules only if it is
  *	referenced somewhere in var nodes of the query.
  */
+
+typedef struct {
+	int			rt_index;
+	int			sublevels_up;
+} rangeTableEntry_used_context;
+
 static bool
-rangeTableEntry_used(Node *node, int rt_index, int sublevels_up)
+rangeTableEntry_used_walker (Node *node,
+							 rangeTableEntry_used_context *context)
 {
 	if (node == NULL)
-		return FALSE;
-
-	switch (nodeTag(node))
+		return false;
+	if (IsA(node, Var))
 	{
-		case T_TargetEntry:
-			{
-				TargetEntry *tle = (TargetEntry *) node;
-
-				return rangeTableEntry_used(
-											(Node *) (tle->expr),
-											rt_index,
-											sublevels_up);
-			}
-			break;
-
-		case T_Aggref:
-			{
-				Aggref	   *aggref = (Aggref *) node;
-
-				return rangeTableEntry_used(
-											(Node *) (aggref->target),
-											rt_index,
-											sublevels_up);
-			}
-			break;
-
-		case T_GroupClause:
-			return FALSE;
-
-		case T_Expr:
-			{
-				Expr	   *exp = (Expr *) node;
-
-				return rangeTableEntry_used(
-											(Node *) (exp->args),
-											rt_index,
-											sublevels_up);
-			}
-			break;
-
-		case T_Iter:
-			{
-				Iter	   *iter = (Iter *) node;
-
-				return rangeTableEntry_used(
-											(Node *) (iter->iterexpr),
-											rt_index,
-											sublevels_up);
-			}
-			break;
-
-		case T_ArrayRef:
-			{
-				ArrayRef   *ref = (ArrayRef *) node;
-
-				if (rangeTableEntry_used(
-										 (Node *) (ref->refupperindexpr),
-										 rt_index,
-										 sublevels_up))
-					return TRUE;
-
-				if (rangeTableEntry_used(
-										 (Node *) (ref->reflowerindexpr),
-										 rt_index,
-										 sublevels_up))
-					return TRUE;
-
-				if (rangeTableEntry_used(
-										 (Node *) (ref->refexpr),
-										 rt_index,
-										 sublevels_up))
-					return TRUE;
-
-				if (rangeTableEntry_used(
-										 (Node *) (ref->refassgnexpr),
-										 rt_index,
-										 sublevels_up))
-					return TRUE;
-
-				return FALSE;
-			}
-			break;
-
-		case T_Var:
-			{
-				Var		   *var = (Var *) node;
-
-				if (var->varlevelsup == sublevels_up)
-					return var->varno == rt_index;
-				else
-					return FALSE;
-			}
-			break;
-
-		case T_Param:
-			return FALSE;
-
-		case T_Const:
-			return FALSE;
-
-		case T_List:
-			{
-				List	   *l;
-
-				foreach(l, (List *) node)
-				{
-					if (rangeTableEntry_used(
-											 (Node *) lfirst(l),
-											 rt_index,
-											 sublevels_up))
-						return TRUE;
-				}
-				return FALSE;
-			}
-			break;
-
-		case T_SubLink:
-			{
-				SubLink    *sub = (SubLink *) node;
-
-				if (rangeTableEntry_used(
-										 (Node *) (sub->lefthand),
-										 rt_index,
-										 sublevels_up))
-					return TRUE;
-
-				if (rangeTableEntry_used(
-										 (Node *) (sub->subselect),
-										 rt_index,
-										 sublevels_up + 1))
-					return TRUE;
-
-				return FALSE;
-			}
-			break;
-
-		case T_CaseExpr:
-			{
-				CaseExpr   *exp = (CaseExpr *) node;
-
-				if (rangeTableEntry_used(
-										 (Node *) (exp->args),
-										 rt_index,
-										 sublevels_up))
-					return TRUE;
-
-				if (rangeTableEntry_used(
-										 (Node *) (exp->defresult),
-										 rt_index,
-										 sublevels_up))
-					return TRUE;
-
-				return FALSE;
-			}
-			break;
-
-		case T_CaseWhen:
-			{
-				CaseWhen   *when = (CaseWhen *) node;
-
-				if (rangeTableEntry_used(
-										 (Node *) (when->expr),
-										 rt_index,
-										 sublevels_up))
-					return TRUE;
-
-				if (rangeTableEntry_used(
-										 (Node *) (when->result),
-										 rt_index,
-										 sublevels_up))
-					return TRUE;
-
-				return FALSE;
-			}
-			break;
-
-		case T_Query:
-			{
-				Query	   *qry = (Query *) node;
-
-				if (rangeTableEntry_used(
-										 (Node *) (qry->targetList),
-										 rt_index,
-										 sublevels_up))
-					return TRUE;
-
-				if (rangeTableEntry_used(
-										 (Node *) (qry->qual),
-										 rt_index,
-										 sublevels_up))
-					return TRUE;
-
-				if (rangeTableEntry_used(
-										 (Node *) (qry->havingQual),
-										 rt_index,
-										 sublevels_up))
-					return TRUE;
-
-				return FALSE;
-			}
-			break;
-
-		default:
-			elog(NOTICE, "unknown node tag %d in rangeTableEntry_used()", nodeTag(node));
-			elog(NOTICE, "Node is: %s", nodeToString(node));
-			break;
-
+		Var		   *var = (Var *) node;
 
+		if (var->varlevelsup == context->sublevels_up &&
+			var->varno == context->rt_index)
+			return true;
+		return false;
 	}
+	if (IsA(node, SubLink))
+	{
+		/*
+		 * Standard expression_tree_walker will not recurse into subselect,
+		 * but here we must do so.
+		 */
+		SubLink    *sub = (SubLink *) node;
+
+		if (rangeTableEntry_used_walker((Node *) (sub->lefthand), context))
+			return true;
+		if (rangeTableEntry_used((Node *) (sub->subselect),
+								 context->rt_index,
+								 context->sublevels_up + 1))
+			return true;
+		return false;
+	}
+	if (IsA(node, Query))
+	{
+		/* Reach here after recursing down into subselect above... */
+		Query	   *qry = (Query *) node;
+
+		if (rangeTableEntry_used_walker((Node *) (qry->targetList), context))
+			return true;
+		if (rangeTableEntry_used_walker((Node *) (qry->qual), context))
+			return true;
+		if (rangeTableEntry_used_walker((Node *) (qry->havingQual), context))
+			return true;
+		return false;
+	}
+	return expression_tree_walker(node, rangeTableEntry_used_walker,
+								  (void *) context);
+}
+
+static bool
+rangeTableEntry_used(Node *node, int rt_index, int sublevels_up)
+{
+	rangeTableEntry_used_context context;
 
-	return FALSE;
+	context.rt_index = rt_index;
+	context.sublevels_up = sublevels_up;
+	return rangeTableEntry_used_walker(node, &context);
 }
 
 
@@ -358,667 +220,247 @@ rangeTableEntry_used(Node *node, int rt_index, int sublevels_up)
  *	Check if a specific attribute number of a RTE is used
  *	somewhere in the query
  */
+
+typedef struct {
+	int			rt_index;
+	int			attno;
+	int			sublevels_up;
+} attribute_used_context;
+
 static bool
-attribute_used(Node *node, int rt_index, int attno, int sublevels_up)
+attribute_used_walker (Node *node,
+					   attribute_used_context *context)
 {
 	if (node == NULL)
-		return FALSE;
-
-	switch (nodeTag(node))
+		return false;
+	if (IsA(node, Var))
 	{
-		case T_TargetEntry:
-			{
-				TargetEntry *tle = (TargetEntry *) node;
-
-				return attribute_used(
-									  (Node *) (tle->expr),
-									  rt_index,
-									  attno,
-									  sublevels_up);
-			}
-			break;
-
-		case T_Aggref:
-			{
-				Aggref	   *aggref = (Aggref *) node;
-
-				return attribute_used(
-									  (Node *) (aggref->target),
-									  rt_index,
-									  attno,
-									  sublevels_up);
-			}
-			break;
-
-		case T_GroupClause:
-			return FALSE;
-
-		case T_Expr:
-			{
-				Expr	   *exp = (Expr *) node;
-
-				return attribute_used(
-									  (Node *) (exp->args),
-									  rt_index,
-									  attno,
-									  sublevels_up);
-			}
-			break;
-
-		case T_Iter:
-			{
-				Iter	   *iter = (Iter *) node;
-
-				return attribute_used(
-									  (Node *) (iter->iterexpr),
-									  rt_index,
-									  attno,
-									  sublevels_up);
-			}
-			break;
-
-		case T_ArrayRef:
-			{
-				ArrayRef   *ref = (ArrayRef *) node;
-
-				if (attribute_used(
-								   (Node *) (ref->refupperindexpr),
-								   rt_index,
-								   attno,
-								   sublevels_up))
-					return TRUE;
-
-				if (attribute_used(
-								   (Node *) (ref->reflowerindexpr),
-								   rt_index,
-								   attno,
-								   sublevels_up))
-					return TRUE;
-
-				if (attribute_used(
-								   (Node *) (ref->refexpr),
-								   rt_index,
-								   attno,
-								   sublevels_up))
-					return TRUE;
-
-				if (attribute_used(
-								   (Node *) (ref->refassgnexpr),
-								   rt_index,
-								   attno,
-								   sublevels_up))
-					return TRUE;
-
-				return FALSE;
-			}
-			break;
-
-		case T_Var:
-			{
-				Var		   *var = (Var *) node;
-
-				if (var->varlevelsup == sublevels_up)
-					return var->varno == rt_index;
-				else
-					return FALSE;
-			}
-			break;
-
-		case T_Param:
-			return FALSE;
-
-		case T_Const:
-			return FALSE;
-
-		case T_List:
-			{
-				List	   *l;
-
-				foreach(l, (List *) node)
-				{
-					if (attribute_used(
-									   (Node *) lfirst(l),
-									   rt_index,
-									   attno,
-									   sublevels_up))
-						return TRUE;
-				}
-				return FALSE;
-			}
-			break;
-
-		case T_SubLink:
-			{
-				SubLink    *sub = (SubLink *) node;
-
-				if (attribute_used(
-								   (Node *) (sub->lefthand),
-								   rt_index,
-								   attno,
-								   sublevels_up))
-					return TRUE;
-
-				if (attribute_used(
-								   (Node *) (sub->subselect),
-								   rt_index,
-								   attno,
-								   sublevels_up + 1))
-					return TRUE;
-
-				return FALSE;
-			}
-			break;
-
-		case T_Query:
-			{
-				Query	   *qry = (Query *) node;
-
-				if (attribute_used(
-								   (Node *) (qry->targetList),
-								   rt_index,
-								   attno,
-								   sublevels_up))
-					return TRUE;
-
-				if (attribute_used(
-								   (Node *) (qry->qual),
-								   rt_index,
-								   attno,
-								   sublevels_up))
-					return TRUE;
-
-				if (attribute_used(
-								   (Node *) (qry->havingQual),
-								   rt_index,
-								   attno,
-								   sublevels_up))
-					return TRUE;
-
-				return FALSE;
-			}
-			break;
-
-		default:
-			elog(NOTICE, "unknown node tag %d in attribute_used()", nodeTag(node));
-			elog(NOTICE, "Node is: %s", nodeToString(node));
-			break;
-
+		Var		   *var = (Var *) node;
 
+		if (var->varlevelsup == context->sublevels_up &&
+			var->varno == context->rt_index &&
+			var->varattno == context->attno)
+			return true;
+		return false;
+	}
+	if (IsA(node, SubLink))
+	{
+		/*
+		 * Standard expression_tree_walker will not recurse into subselect,
+		 * but here we must do so.
+		 */
+		SubLink    *sub = (SubLink *) node;
+
+		if (attribute_used_walker((Node *) (sub->lefthand), context))
+			return true;
+		if (attribute_used((Node *) (sub->subselect),
+						   context->rt_index,
+						   context->attno,
+						   context->sublevels_up + 1))
+			return true;
+		return false;
+	}
+	if (IsA(node, Query))
+	{
+		/* Reach here after recursing down into subselect above... */
+		Query	   *qry = (Query *) node;
+
+		if (attribute_used_walker((Node *) (qry->targetList), context))
+			return true;
+		if (attribute_used_walker((Node *) (qry->qual), context))
+			return true;
+		if (attribute_used_walker((Node *) (qry->havingQual), context))
+			return true;
+		return false;
 	}
+	return expression_tree_walker(node, attribute_used_walker,
+								  (void *) context);
+}
+
+static bool
+attribute_used(Node *node, int rt_index, int attno, int sublevels_up)
+{
+	attribute_used_context context;
 
-	return FALSE;
+	context.rt_index = rt_index;
+	context.attno = attno;
+	context.sublevels_up = sublevels_up;
+	return attribute_used_walker(node, &context);
 }
 
 
 /*
  * modifyAggrefUplevel -
  *	In the newly created sublink for an aggregate column used in
- *	the qualification, we must adjust the varlevelsup in all the
+ *	the qualification, we must increment the varlevelsup in all the
  *	var nodes.
+ *
+ * NOTE: although this has the form of a walker, we cheat and modify the
+ * Var nodes in-place.  The given expression tree should have been copied
+ * earlier to ensure that no unwanted side-effects occur!
  */
-static void
-modifyAggrefUplevel(Node *node)
+static bool
+modifyAggrefUplevel(Node *node, void *context)
 {
 	if (node == NULL)
-		return;
-
-	switch (nodeTag(node))
+		return false;
+	if (IsA(node, Var))
 	{
-		case T_TargetEntry:
-			{
-				TargetEntry *tle = (TargetEntry *) node;
-
-				modifyAggrefUplevel(
-									(Node *) (tle->expr));
-			}
-			break;
-
-		case T_Aggref:
-			{
-				Aggref	   *aggref = (Aggref *) node;
-
-				modifyAggrefUplevel(
-									(Node *) (aggref->target));
-			}
-			break;
-
-		case T_Expr:
-			{
-				Expr	   *exp = (Expr *) node;
-
-				modifyAggrefUplevel(
-									(Node *) (exp->args));
-			}
-			break;
-
-		case T_Iter:
-			{
-				Iter	   *iter = (Iter *) node;
-
-				modifyAggrefUplevel(
-									(Node *) (iter->iterexpr));
-			}
-			break;
-
-		case T_ArrayRef:
-			{
-				ArrayRef   *ref = (ArrayRef *) node;
-
-				modifyAggrefUplevel(
-									(Node *) (ref->refupperindexpr));
-				modifyAggrefUplevel(
-									(Node *) (ref->reflowerindexpr));
-				modifyAggrefUplevel(
-									(Node *) (ref->refexpr));
-				modifyAggrefUplevel(
-									(Node *) (ref->refassgnexpr));
-			}
-			break;
-
-		case T_Var:
-			{
-				Var		   *var = (Var *) node;
-
-				var->varlevelsup++;
-			}
-			break;
-
-		case T_Param:
-			break;
-
-		case T_Const:
-			break;
-
-		case T_List:
-			{
-				List	   *l;
-
-				foreach(l, (List *) node)
-					modifyAggrefUplevel(
-										(Node *) lfirst(l));
-			}
-			break;
-
-		case T_SubLink:
-			{
-				SubLink    *sub = (SubLink *) node;
-
-				modifyAggrefUplevel(
-									(Node *) (sub->lefthand));
-
-				modifyAggrefUplevel(
-									(Node *) (sub->subselect));
-			}
-			break;
-
-		case T_Query:
-			{
-				Query	   *qry = (Query *) node;
-
-				modifyAggrefUplevel(
-									(Node *) (qry->targetList));
-
-				modifyAggrefUplevel(
-									(Node *) (qry->qual));
-
-				modifyAggrefUplevel(
-									(Node *) (qry->havingQual));
-
-			}
-			break;
-
-		default:
-			elog(NOTICE, "unknown node tag %d in modifyAggrefUplevel()", nodeTag(node));
-			elog(NOTICE, "Node is: %s", nodeToString(node));
-			break;
+		Var		   *var = (Var *) node;
 
+		var->varlevelsup++;
+		return false;
+	}
+	if (IsA(node, SubLink))
+	{
+		/*
+		 * Standard expression_tree_walker will not recurse into subselect,
+		 * but here we must do so.
+		 */
+		SubLink    *sub = (SubLink *) node;
 
+		if (modifyAggrefUplevel((Node *) (sub->lefthand), context))
+			return true;
+		if (modifyAggrefUplevel((Node *) (sub->subselect), context))
+			return true;
+		return false;
 	}
+	if (IsA(node, Query))
+	{
+		/* Reach here after recursing down into subselect above... */
+		Query	   *qry = (Query *) node;
+
+		if (modifyAggrefUplevel((Node *) (qry->targetList), context))
+			return true;
+		if (modifyAggrefUplevel((Node *) (qry->qual), context))
+			return true;
+		if (modifyAggrefUplevel((Node *) (qry->havingQual), context))
+			return true;
+		return false;
+	}
+	return expression_tree_walker(node, modifyAggrefUplevel,
+								  (void *) context);
 }
 
 
 /*
  * modifyAggrefChangeVarnodes -
  *	Change the var nodes in a sublink created for an aggregate column
- *	used in the qualification that is subject of the aggregate
- *	function to point to the correct local RTE.
+ *	used in the qualification to point to the correct local RTE.
+ *
+ * NOTE: although this has the form of a walker, we cheat and modify the
+ * Var nodes in-place.  The given expression tree should have been copied
+ * earlier to ensure that no unwanted side-effects occur!
  */
-static void
-modifyAggrefChangeVarnodes(Node **nodePtr, int rt_index, int new_index, int sublevels_up)
-{
-	Node	   *node = *nodePtr;
 
-	if (node == NULL)
-		return;
+typedef struct {
+	int			rt_index;
+	int			new_index;
+	int			sublevels_up;
+} modifyAggrefChangeVarnodes_context;
 
-	switch (nodeTag(node))
+static bool
+modifyAggrefChangeVarnodes_walker(Node *node,
+								  modifyAggrefChangeVarnodes_context *context)
+{
+	if (node == NULL)
+		return false;
+	if (IsA(node, Var))
 	{
-		case T_TargetEntry:
-			{
-				TargetEntry *tle = (TargetEntry *) node;
-
-				modifyAggrefChangeVarnodes(
-										   (Node **) (&(tle->expr)),
-										   rt_index,
-										   new_index,
-										   sublevels_up);
-			}
-			break;
-
-		case T_Aggref:
-			{
-				Aggref	   *aggref = (Aggref *) node;
-
-				modifyAggrefChangeVarnodes(
-										   (Node **) (&(aggref->target)),
-										   rt_index,
-										   new_index,
-										   sublevels_up);
-			}
-			break;
-
-		case T_GroupClause:
-			break;
-
-		case T_Expr:
-			{
-				Expr	   *exp = (Expr *) node;
-
-				modifyAggrefChangeVarnodes(
-										   (Node **) (&(exp->args)),
-										   rt_index,
-										   new_index,
-										   sublevels_up);
-			}
-			break;
-
-		case T_Iter:
-			{
-				Iter	   *iter = (Iter *) node;
-
-				modifyAggrefChangeVarnodes(
-										   (Node **) (&(iter->iterexpr)),
-										   rt_index,
-										   new_index,
-										   sublevels_up);
-			}
-			break;
-
-		case T_ArrayRef:
-			{
-				ArrayRef   *ref = (ArrayRef *) node;
-
-				modifyAggrefChangeVarnodes(
-									 (Node **) (&(ref->refupperindexpr)),
-										   rt_index,
-										   new_index,
-										   sublevels_up);
-				modifyAggrefChangeVarnodes(
-									 (Node **) (&(ref->reflowerindexpr)),
-										   rt_index,
-										   new_index,
-										   sublevels_up);
-				modifyAggrefChangeVarnodes(
-										   (Node **) (&(ref->refexpr)),
-										   rt_index,
-										   new_index,
-										   sublevels_up);
-				modifyAggrefChangeVarnodes(
-										(Node **) (&(ref->refassgnexpr)),
-										   rt_index,
-										   new_index,
-										   sublevels_up);
-			}
-			break;
-
-		case T_Var:
-			{
-				Var		   *var = (Var *) node;
-
-				if (var->varlevelsup == sublevels_up &&
-					var->varno == rt_index)
-				{
-					var = copyObject(var);
-					var->varno = new_index;
-					var->varnoold = new_index;
-					var->varlevelsup = 0;
-
-					*nodePtr = (Node *) var;
-				}
-			}
-			break;
-
-		case T_Param:
-			break;
-
-		case T_Const:
-			break;
-
-		case T_List:
-			{
-				List	   *l;
-
-				foreach(l, (List *) node)
-					modifyAggrefChangeVarnodes(
-											   (Node **) (&lfirst(l)),
-											   rt_index,
-											   new_index,
-											   sublevels_up);
-			}
-			break;
-
-		case T_SubLink:
-			{
-				SubLink    *sub = (SubLink *) node;
-
-				modifyAggrefChangeVarnodes(
-										   (Node **) (&(sub->lefthand)),
-										   rt_index,
-										   new_index,
-										   sublevels_up);
-
-				modifyAggrefChangeVarnodes(
-										   (Node **) (&(sub->subselect)),
-										   rt_index,
-										   new_index,
-										   sublevels_up + 1);
-			}
-			break;
+		Var		   *var = (Var *) node;
 
-		case T_Query:
-			{
-				Query	   *qry = (Query *) node;
-
-				modifyAggrefChangeVarnodes(
-										   (Node **) (&(qry->targetList)),
-										   rt_index,
-										   new_index,
-										   sublevels_up);
-
-				modifyAggrefChangeVarnodes(
-										   (Node **) (&(qry->qual)),
-										   rt_index,
-										   new_index,
-										   sublevels_up);
-
-				modifyAggrefChangeVarnodes(
-										   (Node **) (&(qry->havingQual)),
-										   rt_index,
-										   new_index,
-										   sublevels_up);
-			}
-			break;
-
-		default:
-			elog(NOTICE, "unknown node tag %d in modifyAggrefChangeVarnodes()", nodeTag(node));
-			elog(NOTICE, "Node is: %s", nodeToString(node));
-			break;
+		if (var->varlevelsup == context->sublevels_up &&
+			var->varno == context->rt_index)
+		{
+			var->varno = context->new_index;
+			var->varnoold = context->new_index;
+			var->varlevelsup = 0;
+		}
+		return false;
+	}
+	if (IsA(node, SubLink))
+	{
+		/*
+		 * Standard expression_tree_walker will not recurse into subselect,
+		 * but here we must do so.
+		 */
+		SubLink    *sub = (SubLink *) node;
+
+		if (modifyAggrefChangeVarnodes_walker((Node *) (sub->lefthand),
+											  context))
+			return true;
+		if (modifyAggrefChangeVarnodes((Node *) (sub->subselect),
+									   context->rt_index,
+									   context->new_index,
+									   context->sublevels_up + 1))
+			return true;
+		return false;
+	}
+	if (IsA(node, Query))
+	{
+		/* Reach here after recursing down into subselect above... */
+		Query	   *qry = (Query *) node;
+
+		if (modifyAggrefChangeVarnodes_walker((Node *) (qry->targetList),
+											  context))
+			return true;
+		if (modifyAggrefChangeVarnodes_walker((Node *) (qry->qual),
+											  context))
+			return true;
+		if (modifyAggrefChangeVarnodes_walker((Node *) (qry->havingQual),
+											  context))
+			return true;
+		return false;
+	}
+	return expression_tree_walker(node, modifyAggrefChangeVarnodes_walker,
+								  (void *) context);
+}
 
+static bool
+modifyAggrefChangeVarnodes(Node *node, int rt_index, int new_index,
+						   int sublevels_up)
+{
+	modifyAggrefChangeVarnodes_context context;
 
-	}
+	context.rt_index = rt_index;
+	context.new_index = new_index;
+	context.sublevels_up = sublevels_up;
+	return modifyAggrefChangeVarnodes_walker(node, &context);
 }
 
 
 /*
  * modifyAggrefDropQual -
- *	remove the pure aggref clase from a qualification
+ *	remove the pure aggref clause from a qualification
+ *
+ * targetNode is a boolean expression node somewhere within the given
+ * expression tree.  When we find it, replace it with a constant TRUE.
+ * The return tree is a modified copy of the given tree; the given tree
+ * is not altered.
+ *
+ * Note: we don't recurse into subselects looking for targetNode; that's
+ * not necessary in the current usage, since in fact targetNode will be
+ * within the same select level as the given toplevel node.
  */
-static void
-modifyAggrefDropQual(Node **nodePtr, Node *orignode, Expr *expr)
+static Node *
+modifyAggrefDropQual(Node *node, Node *targetNode)
 {
-	Node	   *node = *nodePtr;
-
 	if (node == NULL)
-		return;
-
-	switch (nodeTag(node))
+		return NULL;
+	if (node == targetNode)
 	{
-		case T_Var:
-			break;
-
-		case T_Aggref:
-			{
-				Aggref	   *aggref = (Aggref *) node;
-				Aggref	   *oaggref = (Aggref *) orignode;
-
-				modifyAggrefDropQual(
-									 (Node **) (&(aggref->target)),
-									 (Node *) (oaggref->target),
-									 expr);
-			}
-			break;
-
-		case T_Param:
-			break;
-
-		case T_Const:
-			break;
-
-		case T_GroupClause:
-			break;
-
-		case T_Expr:
-			{
-				Expr	   *this_expr = (Expr *) node;
-				Expr	   *orig_expr = (Expr *) orignode;
-
-				if (orig_expr == expr)
-				{
-					Const	   *ctrue;
-
-					if (expr->typeOid != BOOLOID)
-						elog(ERROR,
-							 "aggregate expression in qualification isn't of type bool");
-					ctrue = makeNode(Const);
-					ctrue->consttype = BOOLOID;
-					ctrue->constlen = 1;
-					ctrue->constisnull = FALSE;
-					ctrue->constvalue = (Datum) TRUE;
-					ctrue->constbyval = TRUE;
-
-					*nodePtr = (Node *) ctrue;
-				}
-				else
-					modifyAggrefDropQual(
-										 (Node **) (&(this_expr->args)),
-										 (Node *) (orig_expr->args),
-										 expr);
-			}
-			break;
-
-		case T_Iter:
-			{
-				Iter	   *iter = (Iter *) node;
-				Iter	   *oiter = (Iter *) orignode;
-
-				modifyAggrefDropQual(
-									 (Node **) (&(iter->iterexpr)),
-									 (Node *) (oiter->iterexpr),
-									 expr);
-			}
-			break;
-
-		case T_ArrayRef:
-			{
-				ArrayRef   *ref = (ArrayRef *) node;
-				ArrayRef   *oref = (ArrayRef *) orignode;
-
-				modifyAggrefDropQual(
-									 (Node **) (&(ref->refupperindexpr)),
-									 (Node *) (oref->refupperindexpr),
-									 expr);
-				modifyAggrefDropQual(
-									 (Node **) (&(ref->reflowerindexpr)),
-									 (Node *) (oref->reflowerindexpr),
-									 expr);
-				modifyAggrefDropQual(
-									 (Node **) (&(ref->refexpr)),
-									 (Node *) (oref->refexpr),
-									 expr);
-				modifyAggrefDropQual(
-									 (Node **) (&(ref->refassgnexpr)),
-									 (Node *) (oref->refassgnexpr),
-									 expr);
-			}
-			break;
-
-		case T_List:
-			{
-				List	   *l;
-				List	   *ol = (List *) orignode;
-				int			li = 0;
-
-				foreach(l, (List *) node)
-				{
-					modifyAggrefDropQual(
-										 (Node **) (&(lfirst(l))),
-										 (Node *) nth(li, ol),
-										 expr);
-					li++;
-				}
-			}
-			break;
-
-		case T_SubLink:
-			{
-				SubLink    *sub = (SubLink *) node;
-				SubLink    *osub = (SubLink *) orignode;
-
-				/* what about the lefthand? */
-				modifyAggrefDropQual(
-									 (Node **) (&(sub->subselect)),
-									 (Node *) (osub->subselect),
-									 expr);
-			}
-			break;
-
-		case T_Query:
-			{
-				Query	   *qry = (Query *) node;
-				Query	   *oqry = (Query *) orignode;
-
-				modifyAggrefDropQual(
-									 (Node **) (&(qry->qual)),
-									 (Node *) (oqry->qual),
-									 expr);
-
-				modifyAggrefDropQual(
-									 (Node **) (&(qry->havingQual)),
-									 (Node *) (oqry->havingQual),
-									 expr);
-			}
-			break;
-
-		default:
-			elog(NOTICE, "unknown node tag %d in modifyAggrefDropQual()", nodeTag(node));
-			elog(NOTICE, "Node is: %s", nodeToString(node));
-			break;
-
+		Expr	   *expr = (Expr *) node;
 
+		if (! IsA(expr, Expr) || expr->typeOid != BOOLOID)
+			elog(ERROR,
+				 "aggregate expression in qualification isn't of type bool");
+		return (Node *) makeConst(BOOLOID, 1, (Datum) true,
+								  false, true, false, false);
 	}
+	return expression_tree_mutator(node, modifyAggrefDropQual,
+								   (void *) targetNode);
 }
 
-
 /*
  * modifyAggrefMakeSublink -
  *	Create a sublink node for a qualification expression that
@@ -1029,7 +471,6 @@ modifyAggrefMakeSublink(Expr *origexp, Query *parsetree)
 {
 	SubLink    *sublink;
 	Query	   *subquery;
-	Node	   *subqual;
 	RangeTblEntry *rte;
 	Aggref	   *aggref;
 	Var		   *target;
@@ -1037,24 +478,22 @@ modifyAggrefMakeSublink(Expr *origexp, Query *parsetree)
 	Resdom	   *resdom;
 	Expr	   *exp = copyObject(origexp);
 
-	if (nodeTag(nth(0, exp->args)) == T_Aggref)
+	if (IsA(nth(0, exp->args), Aggref))
 	{
-		if (nodeTag(nth(1, exp->args)) == T_Aggref)
-			elog(ERROR, "rewrite: comparision of 2 aggregate columns not supported");
+		if (IsA(nth(1, exp->args), Aggref))
+			elog(ERROR, "rewrite: comparison of 2 aggregate columns not supported");
 		else
 			elog(ERROR, "rewrite: aggregate column of view must be at right side in qual");
+		/* XXX could try to commute operator, instead of failing */
 	}
 
 	aggref = (Aggref *) nth(1, exp->args);
 	target = (Var *) (aggref->target);
-	/* XXX bogus --- agg's target might not be a Var! */
+	if (! IsA(target, Var))
+		elog(ERROR, "rewrite: aggregates of views only allowed on simple variables for now");
 	rte = (RangeTblEntry *) nth(target->varno - 1, parsetree->rtable);
 
-	tle = makeNode(TargetEntry);
 	resdom = makeNode(Resdom);
-
-	aggref->usenulls = TRUE;	/* XXX safe for all aggs?? */
-
 	resdom->resno = 1;
 	resdom->restype = aggref->aggtype;
 	resdom->restypmod = -1;
@@ -1063,15 +502,14 @@ modifyAggrefMakeSublink(Expr *origexp, Query *parsetree)
 	resdom->reskeyop = 0;
 	resdom->resjunk = false;
 
+	tle = makeNode(TargetEntry);
 	tle->resdom = resdom;
-	tle->expr = (Node *) aggref;
-
-	subqual = copyObject(parsetree->qual);
-	modifyAggrefDropQual((Node **) &subqual, (Node *) parsetree->qual, origexp);
+	tle->expr = (Node *) aggref; /* note this is from the copied expr */
 
 	sublink = makeNode(SubLink);
 	sublink->subLinkType = EXPR_SUBLINK;
-	sublink->useor = FALSE;
+	sublink->useor = false;
+	/* note lefthand and oper are made from the copied expr */
 	sublink->lefthand = lcons(lfirst(exp->args), NIL);
 	sublink->oper = lcons(exp->oper, NIL);
 
@@ -1088,190 +526,65 @@ modifyAggrefMakeSublink(Expr *origexp, Query *parsetree)
 	subquery->unionall = FALSE;
 	subquery->uniqueFlag = NULL;
 	subquery->sortClause = NULL;
-	subquery->rtable = lappend(NIL, rte);
-	subquery->targetList = lappend(NIL, tle);
-	subquery->qual = subqual;
+	subquery->rtable = lcons(rte, NIL);
+	subquery->targetList = lcons(tle, NIL);
+	subquery->qual = modifyAggrefDropQual((Node *) parsetree->qual,
+										  (Node *) origexp);
 	subquery->groupClause = NIL;
 	subquery->havingQual = NULL;
 	subquery->hasAggs = TRUE;
-	subquery->hasSubLinks = FALSE;
-	subquery->unionClause = NULL;
-
-
-	modifyAggrefUplevel((Node *) sublink);
-
-	modifyAggrefChangeVarnodes((Node **) &(sublink->lefthand), target->varno,
-							   1, target->varlevelsup);
-	modifyAggrefChangeVarnodes((Node **) &(sublink->subselect), target->varno,
-							   1, target->varlevelsup);
-
-	return sublink;
-}
-
-
-/*
- * modifyAggrefQual -
- *	Search for qualification expressions that contain aggregate
- *	functions and substiture them by sublinks. These expressions
- *	originally come from qualifications that use aggregate columns
- *	of a view.
- */
-static void
-modifyAggrefQual(Node **nodePtr, Query *parsetree)
-{
-	Node	   *node = *nodePtr;
-
-	if (node == NULL)
-		return;
-
-	switch (nodeTag(node))
-	{
-		case T_Var:
-			break;
-
-		case T_Param:
-			break;
-
-		case T_Const:
-			break;
-
-		case T_GroupClause:
-			break;
-
-		case T_Expr:
-			{
-				Expr	   *exp = (Expr *) node;
-				SubLink    *sub;
-
-
-				if (length(exp->args) != 2)
-				{
-					modifyAggrefQual(
-									 (Node **) (&(exp->args)),
-									 parsetree);
-					break;
-				}
-
-				if (nodeTag(nth(0, exp->args)) != T_Aggref &&
-					nodeTag(nth(1, exp->args)) != T_Aggref)
-				{
-
-					modifyAggrefQual(
-									 (Node **) (&(exp->args)),
-									 parsetree);
-					break;
-				}
-
-				sub = modifyAggrefMakeSublink(exp,
-											  parsetree);
-
-				*nodePtr = (Node *) sub;
-				parsetree->hasSubLinks = TRUE;
-			}
-			break;
-
-		case T_CaseExpr:
-			{
-
-				/*
-				 * We're calling recursively, and this routine knows how
-				 * to handle lists so let it do the work to handle the
-				 * WHEN clauses...
-				 */
-				modifyAggrefQual(
-								 (Node **) (&(((CaseExpr *) node)->args)),
-								 parsetree);
-
-				modifyAggrefQual(
-						   (Node **) (&(((CaseExpr *) node)->defresult)),
-								 parsetree);
-			}
-			break;
-
-		case T_CaseWhen:
-			{
-				modifyAggrefQual(
-								 (Node **) (&(((CaseWhen *) node)->expr)),
-								 parsetree);
-
-				modifyAggrefQual(
-							  (Node **) (&(((CaseWhen *) node)->result)),
-								 parsetree);
-			}
-			break;
-
-		case T_Iter:
-			{
-				Iter	   *iter = (Iter *) node;
-
-				modifyAggrefQual(
-								 (Node **) (&(iter->iterexpr)),
-								 parsetree);
-			}
-			break;
-
-		case T_ArrayRef:
-			{
-				ArrayRef   *ref = (ArrayRef *) node;
-
-				modifyAggrefQual(
-								 (Node **) (&(ref->refupperindexpr)),
-								 parsetree);
-				modifyAggrefQual(
-								 (Node **) (&(ref->reflowerindexpr)),
-								 parsetree);
-				modifyAggrefQual(
-								 (Node **) (&(ref->refexpr)),
-								 parsetree);
-				modifyAggrefQual(
-								 (Node **) (&(ref->refassgnexpr)),
-								 parsetree);
-			}
-			break;
-
-		case T_List:
-			{
-				List	   *l;
-
-				foreach(l, (List *) node)
-					modifyAggrefQual(
-									 (Node **) (&(lfirst(l))),
-									 parsetree);
-			}
-			break;
-
-		case T_SubLink:
-			{
-				SubLink    *sub = (SubLink *) node;
-
-				/* lefthand ??? */
-				modifyAggrefQual(
-								 (Node **) (&(sub->subselect)),
-								 (Query *) (sub->subselect));
-			}
-			break;
-
-		case T_Query:
-			{
-				Query	   *qry = (Query *) node;
+	subquery->hasSubLinks = FALSE;
+	subquery->unionClause = NULL;
 
-				modifyAggrefQual(
-								 (Node **) (&(qry->qual)),
-								 parsetree);
+	modifyAggrefUplevel((Node *) subquery, NULL);
+	/*
+	 * Note: it might appear that we should be passing target->varlevelsup+1
+	 * here, since modifyAggrefUplevel has increased all the varlevelsup
+	 * values in the subquery.  However, target itself is a pointer to a
+	 * Var node in the subquery, so it's been incremented too!  What a kluge
+	 * this all is ... we need to make subquery RTEs so it can go away...
+	 */
+	modifyAggrefChangeVarnodes((Node *) subquery, target->varno,
+							   1, target->varlevelsup);
 
-				modifyAggrefQual(
-								 (Node **) (&(qry->havingQual)),
-								 parsetree);
-			}
-			break;
+	return sublink;
+}
 
-		default:
-			elog(NOTICE, "unknown node tag %d in modifyAggrefQual()", nodeTag(node));
-			elog(NOTICE, "Node is: %s", nodeToString(node));
-			break;
 
+/*
+ * modifyAggrefQual -
+ *	Search for qualification expressions that contain aggregate
+ *	functions and substitute them by sublinks. These expressions
+ *	originally come from qualifications that use aggregate columns
+ *	of a view.
+ *
+ *	The return value is a modified copy of the given expression tree.
+ */
+static Node *
+modifyAggrefQual(Node *node, Query *parsetree)
+{
+	if (node == NULL)
+		return NULL;
+	if (IsA(node, Expr))
+	{
+		Expr	   *expr = (Expr *) node;
 
+		if (length(expr->args) == 2 &&
+			(IsA(lfirst(expr->args), Aggref) ||
+			 IsA(lsecond(expr->args), Aggref)))
+		{
+			SubLink    *sub = modifyAggrefMakeSublink(expr,
+													  parsetree);
+			parsetree->hasSubLinks = true;
+			return (Node *) sub;
+		}
+		/* otherwise, fall through and copy the expr normally */
 	}
+	/* We do NOT recurse into subselects in this routine.  It's sufficient
+	 * to get rid of aggregates that are in the qual expression proper.
+	 */
+	return expression_tree_mutator(node, modifyAggrefQual,
+								   (void *) parsetree);
 }
 
 
@@ -1350,406 +663,141 @@ make_null(Oid type)
 }
 
 
-static void
-apply_RIR_adjust_sublevel(Node *node, int sublevels_up)
+/*
+ * apply_RIR_adjust_sublevel -
+ *	Set the varlevelsup field of all Var nodes in the given expression tree
+ *	to sublevels_up.  We do NOT recurse into subselects.
+ *
+ * NOTE: although this has the form of a walker, we cheat and modify the
+ * Var nodes in-place.  The given expression tree should have been copied
+ * earlier to ensure that no unwanted side-effects occur!
+ */
+static bool
+apply_RIR_adjust_sublevel_walker(Node *node, int *sublevels_up)
 {
 	if (node == NULL)
-		return;
-
-	switch (nodeTag(node))
+		return false;
+	if (IsA(node, Var))
 	{
-		case T_TargetEntry:
-			{
-				TargetEntry *tle = (TargetEntry *) node;
-
-				apply_RIR_adjust_sublevel(
-										  (Node *) (tle->expr),
-										  sublevels_up);
-			}
-			break;
-
-		case T_Aggref:
-			{
-				Aggref	   *aggref = (Aggref *) node;
-
-				apply_RIR_adjust_sublevel(
-										  (Node *) (aggref->target),
-										  sublevels_up);
-			}
-			break;
-
-		case T_GroupClause:
-			break;
-
-		case T_Expr:
-			{
-				Expr	   *exp = (Expr *) node;
-
-				apply_RIR_adjust_sublevel(
-										  (Node *) (exp->args),
-										  sublevels_up);
-			}
-			break;
-
-		case T_Iter:
-			{
-				Iter	   *iter = (Iter *) node;
-
-				apply_RIR_adjust_sublevel(
-										  (Node *) (iter->iterexpr),
-										  sublevels_up);
-			}
-			break;
-
-		case T_ArrayRef:
-			{
-				ArrayRef   *ref = (ArrayRef *) node;
-
-				apply_RIR_adjust_sublevel(
-										  (Node *) (ref->refupperindexpr),
-										  sublevels_up);
-
-				apply_RIR_adjust_sublevel(
-										  (Node *) (ref->reflowerindexpr),
-										  sublevels_up);
-
-				apply_RIR_adjust_sublevel(
-										  (Node *) (ref->refexpr),
-										  sublevels_up);
-
-				apply_RIR_adjust_sublevel(
-										  (Node *) (ref->refassgnexpr),
-										  sublevels_up);
-			}
-			break;
-
-		case T_Var:
-			{
-				Var		   *var = (Var *) node;
-
-				var->varlevelsup = sublevels_up;
-			}
-			break;
-
-		case T_Param:
-			break;
-
-		case T_Const:
-			break;
-
-		case T_List:
-			{
-				List	   *l;
-
-				foreach(l, (List *) node)
-				{
-					apply_RIR_adjust_sublevel(
-											  (Node *) lfirst(l),
-											  sublevels_up);
-				}
-			}
-			break;
-
-		case T_CaseExpr:
-			{
-				CaseExpr   *exp = (CaseExpr *) node;
-
-				apply_RIR_adjust_sublevel(
-										  (Node *) (exp->args),
-										  sublevels_up);
-
-				apply_RIR_adjust_sublevel(
-										  (Node *) (exp->defresult),
-										  sublevels_up);
-			}
-			break;
-
-		case T_CaseWhen:
-			{
-				CaseWhen   *exp = (CaseWhen *) node;
-
-				apply_RIR_adjust_sublevel(
-										  (Node *) (exp->expr),
-										  sublevels_up);
-
-				apply_RIR_adjust_sublevel(
-										  (Node *) (exp->result),
-										  sublevels_up);
-			}
-			break;
-
-		default:
-			elog(NOTICE, "unknown node tag %d in attribute_used()", nodeTag(node));
-			elog(NOTICE, "Node is: %s", nodeToString(node));
-			break;
-
+		Var		   *var = (Var *) node;
 
+		var->varlevelsup = *sublevels_up;
+		return false;
 	}
+	return expression_tree_walker(node, apply_RIR_adjust_sublevel_walker,
+								  (void *) sublevels_up);
 }
 
-
 static void
-apply_RIR_view(Node **nodePtr, int rt_index, RangeTblEntry *rte, List *tlist, int *modified, int sublevels_up)
+apply_RIR_adjust_sublevel(Node *node, int sublevels_up)
 {
-	Node	   *node = *nodePtr;
-
-	if (node == NULL)
-		return;
-
-	switch (nodeTag(node))
-	{
-		case T_TargetEntry:
-			{
-				TargetEntry *tle = (TargetEntry *) node;
-
-				apply_RIR_view(
-							   (Node **) (&(tle->expr)),
-							   rt_index,
-							   rte,
-							   tlist,
-							   modified,
-							   sublevels_up);
-			}
-			break;
-
-		case T_Aggref:
-			{
-				Aggref	   *aggref = (Aggref *) node;
-
-				apply_RIR_view(
-							   (Node **) (&(aggref->target)),
-							   rt_index,
-							   rte,
-							   tlist,
-							   modified,
-							   sublevels_up);
-			}
-			break;
-
-		case T_GroupClause:
-			break;
-
-		case T_Expr:
-			{
-				Expr	   *exp = (Expr *) node;
-
-				apply_RIR_view(
-							   (Node **) (&(exp->args)),
-							   rt_index,
-							   rte,
-							   tlist,
-							   modified,
-							   sublevels_up);
-			}
-			break;
-
-		case T_Iter:
-			{
-				Iter	   *iter = (Iter *) node;
-
-				apply_RIR_view(
-							   (Node **) (&(iter->iterexpr)),
-							   rt_index,
-							   rte,
-							   tlist,
-							   modified,
-							   sublevels_up);
-			}
-			break;
+	apply_RIR_adjust_sublevel_walker(node, &sublevels_up);
+}
 
-		case T_ArrayRef:
-			{
-				ArrayRef   *ref = (ArrayRef *) node;
-
-				apply_RIR_view(
-							   (Node **) (&(ref->refupperindexpr)),
-							   rt_index,
-							   rte,
-							   tlist,
-							   modified,
-							   sublevels_up);
-				apply_RIR_view(
-							   (Node **) (&(ref->reflowerindexpr)),
-							   rt_index,
-							   rte,
-							   tlist,
-							   modified,
-							   sublevels_up);
-				apply_RIR_view(
-							   (Node **) (&(ref->refexpr)),
-							   rt_index,
-							   rte,
-							   tlist,
-							   modified,
-							   sublevels_up);
-				apply_RIR_view(
-							   (Node **) (&(ref->refassgnexpr)),
-							   rt_index,
-							   rte,
-							   tlist,
-							   modified,
-							   sublevels_up);
-			}
-			break;
 
-		case T_Var:
-			{
-				Var		   *var = (Var *) node;
+/*
+ * apply_RIR_view
+ *	Replace Vars matching a given RT index with copies of TL expressions.
+ */
 
-				if (var->varlevelsup == sublevels_up &&
-					var->varno == rt_index)
-				{
-					Node	   *exp;
-
-					if (var->varattno < 0)
-						elog(ERROR, "system column %s not available - %s is a view", get_attname(rte->relid, var->varattno), rte->relname);
-					exp = FindMatchingTLEntry(
-											  tlist,
-											  get_attname(rte->relid,
-														  var->varattno));
-
-					if (exp == NULL)
-					{
-						*nodePtr = make_null(var->vartype);
-						return;
-					}
-
-					exp = copyObject(exp);
-					if (var->varlevelsup > 0)
-						apply_RIR_adjust_sublevel(exp, var->varlevelsup);
-					*nodePtr = exp;
-					*modified = TRUE;
-				}
-			}
-			break;
+typedef struct {
+	int				rt_index;
+	int				sublevels_up;
+	RangeTblEntry  *rte;
+	List		   *tlist;
+	int			   *modified;
+} apply_RIR_view_context;
 
-		case T_Param:
-			break;
+static Node *
+apply_RIR_view_mutator(Node *node,
+					   apply_RIR_view_context *context)
+{
+	if (node == NULL)
+		return NULL;
+	if (IsA(node, Var))
+	{
+		Var		   *var = (Var *) node;
 
-		case T_Const:
-			break;
+		if (var->varlevelsup == context->sublevels_up &&
+			var->varno == context->rt_index)
+		{
+			Node	   *expr;
 
-		case T_List:
-			{
-				List	   *l;
-
-				foreach(l, (List *) node)
-					apply_RIR_view(
-								   (Node **) (&(lfirst(l))),
-								   rt_index,
-								   rte,
-								   tlist,
-								   modified,
-								   sublevels_up);
-			}
-			break;
+			if (var->varattno < 0)
+				elog(ERROR, "system column %s not available - %s is a view",
+					 get_attname(context->rte->relid, var->varattno),
+					 context->rte->relname);
 
-		case T_SubLink:
+			expr = FindMatchingTLEntry(context->tlist,
+									   get_attname(context->rte->relid,
+												   var->varattno));
+			if (expr == NULL)
 			{
-				SubLink    *sub = (SubLink *) node;
-
-				apply_RIR_view(
-							   (Node **) (&(sub->lefthand)),
-							   rt_index,
-							   rte,
-							   tlist,
-							   modified,
-							   sublevels_up);
-
-				apply_RIR_view(
-							   (Node **) (&(sub->subselect)),
-							   rt_index,
-							   rte,
-							   tlist,
-							   modified,
-							   sublevels_up + 1);
+				/* XXX shouldn't this be an error condition? */
+				return make_null(var->vartype);
 			}
-			break;
 
-		case T_Query:
-			{
-				Query	   *qry = (Query *) node;
-
-				apply_RIR_view(
-							   (Node **) (&(qry->targetList)),
-							   rt_index,
-							   rte,
-							   tlist,
-							   modified,
-							   sublevels_up);
-
-				apply_RIR_view(
-							   (Node **) (&(qry->qual)),
-							   rt_index,
-							   rte,
-							   tlist,
-							   modified,
-							   sublevels_up);
-
-				apply_RIR_view(
-							   (Node **) (&(qry->havingQual)),
-							   rt_index,
-							   rte,
-							   tlist,
-							   modified,
-							   sublevels_up);
-			}
-			break;
+			expr = copyObject(expr);
+			if (var->varlevelsup > 0)
+				apply_RIR_adjust_sublevel(expr, var->varlevelsup);
+			*(context->modified) = true;
+			return (Node *) expr;
+		}
+		/* otherwise fall through to copy the var normally */
+	}
+	/*
+	 * Since expression_tree_mutator won't touch subselects, we have to
+	 * handle them specially.
+	 */
+	if (IsA(node, SubLink))
+	{
+		SubLink   *sublink = (SubLink *) node;
+		SubLink   *newnode;
+
+		FLATCOPY(newnode, sublink, SubLink);
+		MUTATE(newnode->lefthand, sublink->lefthand, List *,
+			   apply_RIR_view_mutator, context);
+		context->sublevels_up++;
+		MUTATE(newnode->subselect, sublink->subselect, Node *,
+			   apply_RIR_view_mutator, context);
+		context->sublevels_up--;
+		return (Node *) newnode;
+	}
+	if (IsA(node, Query))
+	{
+		Query  *query = (Query *) node;
+		Query  *newnode;
+
+		FLATCOPY(newnode, query, Query);
+		MUTATE(newnode->targetList, query->targetList, List *,
+			   apply_RIR_view_mutator, context);
+		MUTATE(newnode->qual, query->qual, Node *,
+			   apply_RIR_view_mutator, context);
+		MUTATE(newnode->havingQual, query->havingQual, Node *,
+			   apply_RIR_view_mutator, context);
+		return (Node *) newnode;
+	}
+	return expression_tree_mutator(node, apply_RIR_view_mutator,
+								   (void *) context);
+}
 
-		case T_CaseExpr:
-			{
-				CaseExpr   *exp = (CaseExpr *) node;
-
-				apply_RIR_view(
-							   (Node **) (&(exp->args)),
-							   rt_index,
-							   rte,
-							   tlist,
-							   modified,
-							   sublevels_up);
-
-				apply_RIR_view(
-							   (Node **) (&(exp->defresult)),
-							   rt_index,
-							   rte,
-							   tlist,
-							   modified,
-							   sublevels_up);
-			}
-			break;
+static Node *
+apply_RIR_view(Node *node, int rt_index, RangeTblEntry *rte, List *tlist,
+			   int *modified, int sublevels_up)
+{
+	apply_RIR_view_context	context;
 
-		case T_CaseWhen:
-			{
-				CaseWhen   *exp = (CaseWhen *) node;
-
-				apply_RIR_view(
-							   (Node **) (&(exp->expr)),
-							   rt_index,
-							   rte,
-							   tlist,
-							   modified,
-							   sublevels_up);
-
-				apply_RIR_view(
-							   (Node **) (&(exp->result)),
-							   rt_index,
-							   rte,
-							   tlist,
-							   modified,
-							   sublevels_up);
-			}
-			break;
+	context.rt_index = rt_index;
+	context.sublevels_up = sublevels_up;
+	context.rte = rte;
+	context.tlist = tlist;
+	context.modified = modified;
 
-		default:
-			elog(NOTICE, "unknown node tag %d in apply_RIR_view()", nodeTag(node));
-			elog(NOTICE, "Node is: %s", nodeToString(node));
-			break;
-	}
+	return apply_RIR_view_mutator(node, &context);
 }
 
-extern void CheckSelectForUpdate(Query *rule_action);	/* in analyze.c */
 
-static void
+static Query *
 ApplyRetrieveRule(Query *parsetree,
 				  RewriteRule *rule,
 				  int rt_index,
@@ -1764,7 +812,7 @@ ApplyRetrieveRule(Query *parsetree,
 			   *l;
 	int			nothing,
 				rt_length;
-	int			badsql = FALSE;
+	int			badsql = false;
 
 	rule_qual = rule->qual;
 	if (rule->actions)
@@ -1773,7 +821,7 @@ ApplyRetrieveRule(Query *parsetree,
 										 * rules with more than one
 										 * action? -ay */
 
-			return;
+			return parsetree;
 		rule_action = copyObject(lfirst(rule->actions));
 		nothing = FALSE;
 	}
@@ -1817,6 +865,7 @@ ApplyRetrieveRule(Query *parsetree,
 		 * them.
 		 */
 		((RowMark *) lfirst(l))->info &= ~ROW_MARK_FOR_UPDATE;
+
 		foreach(l2, rule_action->rtable)
 		{
 
@@ -1847,12 +896,16 @@ ApplyRetrieveRule(Query *parsetree,
 
 	if (relation_level)
 	{
-		apply_RIR_view((Node **) &parsetree, rt_index,
-					   (RangeTblEntry *) nth(rt_index - 1, rtable),
-					   rule_action->targetList, modified, 0);
-		apply_RIR_view((Node **) &rule_action, rt_index,
-					   (RangeTblEntry *) nth(rt_index - 1, rtable),
-					   rule_action->targetList, modified, 0);
+		RangeTblEntry  *rte = (RangeTblEntry *) nth(rt_index - 1, rtable);
+
+		parsetree = (Query *) apply_RIR_view((Node *) parsetree,
+											 rt_index, rte,
+											 rule_action->targetList,
+											 modified, 0);
+		rule_action = (Query *) apply_RIR_view((Node *) rule_action,
+											   rt_index, rte,
+											   rule_action->targetList,
+											   modified, 0);
 	}
 	else
 	{
@@ -1868,153 +921,59 @@ ApplyRetrieveRule(Query *parsetree,
 		parsetree->hasAggs = (rule_action->hasAggs || parsetree->hasAggs);
 		parsetree->hasSubLinks = (rule_action->hasSubLinks || parsetree->hasSubLinks);
 	}
+
+	return parsetree;
 }
 
 
-static void
-fireRIRonSubselect(Node *node)
+/*
+ * fireRIRonSubselect -
+ *	Apply fireRIRrules() to each subselect found in the given tree.
+ *
+ * NOTE: although this has the form of a walker, we cheat and modify the
+ * SubLink nodes in-place.  It is caller's responsibility to ensure that
+ * no unwanted side-effects occur!
+ */
+static bool
+fireRIRonSubselect(Node *node, void *context)
 {
 	if (node == NULL)
-		return;
-
-	switch (nodeTag(node))
+		return false;
+	if (IsA(node, SubLink))
 	{
-		case T_TargetEntry:
-			{
-				TargetEntry *tle = (TargetEntry *) node;
-
-				fireRIRonSubselect(
-								   (Node *) (tle->expr));
-			}
-			break;
-
-		case T_Aggref:
-			{
-				Aggref	   *aggref = (Aggref *) node;
-
-				fireRIRonSubselect(
-								   (Node *) (aggref->target));
-			}
-			break;
-
-		case T_GroupClause:
-			break;
-
-		case T_Expr:
-			{
-				Expr	   *exp = (Expr *) node;
-
-				fireRIRonSubselect(
-								   (Node *) (exp->args));
-			}
-			break;
-
-		case T_Iter:
-			{
-				Iter	   *iter = (Iter *) node;
-
-				fireRIRonSubselect(
-								   (Node *) (iter->iterexpr));
-			}
-			break;
-
-		case T_ArrayRef:
-			{
-				ArrayRef   *ref = (ArrayRef *) node;
-
-				fireRIRonSubselect(
-								   (Node *) (ref->refupperindexpr));
-				fireRIRonSubselect(
-								   (Node *) (ref->reflowerindexpr));
-				fireRIRonSubselect(
-								   (Node *) (ref->refexpr));
-				fireRIRonSubselect(
-								   (Node *) (ref->refassgnexpr));
-			}
-			break;
-
-		case T_Var:
-			break;
-
-		case T_Param:
-			break;
-
-		case T_Const:
-			break;
-
-		case T_List:
-			{
-				List	   *l;
-
-				foreach(l, (List *) node)
-					fireRIRonSubselect(
-									   (Node *) (lfirst(l)));
-			}
-			break;
-
-		case T_SubLink:
-			{
-				SubLink    *sub = (SubLink *) node;
-				Query	   *qry;
-
-				fireRIRonSubselect(
-								   (Node *) (sub->lefthand));
-
-				qry = fireRIRrules((Query *) (sub->subselect));
-
-				fireRIRonSubselect(
-								   (Node *) qry);
-
-				sub->subselect = (Node *) qry;
-			}
-			break;
-
-		case T_CaseExpr:
-			{
-				CaseExpr   *exp = (CaseExpr *) node;
-
-				fireRIRonSubselect(
-								   (Node *) (exp->args));
-
-				fireRIRonSubselect(
-								   (Node *) (exp->defresult));
-			}
-			break;
-
-		case T_CaseWhen:
-			{
-				CaseWhen   *exp = (CaseWhen *) node;
-
-				fireRIRonSubselect(
-								   (Node *) (exp->expr));
-
-				fireRIRonSubselect(
-								   (Node *) (exp->result));
-			}
-			break;
-
-		case T_Query:
-			{
-				Query	   *qry = (Query *) node;
-
-				fireRIRonSubselect(
-								   (Node *) (qry->targetList));
-
-				fireRIRonSubselect(
-								   (Node *) (qry->qual));
-
-				fireRIRonSubselect(
-								   (Node *) (qry->havingQual));
-			}
-			break;
-
-		default:
-			elog(NOTICE, "unknown node tag %d in fireRIRonSubselect()", nodeTag(node));
-			elog(NOTICE, "Node is: %s", nodeToString(node));
-			break;
-
-
+		/*
+		 * Standard expression_tree_walker will not recurse into subselect,
+		 * but here we must do so.
+		 */
+		SubLink    *sub = (SubLink *) node;
+		Query	   *qry;
+
+		/* Process lefthand args */
+		if (fireRIRonSubselect((Node *) (sub->lefthand), context))
+			return true;
+		/* Do what we came for */
+		qry = fireRIRrules((Query *) (sub->subselect));
+		sub->subselect = (Node *) qry;
+		/* Must recurse to handle any sub-subselects! */
+		if (fireRIRonSubselect((Node *) qry, context))
+			return true;
+		return false;
+	}
+	if (IsA(node, Query))
+	{
+		/* Reach here after recursing down into subselect above... */
+		Query	   *qry = (Query *) node;
+
+		if (fireRIRonSubselect((Node *) (qry->targetList), context))
+			return true;
+		if (fireRIRonSubselect((Node *) (qry->qual), context))
+			return true;
+		if (fireRIRonSubselect((Node *) (qry->havingQual), context))
+			return true;
+		return false;
 	}
+	return expression_tree_walker(node, fireRIRonSubselect,
+								  (void *) context);
 }
 
 
@@ -2032,7 +991,7 @@ fireRIRrules(Query *parsetree)
 	RuleLock   *rules;
 	RewriteRule *rule;
 	RewriteRule RIRonly;
-	int			modified;
+	int			modified = false;
 	int			i;
 	List	   *l;
 
@@ -2104,19 +1063,20 @@ fireRIRrules(Query *parsetree)
 			RIRonly.qual = rule->qual;
 			RIRonly.actions = rule->actions;
 
-			ApplyRetrieveRule(parsetree,
-							  &RIRonly,
-							  rt_index,
-							  RIRonly.attrno == -1,
-							  rel,
-							  &modified);
+			parsetree = ApplyRetrieveRule(parsetree,
+										  &RIRonly,
+										  rt_index,
+										  RIRonly.attrno == -1,
+										  rel,
+										  &modified);
 		}
 
 		heap_close(rel, AccessShareLock);
 	}
 
-	fireRIRonSubselect((Node *) parsetree);
-	modifyAggrefQual((Node **) &(parsetree->qual), parsetree);
+	fireRIRonSubselect((Node *) parsetree, NULL);
+
+	parsetree->qual = modifyAggrefQual(parsetree->qual, parsetree);
 
 	return parsetree;
 }
@@ -2639,9 +1599,9 @@ BasicQueryRewrite(Query *parsetree)
 		 */
 		if (query->hasAggs)
 			query->hasAggs = checkQueryHasAggs((Node *) (query->targetList))
-				| checkQueryHasAggs((Node *) (query->havingQual));
+				|| checkQueryHasAggs((Node *) (query->havingQual));
 		query->hasSubLinks = checkQueryHasSubLink((Node *) (query->qual))
-			| checkQueryHasSubLink((Node *) (query->havingQual));
+			|| checkQueryHasSubLink((Node *) (query->havingQual));
 		results = lappend(results, query);
 	}
 	return results;
diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c
index 16b31eae84d..3f9b4da2698 100644
--- a/src/backend/rewrite/rewriteManip.c
+++ b/src/backend/rewrite/rewriteManip.c
@@ -6,7 +6,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteManip.c,v 1.40 1999/08/25 23:21:43 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteManip.c,v 1.41 1999/10/01 04:08:24 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -19,410 +19,182 @@
 #include "utils/builtins.h"
 #include "utils/lsyscache.h"
 
-static void ResolveNew(RewriteInfo *info, List *targetlist,
-		   Node **node, int sublevels_up);
 
+/* macros borrowed from expression_tree_mutator */
 
-/*
- * OffsetVarnodes -
- */
-void
-OffsetVarNodes(Node *node, int offset, int sublevels_up)
-{
-	if (node == NULL)
-		return;
-
-	switch (nodeTag(node))
-	{
-		case T_TargetEntry:
-			{
-				TargetEntry *tle = (TargetEntry *) node;
-
-				OffsetVarNodes(
-							   (Node *) (tle->expr),
-							   offset,
-							   sublevels_up);
-			}
-			break;
-
-		case T_Aggref:
-			{
-				Aggref	   *aggref = (Aggref *) node;
-
-				OffsetVarNodes(
-							   (Node *) (aggref->target),
-							   offset,
-							   sublevels_up);
-			}
-			break;
-
-		case T_GroupClause:
-			break;
-
-		case T_Expr:
-			{
-				Expr	   *exp = (Expr *) node;
-
-				OffsetVarNodes(
-							   (Node *) (exp->args),
-							   offset,
-							   sublevels_up);
-			}
-			break;
-
-		case T_Iter:
-			{
-				Iter	   *iter = (Iter *) node;
-
-				OffsetVarNodes(
-							   (Node *) (iter->iterexpr),
-							   offset,
-							   sublevels_up);
-			}
-			break;
-
-		case T_ArrayRef:
-			{
-				ArrayRef   *ref = (ArrayRef *) node;
-
-				OffsetVarNodes(
-							   (Node *) (ref->refupperindexpr),
-							   offset,
-							   sublevels_up);
-				OffsetVarNodes(
-							   (Node *) (ref->reflowerindexpr),
-							   offset,
-							   sublevels_up);
-				OffsetVarNodes(
-							   (Node *) (ref->refexpr),
-							   offset,
-							   sublevels_up);
-				OffsetVarNodes(
-							   (Node *) (ref->refassgnexpr),
-							   offset,
-							   sublevels_up);
-			}
-			break;
-
-		case T_Var:
-			{
-				Var		   *var = (Var *) node;
-
-				if (var->varlevelsup == sublevels_up)
-				{
-					var->varno += offset;
-					var->varnoold += offset;
-				}
-			}
-			break;
-
-		case T_Param:
-			break;
-
-		case T_Const:
-			break;
+#define FLATCOPY(newnode, node, nodetype)  \
+	( (newnode) = makeNode(nodetype), \
+	  memcpy((newnode), (node), sizeof(nodetype)) )
 
-		case T_List:
-			{
-				List	   *l;
-
-				foreach(l, (List *) node)
-					OffsetVarNodes(
-								   (Node *) lfirst(l),
-								   offset,
-								   sublevels_up);
-			}
-			break;
-
-		case T_SubLink:
-			{
-				SubLink    *sub = (SubLink *) node;
-
-				OffsetVarNodes(
-							   (Node *) (sub->lefthand),
-							   offset,
-							   sublevels_up);
-
-				OffsetVarNodes(
-							   (Node *) (sub->subselect),
-							   offset,
-							   sublevels_up + 1);
-			}
-			break;
-
-		case T_Query:
-			{
-				Query	   *qry = (Query *) node;
-
-				OffsetVarNodes(
-							   (Node *) (qry->targetList),
-							   offset,
-							   sublevels_up);
-
-				OffsetVarNodes(
-							   (Node *) (qry->qual),
-							   offset,
-							   sublevels_up);
-
-				OffsetVarNodes(
-							   (Node *) (qry->havingQual),
-							   offset,
-							   sublevels_up);
-			}
-			break;
-
-		case T_CaseExpr:
-			{
-				CaseExpr   *exp = (CaseExpr *) node;
-
-				OffsetVarNodes(
-							   (Node *) (exp->args),
-							   offset,
-							   sublevels_up);
-
-				OffsetVarNodes(
-							   (Node *) (exp->defresult),
-							   offset,
-							   sublevels_up);
-			}
-			break;
-
-		case T_CaseWhen:
-			{
-				CaseWhen   *exp = (CaseWhen *) node;
+#define MUTATE(newfield, oldfield, fieldtype, mutator, context)  \
+		( (newfield) = (fieldtype) mutator((Node *) (oldfield), (context)) )
 
-				OffsetVarNodes(
-							   (Node *) (exp->expr),
-							   offset,
-							   sublevels_up);
 
-				OffsetVarNodes(
-							   (Node *) (exp->result),
-							   offset,
-							   sublevels_up);
-			}
-			break;
+/*
+ * OffsetVarNodes - adjust Vars when appending one query's RT to another
+ *
+ * Find all Var nodes in the given tree with varlevelsup == sublevels_up,
+ * and increment their varno fields (rangetable indexes) by 'offset'.
+ * The varnoold fields are adjusted similarly.
+ *
+ * NOTE: although this has the form of a walker, we cheat and modify the
+ * Var nodes in-place.  The given expression tree should have been copied
+ * earlier to ensure that no unwanted side-effects occur!
+ */
 
-		default:
-			elog(NOTICE, "unknown node tag %d in OffsetVarNodes()", nodeTag(node));
-			elog(NOTICE, "Node is: %s", nodeToString(node));
-			break;
+typedef struct {
+	int			offset;
+	int			sublevels_up;
+} OffsetVarNodes_context;
 
+static bool
+OffsetVarNodes_walker(Node *node, OffsetVarNodes_context *context)
+{
+	if (node == NULL)
+		return false;
+	if (IsA(node, Var))
+	{
+		Var		   *var = (Var *) node;
 
+		if (var->varlevelsup == context->sublevels_up)
+		{
+			var->varno += context->offset;
+			var->varnoold += context->offset;
+		}
+		return false;
 	}
+	if (IsA(node, SubLink))
+	{
+		/*
+		 * Standard expression_tree_walker will not recurse into subselect,
+		 * but here we must do so.
+		 */
+		SubLink    *sub = (SubLink *) node;
+
+		if (OffsetVarNodes_walker((Node *) (sub->lefthand),
+								  context))
+			return true;
+		OffsetVarNodes((Node *) (sub->subselect),
+					   context->offset,
+					   context->sublevels_up + 1);
+		return false;
+	}
+	if (IsA(node, Query))
+	{
+		/* Reach here after recursing down into subselect above... */
+		Query	   *qry = (Query *) node;
+
+		if (OffsetVarNodes_walker((Node *) (qry->targetList),
+								  context))
+			return true;
+		if (OffsetVarNodes_walker((Node *) (qry->qual),
+								  context))
+			return true;
+		if (OffsetVarNodes_walker((Node *) (qry->havingQual),
+								  context))
+			return true;
+		return false;
+	}
+	return expression_tree_walker(node, OffsetVarNodes_walker,
+								  (void *) context);
 }
 
-
-/*
- * ChangeVarNodes -
- */
 void
-ChangeVarNodes(Node *node, int rt_index, int new_index, int sublevels_up)
+OffsetVarNodes(Node *node, int offset, int sublevels_up)
 {
-	if (node == NULL)
-		return;
-
-	switch (nodeTag(node))
-	{
-		case T_TargetEntry:
-			{
-				TargetEntry *tle = (TargetEntry *) node;
-
-				ChangeVarNodes(
-							   (Node *) (tle->expr),
-							   rt_index,
-							   new_index,
-							   sublevels_up);
-			}
-			break;
-
-		case T_Aggref:
-			{
-				Aggref	   *aggref = (Aggref *) node;
-
-				ChangeVarNodes(
-							   (Node *) (aggref->target),
-							   rt_index,
-							   new_index,
-							   sublevels_up);
-			}
-			break;
-
-		case T_GroupClause:
-			break;
-
-		case T_Expr:
-			{
-				Expr	   *exp = (Expr *) node;
-
-				ChangeVarNodes(
-							   (Node *) (exp->args),
-							   rt_index,
-							   new_index,
-							   sublevels_up);
-			}
-			break;
-
-		case T_Iter:
-			{
-				Iter	   *iter = (Iter *) node;
-
-				ChangeVarNodes(
-							   (Node *) (iter->iterexpr),
-							   rt_index,
-							   new_index,
-							   sublevels_up);
-			}
-			break;
-
-		case T_ArrayRef:
-			{
-				ArrayRef   *ref = (ArrayRef *) node;
-
-				ChangeVarNodes(
-							   (Node *) (ref->refupperindexpr),
-							   rt_index,
-							   new_index,
-							   sublevels_up);
-				ChangeVarNodes(
-							   (Node *) (ref->reflowerindexpr),
-							   rt_index,
-							   new_index,
-							   sublevels_up);
-				ChangeVarNodes(
-							   (Node *) (ref->refexpr),
-							   rt_index,
-							   new_index,
-							   sublevels_up);
-				ChangeVarNodes(
-							   (Node *) (ref->refassgnexpr),
-							   rt_index,
-							   new_index,
-							   sublevels_up);
-			}
-			break;
-
-		case T_Var:
-			{
-				Var		   *var = (Var *) node;
-
-				if (var->varlevelsup == sublevels_up &&
-					var->varno == rt_index)
-				{
-					var->varno = new_index;
-					var->varnoold = new_index;
-				}
-			}
-			break;
-
-		case T_Param:
-			break;
-
-		case T_Const:
-			break;
-
-		case T_List:
-			{
-				List	   *l;
-
-				foreach(l, (List *) node)
-					ChangeVarNodes(
-								   (Node *) lfirst(l),
-								   rt_index,
-								   new_index,
-								   sublevels_up);
-			}
-			break;
+	OffsetVarNodes_context context;
 
-		case T_SubLink:
-			{
-				SubLink    *sub = (SubLink *) node;
-
-				ChangeVarNodes(
-							   (Node *) (sub->lefthand),
-							   rt_index,
-							   new_index,
-							   sublevels_up);
-
-				ChangeVarNodes(
-							   (Node *) (sub->subselect),
-							   rt_index,
-							   new_index,
-							   sublevels_up + 1);
-			}
-			break;
-
-		case T_Query:
-			{
-				Query	   *qry = (Query *) node;
-
-				ChangeVarNodes(
-							   (Node *) (qry->targetList),
-							   rt_index,
-							   new_index,
-							   sublevels_up);
-
-				ChangeVarNodes(
-							   (Node *) (qry->qual),
-							   rt_index,
-							   new_index,
-							   sublevels_up);
-
-				ChangeVarNodes(
-							   (Node *) (qry->havingQual),
-							   rt_index,
-							   new_index,
-							   sublevels_up);
-			}
-			break;
-
-		case T_CaseExpr:
-			{
-				CaseExpr   *exp = (CaseExpr *) node;
-
-				ChangeVarNodes(
-							   (Node *) (exp->args),
-							   rt_index,
-							   new_index,
-							   sublevels_up);
-
-				ChangeVarNodes(
-							   (Node *) (exp->defresult),
-							   rt_index,
-							   new_index,
-							   sublevels_up);
-			}
-			break;
+	context.offset = offset;
+	context.sublevels_up = sublevels_up;
+	OffsetVarNodes_walker(node, &context);
+}
 
-		case T_CaseWhen:
-			{
-				CaseWhen   *exp = (CaseWhen *) node;
-
-				ChangeVarNodes(
-							   (Node *) (exp->expr),
-							   rt_index,
-							   new_index,
-							   sublevels_up);
-
-				ChangeVarNodes(
-							   (Node *) (exp->result),
-							   rt_index,
-							   new_index,
-							   sublevels_up);
-			}
-			break;
+/*
+ * ChangeVarNodes - adjust Var nodes for a specific change of RT index
+ *
+ * Find all Var nodes in the given tree belonging to a specific relation
+ * (identified by sublevels_up and rt_index), and change their varno fields
+ * to 'new_index'.  The varnoold fields are changed too.
+ *
+ * NOTE: although this has the form of a walker, we cheat and modify the
+ * Var nodes in-place.  The given expression tree should have been copied
+ * earlier to ensure that no unwanted side-effects occur!
+ */
 
-		default:
-			elog(NOTICE, "unknown node tag %d in ChangeVarNodes()", nodeTag(node));
-			elog(NOTICE, "Node is: %s", nodeToString(node));
-			break;
+typedef struct {
+	int			rt_index;
+	int			new_index;
+	int			sublevels_up;
+} ChangeVarNodes_context;
 
+static bool
+ChangeVarNodes_walker(Node *node, ChangeVarNodes_context *context)
+{
+	if (node == NULL)
+		return false;
+	if (IsA(node, Var))
+	{
+		Var		   *var = (Var *) node;
 
+		if (var->varlevelsup == context->sublevels_up &&
+			var->varno == context->rt_index)
+		{
+			var->varno = context->new_index;
+			var->varnoold = context->new_index;
+		}
+		return false;
+	}
+	if (IsA(node, SubLink))
+	{
+		/*
+		 * Standard expression_tree_walker will not recurse into subselect,
+		 * but here we must do so.
+		 */
+		SubLink    *sub = (SubLink *) node;
+
+		if (ChangeVarNodes_walker((Node *) (sub->lefthand),
+								  context))
+			return true;
+		ChangeVarNodes((Node *) (sub->subselect),
+					   context->rt_index,
+					   context->new_index,
+					   context->sublevels_up + 1);
+		return false;
 	}
+	if (IsA(node, Query))
+	{
+		/* Reach here after recursing down into subselect above... */
+		Query	   *qry = (Query *) node;
+
+		if (ChangeVarNodes_walker((Node *) (qry->targetList),
+								  context))
+			return true;
+		if (ChangeVarNodes_walker((Node *) (qry->qual),
+								  context))
+			return true;
+		if (ChangeVarNodes_walker((Node *) (qry->havingQual),
+								  context))
+			return true;
+		return false;
+	}
+	return expression_tree_walker(node, ChangeVarNodes_walker,
+								  (void *) context);
 }
 
+void
+ChangeVarNodes(Node *node, int rt_index, int new_index, int sublevels_up)
+{
+	ChangeVarNodes_context context;
 
+	context.rt_index = rt_index;
+	context.new_index = new_index;
+	context.sublevels_up = sublevels_up;
+	ChangeVarNodes_walker(node, &context);
+}
 
+/*
+ * Add the given qualifier condition to the query's WHERE clause
+ */
 void
 AddQual(Query *parsetree, Node *qual)
 {
@@ -433,18 +205,19 @@ AddQual(Query *parsetree, Node *qual)
 		return;
 
 	/* INTERSECT want's the original, but we need to copy - Jan */
-	/* copy = qual; */
 	copy = copyObject(qual);
 
 	old = parsetree->qual;
 	if (old == NULL)
 		parsetree->qual = copy;
 	else
-		parsetree->qual = (Node *) make_andclause(makeList(parsetree->qual, copy, -1));
+		parsetree->qual = (Node *) make_andclause(makeList(old, copy, -1));
 }
 
-/* Adds the given havingQual to the one already contained in the parsetree just as
- * AddQual does for the normal 'where' qual */
+/*
+ * Add the given havingQual to the one already contained in the parsetree
+ * just as AddQual does for the normal 'where' qual
+ */
 void
 AddHavingQual(Query *parsetree, Node *havingQual)
 {
@@ -455,46 +228,43 @@ AddHavingQual(Query *parsetree, Node *havingQual)
 		return;
 
 	/* INTERSECT want's the original, but we need to copy - Jan */
-	/* copy = havingQual; */
 	copy = copyObject(havingQual);
 
 	old = parsetree->havingQual;
 	if (old == NULL)
 		parsetree->havingQual = copy;
 	else
-		parsetree->havingQual = (Node *) make_andclause(makeList(parsetree->havingQual, copy, -1));
+		parsetree->havingQual = (Node *) make_andclause(makeList(old, copy, -1));
 }
 
 #ifdef NOT_USED
 void
 AddNotHavingQual(Query *parsetree, Node *havingQual)
 {
-	Node	   *copy;
+	Node	   *notqual;
 
 	if (havingQual == NULL)
 		return;
 
-	/* INTERSECT want's the original, but we need to copy - Jan */
-	/* copy = (Node *) make_notclause((Expr *)havingQual); */
-	copy = (Node *) make_notclause((Expr *) copyObject(havingQual));
+	/* Need not copy input qual, because AddHavingQual will... */
+	notqual = (Node *) make_notclause((Expr *) havingQual);
 
-	AddHavingQual(parsetree, copy);
+	AddHavingQual(parsetree, notqual);
 }
 #endif
 
 void
 AddNotQual(Query *parsetree, Node *qual)
 {
-	Node	   *copy;
+	Node	   *notqual;
 
 	if (qual == NULL)
 		return;
 
-	/* INTERSECT want's the original, but we need to copy - Jan */
-	/* copy = (Node *) make_notclause((Expr *)qual); */
-	copy = (Node *) make_notclause((Expr *) copyObject(qual));
+	/* Need not copy input qual, because AddQual will... */
+	notqual = (Node *) make_notclause((Expr *) qual);
 
-	AddQual(parsetree, copy);
+	AddQual(parsetree, notqual);
 }
 
 
@@ -583,6 +353,7 @@ FixResdomTypes(List *tlist)
 
 #endif
 
+/* Find a targetlist entry by resno */
 static Node *
 FindMatchingNew(List *tlist, int attno)
 {
@@ -598,6 +369,7 @@ FindMatchingNew(List *tlist, int attno)
 	return NULL;
 }
 
+/* Find a targetlist entry by resname */
 static Node *
 FindMatchingTLEntry(List *tlist, char *e_attname)
 {
@@ -615,464 +387,270 @@ FindMatchingTLEntry(List *tlist, char *e_attname)
 	return NULL;
 }
 
-static void
-ResolveNew(RewriteInfo *info, List *targetlist, Node **nodePtr,
-		   int sublevels_up)
-{
-	Node	   *node = *nodePtr;
-
-	if (node == NULL)
-		return;
-
-	switch (nodeTag(node))
-	{
-		case T_TargetEntry:
-			ResolveNew(info, targetlist, &((TargetEntry *) node)->expr,
-					   sublevels_up);
-			break;
-		case T_Aggref:
-			ResolveNew(info, targetlist, &((Aggref *) node)->target,
-					   sublevels_up);
-			break;
-		case T_Expr:
-			ResolveNew(info, targetlist, (Node **) (&(((Expr *) node)->args)),
-					   sublevels_up);
-			break;
-		case T_Iter:
-			ResolveNew(info, targetlist, (Node **) (&(((Iter *) node)->iterexpr)),
-					   sublevels_up);
-			break;
-		case T_ArrayRef:
-			ResolveNew(info, targetlist, (Node **) (&(((ArrayRef *) node)->refupperindexpr)),
-					   sublevels_up);
-			ResolveNew(info, targetlist, (Node **) (&(((ArrayRef *) node)->reflowerindexpr)),
-					   sublevels_up);
-			ResolveNew(info, targetlist, (Node **) (&(((ArrayRef *) node)->refexpr)),
-					   sublevels_up);
-			ResolveNew(info, targetlist, (Node **) (&(((ArrayRef *) node)->refassgnexpr)),
-					   sublevels_up);
-			break;
-		case T_Var:
-			{
-				int			this_varno = (int) ((Var *) node)->varno;
-				int			this_varlevelsup = (int) ((Var *) node)->varlevelsup;
-				Node	   *n;
-
-				if (this_varno == info->new_varno &&
-					this_varlevelsup == sublevels_up)
-				{
-					n = FindMatchingNew(targetlist,
-										((Var *) node)->varattno);
-					if (n == NULL)
-					{
-						if (info->event == CMD_UPDATE)
-						{
-							*nodePtr = n = copyObject(node);
-							((Var *) n)->varno = info->current_varno;
-							((Var *) n)->varnoold = info->current_varno;
-						}
-						else
-							*nodePtr = make_null(((Var *) node)->vartype);
-					}
-					else
-					{
-						*nodePtr = copyObject(n);
-						((Var *) *nodePtr)->varlevelsup = this_varlevelsup;
-					}
-				}
-				break;
-			}
-		case T_List:
-			{
-				List	   *l;
-
-				foreach(l, (List *) node)
-					ResolveNew(info, targetlist, (Node **) &(lfirst(l)),
-							   sublevels_up);
-				break;
-			}
-		case T_SubLink:
-			{
-				SubLink    *sublink = (SubLink *) node;
-				Query	   *query = (Query *) sublink->subselect;
 
-				/* XXX what about lefthand?  What about rest of subquery? */
-				ResolveNew(info, targetlist, (Node **) &(query->qual), sublevels_up + 1);
-			}
-			break;
-		case T_GroupClause:
-			break;
-		default:
-			/* ignore the others */
-			break;
-	}
-}
+/*
+ * ResolveNew - replace Vars with corresponding items from a targetlist
+ *
+ * Vars matching info->new_varno and sublevels_up are replaced by the
+ * entry with matching resno from targetlist, if there is one.
+ */
 
-void
-FixNew(RewriteInfo *info, Query *parsetree)
-{
-	ResolveNew(info, parsetree->targetList,
-			   (Node **) &(info->rule_action->targetList), 0);
-	ResolveNew(info, parsetree->targetList,
-			   (Node **) &info->rule_action->qual, 0);
-	ResolveNew(info, parsetree->targetList,
-			   (Node **) &(info->rule_action->groupClause), 0);
-}
+typedef struct {
+	RewriteInfo	   *info;
+	List		   *targetlist;
+	int				sublevels_up;
+} ResolveNew_context;
 
-static void
-nodeHandleRIRAttributeRule(Node **nodePtr,
-						   List *rtable,
-						   List *targetlist,
-						   int rt_index,
-						   int attr_num,
-						   int *modified,
-						   int *badsql,
-						   int sublevels_up)
+static Node *
+ResolveNew_mutator(Node *node, ResolveNew_context *context)
 {
-	Node	   *node = *nodePtr;
-
 	if (node == NULL)
-		return;
-	switch (nodeTag(node))
+		return NULL;
+	if (IsA(node, Var))
 	{
-		case T_TargetEntry:
-			{
-				TargetEntry *tle = (TargetEntry *) node;
-
-				nodeHandleRIRAttributeRule(&tle->expr, rtable, targetlist,
-									rt_index, attr_num, modified, badsql,
-										   sublevels_up);
-			}
-			break;
-		case T_Aggref:
-			{
-				Aggref	   *aggref = (Aggref *) node;
+		Var		   *var = (Var *) node;
+		int			this_varno = (int) var->varno;
+		int			this_varlevelsup = (int) var->varlevelsup;
 
-				nodeHandleRIRAttributeRule(&aggref->target, rtable, targetlist,
-									rt_index, attr_num, modified, badsql,
-										   sublevels_up);
-			}
-			break;
-		case T_Expr:
-			{
-				Expr	   *expr = (Expr *) node;
-
-				nodeHandleRIRAttributeRule((Node **) (&(expr->args)), rtable,
-										   targetlist, rt_index, attr_num,
-										   modified, badsql,
-										   sublevels_up);
-			}
-			break;
-		case T_Iter:
-			{
-				Iter	   *iter = (Iter *) node;
+		if (this_varno == context->info->new_varno &&
+			this_varlevelsup == context->sublevels_up)
+		{
+			Node	   *n = FindMatchingNew(context->targetlist,
+											var->varattno);
 
-				nodeHandleRIRAttributeRule((Node **) (&(iter->iterexpr)), rtable,
-										   targetlist, rt_index, attr_num,
-										   modified, badsql,
-										   sublevels_up);
-			}
-			break;
-		case T_ArrayRef:
-			{
-				ArrayRef   *ref = (ArrayRef *) node;
-
-				nodeHandleRIRAttributeRule((Node **) (&(ref->refupperindexpr)), rtable,
-										   targetlist, rt_index, attr_num,
-										   modified, badsql,
-										   sublevels_up);
-				nodeHandleRIRAttributeRule((Node **) (&(ref->reflowerindexpr)), rtable,
-										   targetlist, rt_index, attr_num,
-										   modified, badsql,
-										   sublevels_up);
-				nodeHandleRIRAttributeRule((Node **) (&(ref->refexpr)), rtable,
-										   targetlist, rt_index, attr_num,
-										   modified, badsql,
-										   sublevels_up);
-				nodeHandleRIRAttributeRule((Node **) (&(ref->refassgnexpr)), rtable,
-										   targetlist, rt_index, attr_num,
-										   modified, badsql,
-										   sublevels_up);
-			}
-			break;
-		case T_Var:
+			if (n == NULL)
 			{
-				int			this_varno = ((Var *) node)->varno;
-				int			this_varattno = ((Var *) node)->varattno;
-				int			this_varlevelsup = ((Var *) node)->varlevelsup;
-
-				if (this_varno == rt_index &&
-					this_varattno == attr_num &&
-					this_varlevelsup == sublevels_up)
+				if (context->info->event == CMD_UPDATE)
 				{
-					if (((Var *) node)->vartype == 32)
-					{			/* HACK */
-						*nodePtr = make_null(((Var *) node)->vartype);
-						*modified = TRUE;
-						*badsql = TRUE;
-						break;
-					}
-					else
-					{
-						NameData	name_to_look_for;
-
-						name_to_look_for.data[0] = '\0';
-						namestrcpy(&name_to_look_for,
-								(char *) get_attname(getrelid(this_varno,
-															  rtable),
-													 attr_num));
-						if (name_to_look_for.data[0])
-						{
-							Node	   *n;
-
-							n = FindMatchingTLEntry(targetlist, (char *) &name_to_look_for);
-							if (n == NULL)
-								*nodePtr = make_null(((Var *) node)->vartype);
-							else
-								*nodePtr = n;
-							*modified = TRUE;
-						}
-					}
+					/* For update, just change unmatched var's varno */
+					n = copyObject(node);
+					((Var *) n)->varno = context->info->current_varno;
+					((Var *) n)->varnoold = context->info->current_varno;
+					return n;
+				}
+				else
+				{
+					/* Otherwise replace unmatched var with a null */
+					return make_null(var->vartype);
 				}
 			}
-			break;
-		case T_List:
+			else
 			{
-				List	   *i;
-
-				foreach(i, (List *) node)
+				/* Make a copy of the tlist item to return */
+				n = copyObject(n);
+				if (IsA(n, Var))
 				{
-					nodeHandleRIRAttributeRule((Node **) (&(lfirst(i))), rtable,
-										  targetlist, rt_index, attr_num,
-										 modified, badsql, sublevels_up);
+					((Var *) n)->varlevelsup = this_varlevelsup;
 				}
+				/* XXX what to do if tlist item is NOT a var?
+				 * Should we be using something like apply_RIR_adjust_sublevel?
+				 */
+				return n;
 			}
-			break;
-		case T_SubLink:
-			{
-				SubLink    *sublink = (SubLink *) node;
-				Query	   *query = (Query *) sublink->subselect;
+		}
+		/* otherwise fall through to copy the var normally */
+	}
+	/*
+	 * Since expression_tree_mutator won't touch subselects, we have to
+	 * handle them specially.
+	 */
+	if (IsA(node, SubLink))
+	{
+		SubLink   *sublink = (SubLink *) node;
+		SubLink   *newnode;
+
+		FLATCOPY(newnode, sublink, SubLink);
+		MUTATE(newnode->lefthand, sublink->lefthand, List *,
+			   ResolveNew_mutator, context);
+		context->sublevels_up++;
+		MUTATE(newnode->subselect, sublink->subselect, Node *,
+			   ResolveNew_mutator, context);
+		context->sublevels_up--;
+		return (Node *) newnode;
+	}
+	if (IsA(node, Query))
+	{
+		Query  *query = (Query *) node;
+		Query  *newnode;
 
-				/* XXX what about lefthand?  What about rest of subquery? */
-				nodeHandleRIRAttributeRule((Node **) &(query->qual), rtable, targetlist,
-									rt_index, attr_num, modified, badsql,
-										   sublevels_up + 1);
-			}
-			break;
-		default:
-			/* ignore the others */
-			break;
+		/*
+		 * XXX original code for ResolveNew only recursed into qual field
+		 * of subquery.  I'm assuming that was an oversight ... tgl 9/99
+		 */
+
+		FLATCOPY(newnode, query, Query);
+		MUTATE(newnode->targetList, query->targetList, List *,
+			   ResolveNew_mutator, context);
+		MUTATE(newnode->qual, query->qual, Node *,
+			   ResolveNew_mutator, context);
+		MUTATE(newnode->havingQual, query->havingQual, Node *,
+			   ResolveNew_mutator, context);
+		return (Node *) newnode;
 	}
+	return expression_tree_mutator(node, ResolveNew_mutator,
+								   (void *) context);
+}
+
+static Node *
+ResolveNew(Node *node, RewriteInfo *info, List *targetlist,
+		   int sublevels_up)
+{
+	ResolveNew_context	context;
+
+	context.info = info;
+	context.targetlist = targetlist;
+	context.sublevels_up = sublevels_up;
+
+	return ResolveNew_mutator(node, &context);
+}
+
+void
+FixNew(RewriteInfo *info, Query *parsetree)
+{
+	info->rule_action->targetList = (List *)
+		ResolveNew((Node *) info->rule_action->targetList,
+				   info, parsetree->targetList, 0);
+	info->rule_action->qual = ResolveNew(info->rule_action->qual,
+										 info, parsetree->targetList, 0);
+	/* XXX original code didn't fix havingQual; presumably an oversight? */
+	info->rule_action->havingQual = ResolveNew(info->rule_action->havingQual,
+											   info, parsetree->targetList, 0);
 }
 
 /*
+ * HandleRIRAttributeRule
+ *	Replace Vars matching a given RT index with copies of TL expressions.
+ *
  * Handles 'on retrieve to relation.attribute
  *			do instead retrieve (attribute = expression) w/qual'
+ *
+ * XXX Why is this not unified with apply_RIR_view()?
  */
-void
-HandleRIRAttributeRule(Query *parsetree,
-					   List *rtable,
-					   List *targetlist,
-					   int rt_index,
-					   int attr_num,
-					   int *modified,
-					   int *badsql)
-{
 
-	nodeHandleRIRAttributeRule((Node **) (&(parsetree->targetList)), rtable,
-							   targetlist, rt_index, attr_num,
-							   modified, badsql, 0);
-	nodeHandleRIRAttributeRule(&parsetree->qual, rtable, targetlist,
-							   rt_index, attr_num, modified, badsql, 0);
-}
+typedef struct {
+	List		   *rtable;
+	List		   *targetlist;
+	int				rt_index;
+	int				attr_num;
+	int			   *modified;
+	int			   *badsql;
+	int				sublevels_up;
+} HandleRIRAttributeRule_context;
 
-#ifdef NOT_USED
-static void
-nodeHandleViewRule(Node **nodePtr,
-				   List *rtable,
-				   List *targetlist,
-				   int rt_index,
-				   int *modified,
-				   int sublevels_up)
+static Node *
+HandleRIRAttributeRule_mutator(Node *node,
+							   HandleRIRAttributeRule_context *context)
 {
-	Node	   *node = *nodePtr;
-
 	if (node == NULL)
-		return;
-
-	switch (nodeTag(node))
+		return NULL;
+	if (IsA(node, Var))
 	{
-		case T_TargetEntry:
-			{
-				TargetEntry *tle = (TargetEntry *) node;
-
-				nodeHandleViewRule(&(tle->expr), rtable, targetlist,
-								   rt_index, modified, sublevels_up);
-			}
-			break;
-		case T_Aggref:
-			{
-				Aggref	   *aggref = (Aggref *) node;
-
-				nodeHandleViewRule(&(aggref->target), rtable, targetlist,
-								   rt_index, modified, sublevels_up);
-			}
-			break;
-
-			/*
-			 * This has to be done to make queries using groupclauses work
-			 * on views
-			 */
-		case T_GroupClause:
-			{
-				GroupClause *group = (GroupClause *) node;
-
-				nodeHandleViewRule((Node **) (&(group->entry)), rtable, targetlist,
-								   rt_index, modified, sublevels_up);
-			}
-			break;
-		case T_Expr:
-			{
-				Expr	   *expr = (Expr *) node;
-
-				nodeHandleViewRule((Node **) (&(expr->args)),
-								   rtable, targetlist,
-								   rt_index, modified, sublevels_up);
-			}
-			break;
-		case T_Iter:
-			{
-				Iter	   *iter = (Iter *) node;
-
-				nodeHandleViewRule((Node **) (&(iter->iterexpr)),
-								   rtable, targetlist,
-								   rt_index, modified, sublevels_up);
-			}
-			break;
-		case T_ArrayRef:
-			{
-				ArrayRef   *ref = (ArrayRef *) node;
-
-				nodeHandleViewRule((Node **) (&(ref->refupperindexpr)),
-								   rtable, targetlist,
-								   rt_index, modified, sublevels_up);
-				nodeHandleViewRule((Node **) (&(ref->reflowerindexpr)),
-								   rtable, targetlist,
-								   rt_index, modified, sublevels_up);
-				nodeHandleViewRule((Node **) (&(ref->refexpr)),
-								   rtable, targetlist,
-								   rt_index, modified, sublevels_up);
-				nodeHandleViewRule((Node **) (&(ref->refassgnexpr)),
-								   rtable, targetlist,
-								   rt_index, modified, sublevels_up);
-			}
-			break;
-		case T_Var:
-			{
-				Var		   *var = (Var *) node;
-				int			this_varno = var->varno;
-				int			this_varlevelsup = var->varlevelsup;
-				Node	   *n;
-
-				if (this_varno == rt_index &&
-					this_varlevelsup == sublevels_up)
+		Var		   *var = (Var *) node;
+		int			this_varno = var->varno;
+		int			this_varattno = var->varattno;
+		int			this_varlevelsup = var->varlevelsup;
+
+		if (this_varno == context->rt_index &&
+			this_varattno == context->attr_num &&
+			this_varlevelsup == context->sublevels_up)
+		{
+			if (var->vartype == 32)
+			{					/* HACK: disallow SET variables */
+				*context->modified = TRUE;
+				*context->badsql = TRUE;
+				return make_null(var->vartype);
+			}
+			else
+			{
+				NameData	name_to_look_for;
+
+				name_to_look_for.data[0] = '\0';
+				namestrcpy(&name_to_look_for,
+						   (char *) get_attname(getrelid(this_varno,
+														 context->rtable),
+												this_varattno));
+				if (name_to_look_for.data[0])
 				{
-					n = FindMatchingTLEntry(targetlist,
-										 get_attname(getrelid(this_varno,
-															  rtable),
-													 var->varattno));
+					Node	   *n;
+
+					*context->modified = TRUE;
+					n = FindMatchingTLEntry(context->targetlist,
+											(char *) &name_to_look_for);
 					if (n == NULL)
-						*nodePtr = make_null(((Var *) node)->vartype);
+						return make_null(var->vartype);
 					else
-					{
-
-						/*
-						 * This is a hack: The varlevelsup of the orignal
-						 * variable and the new one should be the same.
-						 * Normally we adapt the node by changing a
-						 * pointer to point to a var contained in
-						 * 'targetlist'. In the targetlist all
-						 * varlevelsups are 0 so if we want to change it
-						 * to the original value we have to copy the node
-						 * before! (Maybe this will cause troubles with
-						 * some sophisticated queries on views?)
-						 */
-						if (this_varlevelsup > 0)
-							*nodePtr = copyObject(n);
-						else
-							*nodePtr = n;
-
-						if (nodeTag(nodePtr) == T_Var)
-							((Var *) *nodePtr)->varlevelsup = this_varlevelsup;
-						else
-							nodeHandleViewRule(&n, rtable, targetlist,
-									   rt_index, modified, sublevels_up);
-					}
-					*modified = TRUE;
-				}
-				break;
-			}
-		case T_List:
-			{
-				List	   *l;
-
-				foreach(l, (List *) node)
-				{
-					nodeHandleViewRule((Node **) (&(lfirst(l))),
-									   rtable, targetlist,
-									   rt_index, modified, sublevels_up);
+						return copyObject(n);
 				}
 			}
-			break;
-		case T_SubLink:
-			{
-				SubLink    *sublink = (SubLink *) node;
-				Query	   *query = (Query *) sublink->subselect;
-
-				nodeHandleViewRule((Node **) &(query->qual), rtable, targetlist,
-								   rt_index, modified, sublevels_up + 1);
-
-				/***S*H*D***/
-				nodeHandleViewRule((Node **) &(query->havingQual), rtable, targetlist,
-								   rt_index, modified, sublevels_up + 1);
-				nodeHandleViewRule((Node **) &(query->targetList), rtable, targetlist,
-								   rt_index, modified, sublevels_up + 1);
+		}
+		/* otherwise fall through to copy the var normally */
+	}
+	/*
+	 * Since expression_tree_mutator won't touch subselects, we have to
+	 * handle them specially.
+	 */
+	if (IsA(node, SubLink))
+	{
+		SubLink   *sublink = (SubLink *) node;
+		SubLink   *newnode;
+
+		FLATCOPY(newnode, sublink, SubLink);
+		MUTATE(newnode->lefthand, sublink->lefthand, List *,
+			   HandleRIRAttributeRule_mutator, context);
+		context->sublevels_up++;
+		MUTATE(newnode->subselect, sublink->subselect, Node *,
+			   HandleRIRAttributeRule_mutator, context);
+		context->sublevels_up--;
+		return (Node *) newnode;
+	}
+	if (IsA(node, Query))
+	{
+		Query  *query = (Query *) node;
+		Query  *newnode;
 
+		/*
+		 * XXX original code for HandleRIRAttributeRule only recursed into
+		 * qual field of subquery.  I'm assuming that was an oversight ...
+		 */
 
-				/*
-				 * We also have to adapt the variables used in
-				 * sublink->lefthand
-				 */
-				nodeHandleViewRule((Node **) &(sublink->lefthand), rtable,
-						   targetlist, rt_index, modified, sublevels_up);
-			}
-			break;
-		default:
-			/* ignore the others */
-			break;
+		FLATCOPY(newnode, query, Query);
+		MUTATE(newnode->targetList, query->targetList, List *,
+			   HandleRIRAttributeRule_mutator, context);
+		MUTATE(newnode->qual, query->qual, Node *,
+			   HandleRIRAttributeRule_mutator, context);
+		MUTATE(newnode->havingQual, query->havingQual, Node *,
+			   HandleRIRAttributeRule_mutator, context);
+		return (Node *) newnode;
 	}
+	return expression_tree_mutator(node, HandleRIRAttributeRule_mutator,
+								   (void *) context);
 }
 
 void
-HandleViewRule(Query *parsetree,
-			   List *rtable,
-			   List *targetlist,
-			   int rt_index,
-			   int *modified)
+HandleRIRAttributeRule(Query *parsetree,
+					   List *rtable,
+					   List *targetlist,
+					   int rt_index,
+					   int attr_num,
+					   int *modified,
+					   int *badsql)
 {
-	nodeHandleViewRule(&parsetree->qual, rtable, targetlist, rt_index,
-					   modified, 0);
-	nodeHandleViewRule((Node **) (&(parsetree->targetList)), rtable, targetlist,
-					   rt_index, modified, 0);
-
-	/*
-	 * The variables in the havingQual and groupClause also have to be
-	 * adapted
-	 */
-	nodeHandleViewRule(&parsetree->havingQual, rtable, targetlist, rt_index,
-					   modified, 0);
-	nodeHandleViewRule((Node **) (&(parsetree->groupClause)), rtable, targetlist, rt_index,
-					   modified, 0);
+	HandleRIRAttributeRule_context	context;
+
+	context.rtable = rtable;
+	context.targetlist = targetlist;
+	context.rt_index = rt_index;
+	context.attr_num = attr_num;
+	context.modified = modified;
+	context.badsql = badsql;
+	context.sublevels_up = 0;
+
+	parsetree->targetList = (List *) 
+		HandleRIRAttributeRule_mutator((Node *) parsetree->targetList,
+									   &context);
+	parsetree->qual = HandleRIRAttributeRule_mutator(parsetree->qual,
+													 &context);
+	/* XXX original code did not fix havingQual ... oversight? */
+	parsetree->havingQual = HandleRIRAttributeRule_mutator(parsetree->havingQual,
+														   &context);
 }
-
-#endif
diff --git a/src/include/rewrite/rewriteManip.h b/src/include/rewrite/rewriteManip.h
index 273b2fd4996..fa212ef8f82 100644
--- a/src/include/rewrite/rewriteManip.h
+++ b/src/include/rewrite/rewriteManip.h
@@ -6,7 +6,7 @@
  *
  * Copyright (c) 1994, Regents of the University of California
  *
- * $Id: rewriteManip.h,v 1.17 1999/07/15 15:21:30 momjian Exp $
+ * $Id: rewriteManip.h,v 1.18 1999/10/01 04:08:15 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -16,19 +16,18 @@
 #include "rewrite/rewriteHandler.h"
 
 /* RewriteManip.c */
-void		OffsetVarNodes(Node *node, int offset, int sublevels_up);
-void ChangeVarNodes(Node *node, int old_varno, int new_varno,
-			   int sublevels_up);
-void		AddQual(Query *parsetree, Node *qual);
-void		AddHavingQual(Query *parsetree, Node *havingQual);
+extern void OffsetVarNodes(Node *node, int offset, int sublevels_up);
+extern void ChangeVarNodes(Node *node, int old_varno, int new_varno,
+						   int sublevels_up);
+extern void AddQual(Query *parsetree, Node *qual);
+extern void AddHavingQual(Query *parsetree, Node *havingQual);
+extern void AddNotQual(Query *parsetree, Node *qual);
+extern void AddGroupClause(Query *parsetree, List *group_by, List *tlist);
 
-void		AddNotQual(Query *parsetree, Node *qual);
-void		AddGroupClause(Query *parsetree, List *group_by, List *tlist);
+extern void FixNew(RewriteInfo *info, Query *parsetree);
 
-void		FixNew(RewriteInfo *info, Query *parsetree);
-
-void HandleRIRAttributeRule(Query *parsetree, List *rtable, List *targetlist,
-					   int rt_index, int attr_num, int *modified,
-					   int *badpostquel);
+extern void HandleRIRAttributeRule(Query *parsetree, List *rtable,
+								   List *targetlist, int rt_index,
+								   int attr_num, int *modified, int *badsql);
 
 #endif	 /* REWRITEMANIP_H */
-- 
GitLab