Add test code to copy all parse/plan trees. Repair essential omissions

in copyfuncs and equalfuncs exposed by regression tests.  We still have
some work to do: these modules really ought to handle most or all of
the utility statement node types.  But it's better than it was.

src/backend/nodes/copyfuncs.c

@@ -3,12 +3,23 @@
  * copyfuncs.c
  *	  Copy functions for Postgres tree nodes.
  *
+ * NOTE: a general convention when copying or comparing plan nodes is
+ * that we ignore the executor state subnode.  We do not need to look
+ * at it because no current uses of copyObject() or equal() need to
+ * deal with already-executing plan trees.  By leaving the state subnodes
+ * out, we avoid needing to write copy/compare routines for all the
+ * different executor state node types.
+ *
+ * Another class of nodes not currently handled is nodes that appear
+ * only in "raw" parsetrees (gram.y output not yet analyzed by the parser).
+ * Perhaps some day that will need to be supported.
+ *
+ *
  * Portions Copyright (c) 1996-2000, PostgreSQL, Inc
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v 1.114 2000/06/18 22:44:05 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v 1.115 2000/06/29 07:35:56 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1671,9 +1682,6 @@ copyObject(void *from)
 		case T_Agg:
 			retval = _copyAgg(from);
 			break;
-		case T_GroupClause:
-			retval = _copyGroupClause(from);
-			break;
 		case T_Unique:
 			retval = _copyUnique(from);
 			break;
@@ -1699,9 +1707,6 @@ copyObject(void *from)
 		case T_Var:
 			retval = _copyVar(from);
 			break;
-		case T_Attr:
-			retval = _copyAttr(from);
-			break;
 		case T_Oper:
 			retval = _copyOper(from);
 			break;
@@ -1711,6 +1716,12 @@ copyObject(void *from)
 		case T_Param:
 			retval = _copyParam(from);
 			break;
+		case T_Aggref:
+			retval = _copyAggref(from);
+			break;
+		case T_SubLink:
+			retval = _copySubLink(from);
+			break;
 		case T_Func:
 			retval = _copyFunc(from);
 			break;
@@ -1720,21 +1731,12 @@ copyObject(void *from)
 		case T_ArrayRef:
 			retval = _copyArrayRef(from);
 			break;
-		case T_Aggref:
-			retval = _copyAggref(from);
-			break;
-		case T_SubLink:
-			retval = _copySubLink(from);
+		case T_Iter:
+			retval = _copyIter(from);
 			break;
 		case T_RelabelType:
 			retval = _copyRelabelType(from);
 			break;
-		case T_CaseExpr:
-			retval = _copyCaseExpr(from);
-			break;
-		case T_CaseWhen:
-			retval = _copyCaseWhen(from);
-			break;
 
 			/*
 			 * RELATION NODES
@@ -1769,9 +1771,6 @@ copyObject(void *from)
 		case T_JoinInfo:
 			retval = _copyJoinInfo(from);
 			break;
-		case T_Iter:
-			retval = _copyIter(from);
-			break;
 		case T_Stream:
 			retval = _copyStream(from);
 			break;
@@ -1780,29 +1779,35 @@ copyObject(void *from)
 			break;
 
 			/*
-			 * PARSE NODES
+			 * VALUE NODES
 			 */
-		case T_TargetEntry:
-			retval = _copyTargetEntry(from);
-			break;
-		case T_RangeTblEntry:
-			retval = _copyRangeTblEntry(from);
-			break;
-		case T_RowMark:
-			retval = _copyRowMark(from);
-			break;
-		case T_SortClause:
-			retval = _copySortClause(from);
-			break;
-		case T_A_Const:
-			retval = _copyAConst(from);
-			break;
-		case T_TypeName:
-			retval = _copyTypeName(from);
+		case T_Integer:
+		case T_Float:
+		case T_String:
+			retval = _copyValue(from);
 			break;
-		case T_TypeCast:
-			retval = _copyTypeCast(from);
+		case T_List:
+			{
+				List	   *list = from,
+						   *l,
+						   *nl;
+
+				/* rather ugly coding for speed... */
+				/* Note the input list cannot be NIL if we got here. */
+				nl = lcons(copyObject(lfirst(list)), NIL);
+				retval = nl;
+
+				foreach(l, lnext(list))
+				{
+					lnext(nl) = lcons(copyObject(lfirst(l)), NIL);
+					nl = lnext(nl);
+				}
+			}
 			break;
+
+			/*
+			 * PARSE NODES
+			 */
 		case T_Query:
 			retval = _copyQuery(from);
 			break;
