Commit 465e09da authored by Tom Lane's avatar Tom Lane

Add support for more extensive testing of raw_expression_tree_walker().

If RAW_EXPRESSION_COVERAGE_TEST is defined, do a no-op tree walk over
every basic DML statement submitted to parse analysis.  If we'd had this
in place earlier, bug #14153 would have been caught by buildfarm testing.
The difficulty is that raw_expression_tree_walker() is only used in
limited cases involving CTEs (particularly recursive ones), so it's
very easy for an oversight in it to not be noticed during testing of a
seemingly-unrelated feature.

The type of error we can expect to catch with this is complete omission
of a node type from raw_expression_tree_walker(), and perhaps also
recursion into a field that doesn't contain a node tree, though that
would be an unlikely mistake.  It won't catch failure to add new fields
that need to be recursed into, unfortunately.

I'll go enable this on one or two of my own buildfarm animals once
bug #14153 is dealt with.

Discussion: <27861.1464040417@sss.pgh.pa.us>
parent 8a4930e3
...@@ -2991,8 +2991,10 @@ query_or_expression_tree_mutator(Node *node, ...@@ -2991,8 +2991,10 @@ query_or_expression_tree_mutator(Node *node,
* Unlike expression_tree_walker, there is no special rule about query * Unlike expression_tree_walker, there is no special rule about query
* boundaries: we descend to everything that's possibly interesting. * boundaries: we descend to everything that's possibly interesting.
* *
* Currently, the node type coverage extends to SelectStmt and everything * Currently, the node type coverage here extends only to DML statements
* that could appear under it, but not other statement types. * (SELECT/INSERT/UPDATE/DELETE) and nodes that can appear in them, because
* this is used mainly during analysis of CTEs, and only DML statements can
* appear in CTEs.
*/ */
bool bool
raw_expression_tree_walker(Node *node, raw_expression_tree_walker(Node *node,
......
...@@ -74,6 +74,9 @@ static Query *transformCreateTableAsStmt(ParseState *pstate, ...@@ -74,6 +74,9 @@ static Query *transformCreateTableAsStmt(ParseState *pstate,
CreateTableAsStmt *stmt); CreateTableAsStmt *stmt);
static void transformLockingClause(ParseState *pstate, Query *qry, static void transformLockingClause(ParseState *pstate, Query *qry,
LockingClause *lc, bool pushedDown); LockingClause *lc, bool pushedDown);
#ifdef RAW_EXPRESSION_COVERAGE_TEST
static bool test_raw_expression_coverage(Node *node, void *context);
#endif
/* /*
...@@ -220,6 +223,25 @@ transformStmt(ParseState *pstate, Node *parseTree) ...@@ -220,6 +223,25 @@ transformStmt(ParseState *pstate, Node *parseTree)
{ {
Query *result; Query *result;
/*
* We apply RAW_EXPRESSION_COVERAGE_TEST testing to basic DML statements;
* we can't just run it on everything because raw_expression_tree_walker()
* doesn't claim to handle utility statements.
*/
#ifdef RAW_EXPRESSION_COVERAGE_TEST
switch (nodeTag(parseTree))
{
case T_SelectStmt:
case T_InsertStmt:
case T_UpdateStmt:
case T_DeleteStmt:
(void) test_raw_expression_coverage(parseTree, NULL);
break;
default:
break;
}
#endif /* RAW_EXPRESSION_COVERAGE_TEST */
switch (nodeTag(parseTree)) switch (nodeTag(parseTree))
{ {
/* /*
...@@ -2713,3 +2735,25 @@ applyLockingClause(Query *qry, Index rtindex, ...@@ -2713,3 +2735,25 @@ applyLockingClause(Query *qry, Index rtindex,
rc->pushedDown = pushedDown; rc->pushedDown = pushedDown;
qry->rowMarks = lappend(qry->rowMarks, rc); qry->rowMarks = lappend(qry->rowMarks, rc);
} }
/*
* Coverage testing for raw_expression_tree_walker().
*
* When enabled, we run raw_expression_tree_walker() over every DML statement
* submitted to parse analysis. Without this provision, that function is only
* applied in limited cases involving CTEs, and we don't really want to have
* to test everything inside as well as outside a CTE.
*/
#ifdef RAW_EXPRESSION_COVERAGE_TEST
static bool
test_raw_expression_coverage(Node *node, void *context)
{
if (node == NULL)
return false;
return raw_expression_tree_walker(node,
test_raw_expression_coverage,
context);
}
#endif /* RAW_EXPRESSION_COVERAGE_TEST */
...@@ -273,6 +273,13 @@ ...@@ -273,6 +273,13 @@
*/ */
/* #define COPY_PARSE_PLAN_TREES */ /* #define COPY_PARSE_PLAN_TREES */
/*
* Define this to force all raw parse trees for DML statements to be scanned
* by raw_expression_tree_walker(), to facilitate catching errors and
* omissions in that function.
*/
/* #define RAW_EXPRESSION_COVERAGE_TEST */
/* /*
* Enable debugging print statements for lock-related operations. * Enable debugging print statements for lock-related operations.
*/ */
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment