From: Tom Lane Date: Thu, 29 Jun 2000 07:35:57 +0000 (+0000) Subject: Add test code to copy all parse/plan trees. Repair essential omissions X-Git-Tag: REL7_1_BETA~1047 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=43ba1b4420eca0bf227ad2bae87f7955093e1cc0;p=postgresql 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. --- diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index fba792cc84..605fe70e6c 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/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; diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index ddb5c72391..308f4d90e8 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/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)); diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index e018114a5d..724cfc2933 100644 --- a/src/backend/tcop/postgres.c +++ b/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 #include #include @@ -24,9 +26,6 @@ #include #include #include - -#include "postgres.h" - #include #if HAVE_SYS_SELECT_H #include @@ -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"); } /*