@@ -1837,35 +1842,44 @@ copyObject(void *from)
 			retval = _copyLockStmt(from);
 			break;
 
-			/*
-			 * VALUE NODES
-			 */
-		case T_Integer:
-		case T_Float:
-		case T_String:
-			retval = _copyValue(from);
+		case T_Attr:
+			retval = _copyAttr(from);
 			break;
-		case T_List:
-			{
-				List	   *list = from,
-						   *l,
-						   *nl;
-
-				/* rather ugly coding for speed... */
-				/* Note the input list cannot be NIL if we got here. */
-				nl = lcons(copyObject(lfirst(list)), NIL);
-				retval = nl;
-
-				foreach(l, lnext(list))
-				{
-					lnext(nl) = lcons(copyObject(lfirst(l)), NIL);
-					nl = lnext(nl);
-				}
-			}
+		case T_A_Const:
+			retval = _copyAConst(from);
+			break;
+		case T_TypeCast:
+			retval = _copyTypeCast(from);
+			break;
+		case T_TypeName:
+			retval = _copyTypeName(from);
+			break;
+		case T_TargetEntry:
+			retval = _copyTargetEntry(from);
+			break;
+		case T_RangeTblEntry:
+			retval = _copyRangeTblEntry(from);
 			break;
+		case T_SortClause:
+			retval = _copySortClause(from);
+			break;
+		case T_GroupClause:
+			retval = _copyGroupClause(from);
+			break;
+		case T_CaseExpr:
+			retval = _copyCaseExpr(from);
+			break;
+		case T_CaseWhen:
+			retval = _copyCaseWhen(from);
+			break;
+		case T_RowMark:
+			retval = _copyRowMark(from);
+			break;
+
 		default:
-			elog(ERROR, "copyObject: don't know how to copy %d", nodeTag(from));
-			retval = from;
+			elog(ERROR, "copyObject: don't know how to copy node type %d",
+				 nodeTag(from));
+			retval = from;		/* keep compiler quiet */
 			break;
 	}
 	return retval;

src/backend/nodes/equalfuncs.c

@@ -1,14 +1,30 @@
 /*-------------------------------------------------------------------------
  *
  * equalfuncs.c
- *	  equality functions to compare node trees
+ *	  Equality functions to compare node trees.
+ *
+ * NOTE: a general convention when copying or comparing plan nodes is
+ * that we ignore the executor state subnode.  We do not need to look
+ * at it because no current uses of copyObject() or equal() need to
+ * deal with already-executing plan trees.  By leaving the state subnodes
+ * out, we avoid needing to write copy/compare routines for all the
+ * different executor state node types.
+ *
+ * Currently, in fact, equal() doesn't know how to compare Plan nodes
+ * at all, let alone their executor-state subnodes.  This will probably
+ * need to be fixed someday, but presently there is no need to compare
+ * plan trees.
+ *
+ * Another class of nodes not currently handled is nodes that appear
+ * only in "raw" parsetrees (gram.y output not yet analyzed by the parser).
+ * Perhaps some day that will need to be supported.
+ *
  *
  * Portions Copyright (c) 1996-2000, PostgreSQL, Inc
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/nodes/equalfuncs.c,v 1.66 2000/04/12 17:15:16 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/nodes/equalfuncs.c,v 1.67 2000/06/29 07:35:56 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -452,56 +468,6 @@ _equalHashPath(HashPath *a, HashPath *b)
 	return true;
 }
 
-/* XXX	This equality function is a quick hack, should be
- *		fixed to compare all fields.
- *
- * XXX	Why is this even here?	We don't have equal() funcs for
- *		any other kinds of Plan nodes... likely this is dead code...
- */
-static bool
-_equalIndexScan(IndexScan *a, IndexScan *b)
-{
-
-	/*
-	 * if(a->scan.plan.cost != b->scan.plan.cost) return(false);
-	 */
-
-	if (!equal(a->indxqual, b->indxqual))
-		return false;
-
-	if (a->scan.scanrelid != b->scan.scanrelid)
-		return false;
-
-	if (a->indxorderdir != b->indxorderdir)
-		return false;
-
-	if (!equali(a->indxid, b->indxid))
-		return false;
-	return true;
-}
-
-static bool
-_equalTidScan(TidScan *a, TidScan *b)
-{
-	Assert(IsA(a, TidScan));
-	Assert(IsA(b, TidScan));
-
-	/*
-	 * if(a->scan.plan.cost != b->scan.plan.cost) return(false);
-	 */
-
-	if (a->needRescan != b->needRescan)
-		return false;
-
-	if (!equal(a->tideval, b->tideval))
-		return false;
-
-	if (a->scan.scanrelid != b->scan.scanrelid)
-		return false;
-
-	return true;
-}
-
 static bool
 _equalSubPlan(SubPlan *a, SubPlan *b)
 {
@@ -703,6 +669,17 @@ _equalSortClause(SortClause *a, SortClause *b)
 	return true;
 }
 
