Convert node test compile-time settings into run-time parameters

This converts

    COPY_PARSE_PLAN_TREES
    WRITE_READ_PARSE_PLAN_TREES
    RAW_EXPRESSION_COVERAGE_TEST

into run-time parameters

    debug_copy_parse_plan_trees
    debug_write_read_parse_plan_trees
    debug_raw_expression_coverage_test

They can be activated for tests using PG_TEST_INITDB_EXTRA_OPTS.

The compile-time symbols are kept for build farm compatibility, but
they now just determine the default value of the run-time settings.

Furthermore, support for these settings is not compiled in at all
unless assertions are enabled, or the new symbol
DEBUG_NODE_TESTS_ENABLED is defined at compile time, or any of the
legacy compile-time setting symbols are defined.  So there is no
run-time overhead in production builds.  (This is similar to the
handling of DISCARD_CACHES_ENABLED.)

Discussion: https://www.postgresql.org/message-id/flat/30747bd8-f51e-4e0c-a310-a6e2c37ec8aa%40eisentraut.org
This commit is contained in:
Peter Eisentraut 2024-08-01 09:37:44 +02:00
parent a67da49e1d
commit a292c98d62
12 changed files with 213 additions and 65 deletions

View File

@ -133,9 +133,10 @@ task:
DISK_SIZE: 50
CCACHE_DIR: /tmp/ccache_dir
CPPFLAGS: -DRELCACHE_FORCE_RELEASE -DCOPY_PARSE_PLAN_TREES -DWRITE_READ_PARSE_PLAN_TREES -DRAW_EXPRESSION_COVERAGE_TEST -DENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS
CPPFLAGS: -DRELCACHE_FORCE_RELEASE -DENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS
CFLAGS: -Og -ggdb
PG_TEST_INITDB_EXTRA_OPTS: -c debug_copy_parse_plan_trees=on -c debug_write_read_parse_plan_trees=on -c debug_raw_expression_coverage_test=on
PG_TEST_PG_UPGRADE_MODE: --link
<<: *freebsd_task_template

View File

@ -11425,6 +11425,29 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
</listitem>
</varlistentry>
<varlistentry id="guc-debug-copy-parse-plan-trees" xreflabel="debug_copy_parse_plan_trees">
<term><varname>debug_copy_parse_plan_trees</varname> (<type>boolean</type>)
<indexterm>
<primary><varname>debug_copy_parse_plan_trees</varname> configuration parameter</primary>
</indexterm>
</term>
<listitem>
<para>
Enabling this forces all parse and plan trees to be passed through
<function>copyObject()</function>, to facilitate catching errors and
omissions in <function>copyObject()</function>. The default is off.
</para>
<para>
This parameter is only available when
<symbol>DEBUG_NODE_TESTS_ENABLED</symbol> was defined at compile time
(which happens automatically when using the
<application>configure</application> option
<option>--enable-cassert</option>).
</para>
</listitem>
</varlistentry>
<varlistentry id="guc-debug-discard-caches" xreflabel="debug_discard_caches">
<term><varname>debug_discard_caches</varname> (<type>integer</type>)
<indexterm>
@ -11540,6 +11563,54 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
</listitem>
</varlistentry>
<varlistentry id="guc-debug-raw-expression-coverage-test" xreflabel="debug_raw_expression_coverage_test">
<term><varname>debug_raw_expression_coverage_test</varname> (<type>boolean</type>)
<indexterm>
<primary><varname>debug_raw_expression_coverage_test</varname> configuration parameter</primary>
</indexterm>
</term>
<listitem>
<para>
Enabling this forces all raw parse trees for DML statements to be
scanned by <function>raw_expression_tree_walker()</function>, to
facilitate catching errors and omissions in that function. The
default is off.
</para>
<para>
This parameter is only available when
<symbol>DEBUG_NODE_TESTS_ENABLED</symbol> was defined at compile time
(which happens automatically when using the
<application>configure</application> option
<option>--enable-cassert</option>).
</para>
</listitem>
</varlistentry>
<varlistentry id="guc-debug-write-read-parse-plan-trees" xreflabel="debug_write_read_parse_plan_trees">
<term><varname>debug_write_read_parse_plan_trees</varname> (<type>boolean</type>)
<indexterm>
<primary><varname>debug_write_read_parse_plan_trees</varname> configuration parameter</primary>
</indexterm>
</term>
<listitem>
<para>
Enabling this forces all parse and plan trees to be passed through
<filename>outfuncs.c</filename>/<filename>readfuncs.c</filename>, to
facilitate catching errors and omissions in those modules. The
default is off.
</para>
<para>
This parameter is only available when
<symbol>DEBUG_NODE_TESTS_ENABLED</symbol> was defined at compile time
(which happens automatically when using the
<application>configure</application> option
<option>--enable-cassert</option>).
</para>
</listitem>
</varlistentry>
<varlistentry id="guc-ignore-system-indexes" xreflabel="ignore_system_indexes">
<term><varname>ignore_system_indexes</varname> (<type>boolean</type>)
<indexterm>

View File

@ -98,10 +98,11 @@ Suppose you want to define a node Foo:
node types to find all the places to touch.
(Except for frequently-created nodes, don't bother writing a creator
function in makefuncs.c.)
4. Consider testing your new code with COPY_PARSE_PLAN_TREES,
WRITE_READ_PARSE_PLAN_TREES, and RAW_EXPRESSION_COVERAGE_TEST to ensure
support has been added everywhere that it's necessary; see
pg_config_manual.h about these.
4. Consider testing your new code with debug_copy_parse_plan_trees,
debug_write_read_parse_plan_trees, and
debug_raw_expression_coverage_test to ensure support has been added
everywhere that it's necessary (e.g., run the tests with
PG_TEST_INITDB_EXTRA_OPTS='-c debug_...=on').
Adding a new node type moves the numbers associated with existing
tags, so you'll need to recompile the whole tree after doing this.

View File

@ -32,7 +32,7 @@
static const char *pg_strtok_ptr = NULL;
/* State flag that determines how readfuncs.c should treat location fields */
#ifdef WRITE_READ_PARSE_PLAN_TREES
#ifdef DEBUG_NODE_TESTS_ENABLED
bool restore_location_fields = false;
#endif
@ -43,14 +43,14 @@ bool restore_location_fields = false;
*
* restore_loc_fields instructs readfuncs.c whether to restore location
* fields rather than set them to -1. This is currently only supported
* in builds with the WRITE_READ_PARSE_PLAN_TREES debugging flag set.
* in builds with DEBUG_NODE_TESTS_ENABLED defined.
*/
static void *
stringToNodeInternal(const char *str, bool restore_loc_fields)
{
void *retval;
const char *save_strtok;
#ifdef WRITE_READ_PARSE_PLAN_TREES
#ifdef DEBUG_NODE_TESTS_ENABLED
bool save_restore_location_fields;
#endif
@ -67,7 +67,7 @@ stringToNodeInternal(const char *str, bool restore_loc_fields)
/*
* If enabled, likewise save/restore the location field handling flag.
*/
#ifdef WRITE_READ_PARSE_PLAN_TREES
#ifdef DEBUG_NODE_TESTS_ENABLED
save_restore_location_fields = restore_location_fields;
restore_location_fields = restore_loc_fields;
#endif
@ -76,7 +76,7 @@ stringToNodeInternal(const char *str, bool restore_loc_fields)
pg_strtok_ptr = save_strtok;
#ifdef WRITE_READ_PARSE_PLAN_TREES
#ifdef DEBUG_NODE_TESTS_ENABLED
restore_location_fields = save_restore_location_fields;
#endif
@ -92,7 +92,7 @@ stringToNode(const char *str)
return stringToNodeInternal(str, false);
}
#ifdef WRITE_READ_PARSE_PLAN_TREES
#ifdef DEBUG_NODE_TESTS_ENABLED
void *
stringToNodeWithLocations(const char *str)

View File