+static bool
+_equalRowMark(RowMark *a, RowMark *b)
+{
+	if (a->rti != b->rti)
+		return false;
+	if (a->info != b->info)
+		return false;
+
+	return true;
+}
+
 static bool
 _equalTargetEntry(TargetEntry *a, TargetEntry *b)
 {
@@ -792,6 +769,9 @@ equal(void *a, void *b)
 
 	switch (nodeTag(a))
 	{
+		case T_SubPlan:
+			retval = _equalSubPlan(a, b);
+			break;
 		case T_Resdom:
 			retval = _equalResdom(a, b);
 			break;
@@ -801,24 +781,9 @@ equal(void *a, void *b)
 		case T_Expr:
 			retval = _equalExpr(a, b);
 			break;
-		case T_Iter:
-			retval = _equalIter(a, b);
-			break;
-		case T_Stream:
-			retval = _equalStream(a, b);
-			break;
-		case T_Attr:
-			retval = _equalAttr(a, b);
-			break;
 		case T_Var:
 			retval = _equalVar(a, b);
 			break;
-		case T_Array:
-			retval = _equalArray(a, b);
-			break;
-		case T_ArrayRef:
-			retval = _equalArrayRef(a, b);
-			break;
 		case T_Oper:
 			retval = _equalOper(a, b);
 			break;
@@ -834,23 +799,23 @@ equal(void *a, void *b)
 		case T_SubLink:
 			retval = _equalSubLink(a, b);
 			break;
-		case T_RelabelType:
-			retval = _equalRelabelType(a, b);
-			break;
 		case T_Func:
 			retval = _equalFunc(a, b);
 			break;
-		case T_RestrictInfo:
-			retval = _equalRestrictInfo(a, b);
+		case T_Array:
+			retval = _equalArray(a, b);
 			break;
-		case T_RelOptInfo:
-			retval = _equalRelOptInfo(a, b);
+		case T_ArrayRef:
+			retval = _equalArrayRef(a, b);
 			break;
-		case T_IndexOptInfo:
-			retval = _equalIndexOptInfo(a, b);
+		case T_Iter:
+			retval = _equalIter(a, b);
 			break;
-		case T_PathKeyItem:
-			retval = _equalPathKeyItem(a, b);
+		case T_RelabelType:
+			retval = _equalRelabelType(a, b);
+			break;
+		case T_RelOptInfo:
+			retval = _equalRelOptInfo(a, b);
 			break;
 		case T_Path:
 			retval = _equalPath(a, b);
@@ -858,9 +823,6 @@ equal(void *a, void *b)
 		case T_IndexPath:
 			retval = _equalIndexPath(a, b);
 			break;
-		case T_TidPath:
-			retval = _equalTidPath(a, b);
-			break;
 		case T_NestPath:
 			retval = _equalNestPath(a, b);
 			break;
@@ -870,25 +832,29 @@ equal(void *a, void *b)
 		case T_HashPath:
 			retval = _equalHashPath(a, b);
 			break;
-		case T_IndexScan:
-			retval = _equalIndexScan(a, b);
-			break;
-		case T_TidScan:
-			retval = _equalTidScan(a, b);
+		case T_PathKeyItem:
+			retval = _equalPathKeyItem(a, b);
 			break;
-		case T_SubPlan:
-			retval = _equalSubPlan(a, b);
+		case T_RestrictInfo:
+			retval = _equalRestrictInfo(a, b);
 			break;
 		case T_JoinInfo:
 			retval = _equalJoinInfo(a, b);
 			break;
+		case T_Stream:
+			retval = _equalStream(a, b);
+			break;
+		case T_TidPath:
+			retval = _equalTidPath(a, b);
+			break;
+		case T_IndexOptInfo:
+			retval = _equalIndexOptInfo(a, b);
+			break;
 		case T_EState:
 			retval = _equalEState(a, b);
 			break;
-		case T_Integer:
-		case T_Float:
-		case T_String:
-			retval = _equalValue(a, b);
+		case T_Attr:
+			retval = _equalAttr(a, b);
 			break;
 		case T_List:
 			{
@@ -911,9 +877,17 @@ equal(void *a, void *b)
 				retval = true;
 			}
 			break;
+		case T_Integer:
+		case T_Float:
+		case T_String:
+			retval = _equalValue(a, b);
+			break;
 		case T_Query:
 			retval = _equalQuery(a, b);
 			break;
+		case T_TargetEntry:
+			retval = _equalTargetEntry(a, b);
+			break;
 		case T_RangeTblEntry:
 			retval = _equalRangeTblEntry(a, b);
 			break;
@@ -924,15 +898,16 @@ equal(void *a, void *b)
 			/* GroupClause is equivalent to SortClause */
 			retval = _equalSortClause(a, b);
 			break;
-		case T_TargetEntry:
-			retval = _equalTargetEntry(a, b);
-			break;
 		case T_CaseExpr:
 			retval = _equalCaseExpr(a, b);
 			break;
 		case T_CaseWhen:
 			retval = _equalCaseWhen(a, b);
 			break;
+		case T_RowMark:
+			retval = _equalRowMark(a, b);
+			break;
+
 		default:
 			elog(NOTICE, "equal: don't know whether nodes of type %d are equal",
 				 nodeTag(a));

src/backend/tcop/postgres.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/tcop/postgres.c,v 1.162 2000/06/28 03:32:18 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/tcop/postgres.c,v 1.163 2000/06/29 07:35:57 tgl Exp $
  *
  * NOTES
  *	  this is the "main" module of the postgres backend and
@@ -17,6 +17,8 @@
  *-------------------------------------------------------------------------
  */
 
+#include "postgres.h"
+
 #include <unistd.h>
 #include <signal.h>
 #include <time.h>
@@ -24,9 +26,6 @@
 #include <sys/types.h>
 #include <fcntl.h>
 #include <sys/socket.h>
-
-#include "postgres.h"
-
 #include <errno.h>
 #if HAVE_SYS_SELECT_H
 #include <sys/select.h>
@@ -408,6 +407,31 @@ pg_parse_and_rewrite(char *query_string,	/* string to execute */
 
 	querytree_list = new_list;
 
+#ifdef COPY_PARSE_PLAN_TREES
+	/* Optional debugging check: pass parsetree output through copyObject() */
+	/*
+	 * Note: we run this test after rewrite, not before, because copyObject()
+	 * does not handle most kinds of nodes that are used only in raw parse
+	 * trees.  The present (bizarre) implementation of UNION/INTERSECT/EXCEPT
+	 * doesn't run analysis of the second and later subqueries until rewrite,
+	 * so we'd get false failures on these queries if we did it beforehand.
+	 *
+	 * Currently, copyObject doesn't know about most of the utility query
+	 * types, so suppress the check until that can be fixed... it should
+	 * be fixed, though.
+	 */
+	if (querytree_list &&
+		((Query *) lfirst(querytree_list))->commandType != CMD_UTILITY)
+	{
+		new_list = (List *) copyObject(querytree_list);
+		/* This checks both copyObject() and the equal() routines... */
+		if (! equal(new_list, querytree_list))
+			elog(NOTICE, "pg_parse_and_rewrite: copyObject failed on parse tree");
+		else
+			querytree_list = new_list;
+	}
+#endif
+
 	if (Debug_print_rewritten)
 	{
 		if (Debug_pretty_print)
@@ -458,6 +482,24 @@ pg_plan_query(Query *querytree)
 		ShowUsage();
 	}
 
+#ifdef COPY_PARSE_PLAN_TREES
+	/* Optional debugging check: pass plan output through copyObject() */
+	{
+		Plan   *new_plan = (Plan *) copyObject(plan);
+
+		/* equal() currently does not have routines to compare Plan nodes,
+		 * so don't try to test equality here.  Perhaps fix someday?
+		 */
+#ifdef NOT_USED
+		/* This checks both copyObject() and the equal() routines... */
+		if (! equal(new_plan, plan))
+			elog(NOTICE, "pg_plan_query: copyObject failed on plan tree");
+		else
+#endif
+			plan = new_plan;
+	}
+#endif
+
 	/* ----------------
 	 *	Print plan if debugging.
 	 * ----------------
@@ -1366,7 +1408,7 @@ PostgresMain(int argc, char *argv[], int real_argc, char *real_argv[])
 	if (!IsUnderPostmaster)
 	{
 		puts("\nPOSTGRES backend interactive interface ");
-		puts("$Revision: 1.162 $ $Date: 2000/06/28 03:32:18 $\n");
+		puts("$Revision: 1.163 $ $Date: 2000/06/29 07:35:57 $\n");
 	}
 
 	/*