@ -19,7 +19,7 @@
*
* However, if restore_location_fields is true, we do restore location
* fields from the string. This is currently intended only for use by the
* WRITE_READ_PARSE_PLAN_TREES test code, which doesn't want to cause
* debug_write_read_parse_plan_trees test code, which doesn't want to cause
* any change in the node contents.
*
*-------------------------------------------------------------------------
@ -118,7 +118,7 @@
local_node->fldname = nullable_string(token, length)
/* Read a parse location field (and possibly throw away the value) */
#ifdef WRITE_READ_PARSE_PLAN_TREES
#ifdef DEBUG_NODE_TESTS_ENABLED
#define READ_LOCATION_FIELD(fldname) \
token = pg_strtok(&length); /* skip :fldname */ \
token = pg_strtok(&length); /* get field value */ \

View File

@ -50,6 +50,7 @@
#include "parser/parsetree.h"
#include "utils/backend_status.h"
#include "utils/builtins.h"
#include "utils/guc.h"
#include "utils/rel.h"
#include "utils/syscache.h"
@ -84,7 +85,7 @@ static Query *transformCallStmt(ParseState *pstate,
CallStmt *stmt);
static void transformLockingClause(ParseState *pstate, Query *qry,
LockingClause *lc, bool pushedDown);
#ifdef RAW_EXPRESSION_COVERAGE_TEST
#ifdef DEBUG_NODE_TESTS_ENABLED
static bool test_raw_expression_coverage(Node *node, void *context);
#endif
@ -312,25 +313,30 @@ transformStmt(ParseState *pstate, Node *parseTree)
{
Query *result;
#ifdef DEBUG_NODE_TESTS_ENABLED
/*
* 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.
* We apply debug_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))
if (Debug_raw_expression_coverage_test)
{
case T_SelectStmt:
case T_InsertStmt:
case T_UpdateStmt:
case T_DeleteStmt:
case T_MergeStmt:
(void) test_raw_expression_coverage(parseTree, NULL);
break;
default:
break;
switch (nodeTag(parseTree))
{
case T_SelectStmt:
case T_InsertStmt:
case T_UpdateStmt:
case T_DeleteStmt:
case T_MergeStmt:
(void) test_raw_expression_coverage(parseTree, NULL);
break;
default:
break;
}
}
#endif /* RAW_EXPRESSION_COVERAGE_TEST */
#endif /* DEBUG_NODE_TESTS_ENABLED */
/*
* Caution: when changing the set of statement types that have non-default
@ -3575,6 +3581,7 @@ applyLockingClause(Query *qry, Index rtindex,
qry->rowMarks = lappend(qry->rowMarks, rc);
}
#ifdef DEBUG_NODE_TESTS_ENABLED
/*
* Coverage testing for raw_expression_tree_walker().
*
@ -3583,8 +3590,6 @@ applyLockingClause(Query *qry, Index rtindex,
* 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)
{
@ -3594,5 +3599,4 @@ test_raw_expression_coverage(Node *node, void *context)
test_raw_expression_coverage,
context);
}
#endif /* RAW_EXPRESSION_COVERAGE_TEST */
#endif /* DEBUG_NODE_TESTS_ENABLED */

View File

@ -622,8 +622,10 @@ pg_parse_query(const char *query_string)
if (log_parser_stats)
ShowUsage("PARSER STATISTICS");
#ifdef COPY_PARSE_PLAN_TREES
#ifdef DEBUG_NODE_TESTS_ENABLED
/* Optional debugging check: pass raw parsetrees through copyObject() */
if (Debug_copy_parse_plan_trees)
{
List *new_list = copyObject(raw_parsetree_list);
@ -633,13 +635,12 @@ pg_parse_query(const char *query_string)
else
raw_parsetree_list = new_list;
}
#endif
/*
* Optional debugging check: pass raw parsetrees through
* outfuncs/readfuncs
*/
#ifdef WRITE_READ_PARSE_PLAN_TREES
if (Debug_write_read_parse_plan_trees)
{
char *str = nodeToStringWithLocations(raw_parsetree_list);
List *new_list = stringToNodeWithLocations(str);
@ -651,7 +652,8 @@ pg_parse_query(const char *query_string)
else
raw_parsetree_list = new_list;
}
#endif
#endif /* DEBUG_NODE_TESTS_ENABLED */
TRACE_POSTGRESQL_QUERY_PARSE_DONE(query_string);
@ -826,8 +828,10 @@ pg_rewrite_query(Query *query)
if (log_parser_stats)
ShowUsage("REWRITER STATISTICS");
#ifdef COPY_PARSE_PLAN_TREES
#ifdef DEBUG_NODE_TESTS_ENABLED
/* Optional debugging check: pass querytree through copyObject() */
if (Debug_copy_parse_plan_trees)
{
List *new_list;
@ -838,10 +842,9 @@ pg_rewrite_query(Query *query)
else
querytree_list = new_list;
}
#endif
#ifdef WRITE_READ_PARSE_PLAN_TREES
/* Optional debugging check: pass querytree through outfuncs/readfuncs */
if (Debug_write_read_parse_plan_trees)
{
List *new_list = NIL;
ListCell *lc;
@ -868,7 +871,8 @@ pg_rewrite_query(Query *query)
else
querytree_list = new_list;
}
#endif
#endif /* DEBUG_NODE_TESTS_ENABLED */
if (Debug_print_rewritten)
elog_node_display(LOG, "rewritten parse tree", querytree_list,
@ -906,8 +910,10 @@ pg_plan_query(Query *querytree, const char *query_string, int cursorOptions,
if (log_planner_stats)
ShowUsage("PLANNER STATISTICS");
#ifdef COPY_PARSE_PLAN_TREES
#ifdef DEBUG_NODE_TESTS_ENABLED
/* Optional debugging check: pass plan tree through copyObject() */
if (Debug_copy_parse_plan_trees)
{
PlannedStmt *new_plan = copyObject(plan);
@ -923,10 +929,9 @@ pg_plan_query(Query *querytree, const char *query_string, int cursorOptions,
#endif
plan = new_plan;
}
#endif
#ifdef WRITE_READ_PARSE_PLAN_TREES
/* Optional debugging check: pass plan tree through outfuncs/readfuncs */
if (Debug_write_read_parse_plan_trees)
{
char *str;
PlannedStmt *new_plan;
@ -947,7 +952,8 @@ pg_plan_query(Query *querytree, const char *query_string, int cursorOptions,
#endif
plan = new_plan;
}
#endif
#endif /* DEBUG_NODE_TESTS_ENABLED */
/*
* Print plan if debugging.

View File

@ -493,6 +493,12 @@ bool Debug_print_parse = false;
bool Debug_print_rewritten = false;
bool Debug_pretty_print = true;
#ifdef DEBUG_NODE_TESTS_ENABLED
bool Debug_copy_parse_plan_trees;
bool Debug_write_read_parse_plan_trees;
bool Debug_raw_expression_coverage_test;
#endif
bool log_parser_stats = false;
bool log_planner_stats = false;
bool log_executor_stats = false;
@ -1294,6 +1300,59 @@ struct config_bool ConfigureNamesBool[] =
false,
NULL, NULL, NULL
},
#ifdef DEBUG_NODE_TESTS_ENABLED
{
{"debug_copy_parse_plan_trees", PGC_SUSET, DEVELOPER_OPTIONS,
gettext_noop("Set this to force all parse and plan trees to be passed through "
"copyObject(), to facilitate catching errors and omissions in "
"copyObject()."),
NULL,
GUC_NOT_IN_SAMPLE
},
&Debug_copy_parse_plan_trees,
/* support for legacy compile-time setting */
#ifdef COPY_PARSE_PLAN_TREES
true,
#else
false,
#endif
NULL, NULL, NULL
},
{
{"debug_write_read_parse_plan_trees", PGC_SUSET, DEVELOPER_OPTIONS,
gettext_noop("Set this to force all parse and plan trees to be passed through "
"outfuncs.c/readfuncs.c, to facilitate catching errors and omissions in "
"those modules."),
NULL,
GUC_NOT_IN_SAMPLE
},
&Debug_write_read_parse_plan_trees,
/* support for legacy compile-time setting */
#ifdef WRITE_READ_PARSE_PLAN_TREES
true,
#else
false,
#endif
NULL, NULL, NULL
},
{
{"debug_raw_expression_coverage_test", PGC_SUSET, DEVELOPER_OPTIONS,
gettext_noop("Set 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."),
NULL,
GUC_NOT_IN_SAMPLE
},
&Debug_raw_expression_coverage_test,
/* support for legacy compile-time setting */
#ifdef RAW_EXPRESSION_COVERAGE_TEST
true,
#else
false,
#endif
NULL, NULL, NULL
},
#endif /* DEBUG_NODE_TESTS_ENABLED */
{
{"debug_print_parse", PGC_USERSET, LOGGING_WHAT,
gettext_noop("Logs each query's parse tree."),

View File

@ -202,7 +202,7 @@ extern char *bmsToString(const struct Bitmapset *bms);
* nodes/{readfuncs.c,read.c}
*/
extern void *stringToNode(const char *str);
#ifdef WRITE_READ_PARSE_PLAN_TREES
#ifdef DEBUG_NODE_TESTS_ENABLED
extern void *stringToNodeWithLocations(const char *str);
#endif
extern struct Bitmapset *readBitmapset(void);

View File

@ -19,7 +19,7 @@
/*
* variable in read.c that needs to be accessible to readfuncs.c
*/
#ifdef WRITE_READ_PARSE_PLAN_TREES
#ifdef DEBUG_NODE_TESTS_ENABLED
extern PGDLLIMPORT bool restore_location_fields;
#endif

View File

@ -321,11 +321,25 @@
/* #define RECOVER_RELATION_BUILD_MEMORY 1 */ /* Force enable */
/*
* Define this to force all parse and plan trees to be passed through
* copyObject(), to facilitate catching errors and omissions in
* copyObject().
* Define DEBUG_NODE_TESTS_ENABLED to enable use of the GUCs
* debug_copy_parse_plan_trees, debug_write_read_parse_plan_trees, and
* debug_raw_expression_coverage_test, to test coverage of node support
* functions in src/backend/nodes/.
*
* USE_ASSERT_CHECKING builds default to enabling this.
*/
/* #define COPY_PARSE_PLAN_TREES */
/* #define DEBUG_NODE_TESTS_ENABLED */
#if defined(USE_ASSERT_CHECKING) && !defined(DEBUG_NODE_TESTS_ENABLED)
#define DEBUG_NODE_TESTS_ENABLED
#endif
/*
* Backwards compatibility for the older compile-time-only node-tests macros.
*/
#if !defined(DEBUG_NODE_TESTS_ENABLED) && (defined(COPY_PARSE_PLAN_TREES) || defined(WRITE_READ_PARSE_PLAN_TREES) || defined(RAW_EXPRESSION_COVERAGE_TEST))
#define DEBUG_NODE_TESTS_ENABLED
#endif
/*
* Define this to force Bitmapset reallocation on each modification. Helps
@ -333,20 +347,6 @@
*/
/* #define REALLOCATE_BITMAPSETS */
/*
* Define this to force all parse and plan trees to be passed through
* outfuncs.c/readfuncs.c, to facilitate catching errors and omissions in
* those modules.
*/
/* #define WRITE_READ_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.
*/

View File

@ -245,6 +245,12 @@ extern PGDLLIMPORT bool Debug_print_parse;
extern PGDLLIMPORT bool Debug_print_rewritten;
extern PGDLLIMPORT bool Debug_pretty_print;
#ifdef DEBUG_NODE_TESTS_ENABLED
extern PGDLLIMPORT bool Debug_copy_parse_plan_trees;
extern PGDLLIMPORT bool Debug_write_read_parse_plan_trees;
extern PGDLLIMPORT bool Debug_raw_expression_coverage_test;
#endif
extern PGDLLIMPORT bool log_parser_stats;
extern PGDLLIMPORT bool log_planner_stats;
extern PGDLLIMPORT bool log_executor_stats;