Skip to content

Commit a292c98

Browse files
peterepetere
committed
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
1 parent a67da49 commit a292c98

File tree

12 files changed

+212
-64
lines changed

12 files changed

+212
-64
lines changed

‎.cirrus.tasks.yml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,9 +133,10 @@ task:
133133
DISK_SIZE: 50
134134

135135
CCACHE_DIR: /tmp/ccache_dir
136-
CPPFLAGS: -DRELCACHE_FORCE_RELEASE -DCOPY_PARSE_PLAN_TREES -DWRITE_READ_PARSE_PLAN_TREES -DRAW_EXPRESSION_COVERAGE_TEST -DENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS
136+
CPPFLAGS: -DRELCACHE_FORCE_RELEASE -DENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS
137137
CFLAGS: -Og -ggdb
138138

139+
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
139140
PG_TEST_PG_UPGRADE_MODE: --link
140141

141142
<<: *freebsd_task_template

‎doc/src/sgml/config.sgml

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11425,6 +11425,29 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
1142511425
</listitem>
1142611426
</varlistentry>
1142711427

11428+
<varlistentry id="guc-debug-copy-parse-plan-trees" xreflabel="debug_copy_parse_plan_trees">
11429+
<term><varname>debug_copy_parse_plan_trees</varname> (<type>boolean</type>)
11430+
<indexterm>
11431+
<primary><varname>debug_copy_parse_plan_trees</varname> configuration parameter</primary>
11432+
</indexterm>
11433+
</term>
11434+
<listitem>
11435+
<para>
11436+
Enabling this forces all parse and plan trees to be passed through
11437+
<function>copyObject()</function>, to facilitate catching errors and
11438+
omissions in <function>copyObject()</function>. The default is off.
11439+
</para>
11440+
11441+
<para>
11442+
This parameter is only available when
11443+
<symbol>DEBUG_NODE_TESTS_ENABLED</symbol> was defined at compile time
11444+
(which happens automatically when using the
11445+
<application>configure</application> option
11446+
<option>--enable-cassert</option>).
11447+
</para>
11448+
</listitem>
11449+
</varlistentry>
11450+
1142811451
<varlistentry id="guc-debug-discard-caches" xreflabel="debug_discard_caches">
1142911452
<term><varname>debug_discard_caches</varname> (<type>integer</type>)
1143011453
<indexterm>
@@ -11540,6 +11563,54 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
1154011563
</listitem>
1154111564
</varlistentry>
1154211565

11566+
<varlistentry id="guc-debug-raw-expression-coverage-test" xreflabel="debug_raw_expression_coverage_test">
11567+
<term><varname>debug_raw_expression_coverage_test</varname> (<type>boolean</type>)
11568+
<indexterm>
11569+
<primary><varname>debug_raw_expression_coverage_test</varname> configuration parameter</primary>
11570+
</indexterm>
11571+
</term>
11572+
<listitem>
11573+
<para>
11574+
Enabling this forces all raw parse trees for DML statements to be
11575+
scanned by <function>raw_expression_tree_walker()</function>, to
11576+
facilitate catching errors and omissions in that function. The
11577+
default is off.
11578+
</para>
11579+
11580+
<para>
11581+
This parameter is only available when
11582+
<symbol>DEBUG_NODE_TESTS_ENABLED</symbol> was defined at compile time
11583+
(which happens automatically when using the
11584+
<application>configure</application> option
11585+
<option>--enable-cassert</option>).
11586+
</para>
11587+
</listitem>
11588+
</varlistentry>
11589+
11590+
<varlistentry id="guc-debug-write-read-parse-plan-trees" xreflabel="debug_write_read_parse_plan_trees">
11591+
<term><varname>debug_write_read_parse_plan_trees</varname> (<type>boolean</type>)
11592+
<indexterm>
11593+
<primary><varname>debug_write_read_parse_plan_trees</varname> configuration parameter</primary>
11594+
</indexterm>
11595+
</term>
11596+
<listitem>
11597+
<para>
11598+
Enabling this forces all parse and plan trees to be passed through
11599+
<filename>outfuncs.c</filename>/<filename>readfuncs.c</filename>, to
11600+
facilitate catching errors and omissions in those modules. The
11601+
default is off.
11602+
</para>
11603+
11604+
<para>
11605+
This parameter is only available when
11606+
<symbol>DEBUG_NODE_TESTS_ENABLED</symbol> was defined at compile time
11607+
(which happens automatically when using the
11608+
<application>configure</application> option
11609+
<option>--enable-cassert</option>).
11610+
</para>
11611+
</listitem>
11612+
</varlistentry>
11613+
1154311614
<varlistentry id="guc-ignore-system-indexes" xreflabel="ignore_system_indexes">
1154411615
<term><varname>ignore_system_indexes</varname> (<type>boolean</type>)
1154511616
<indexterm>

‎src/backend/nodes/README

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,11 @@ Suppose you want to define a node Foo:
9898
node types to find all the places to touch.
9999
(Except for frequently-created nodes, don't bother writing a creator
100100
function in makefuncs.c.)
101-
4. Consider testing your new code with COPY_PARSE_PLAN_TREES,
102-
WRITE_READ_PARSE_PLAN_TREES, and RAW_EXPRESSION_COVERAGE_TEST to ensure
103-
support has been added everywhere that it's necessary; see
104-
pg_config_manual.h about these.
101+
4. Consider testing your new code with debug_copy_parse_plan_trees,
102+
debug_write_read_parse_plan_trees, and
103+
debug_raw_expression_coverage_test to ensure support has been added
104+
everywhere that it's necessary (e.g., run the tests with
105+
PG_TEST_INITDB_EXTRA_OPTS='-c debug_...=on').
105106

106107
Adding a new node type moves the numbers associated with existing
107108
tags, so you'll need to recompile the whole tree after doing this.

‎src/backend/nodes/read.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
static const char *pg_strtok_ptr = NULL;
3333

3434
/* State flag that determines how readfuncs.c should treat location fields */
35-
#ifdef WRITE_READ_PARSE_PLAN_TREES
35+
#ifdef DEBUG_NODE_TESTS_ENABLED
3636
bool restore_location_fields = false;
3737
#endif
3838

@@ -43,14 +43,14 @@ bool restore_location_fields = false;
4343
*
4444
* restore_loc_fields instructs readfuncs.c whether to restore location
4545
* fields rather than set them to -1. This is currently only supported
46-
* in builds with the WRITE_READ_PARSE_PLAN_TREES debugging flag set.
46+
* in builds with DEBUG_NODE_TESTS_ENABLED defined.
4747
*/
4848
static void *
4949
stringToNodeInternal(const char *str, bool restore_loc_fields)
5050
{
5151
void *retval;
5252
const char *save_strtok;
53-
#ifdef WRITE_READ_PARSE_PLAN_TREES
53+
#ifdef DEBUG_NODE_TESTS_ENABLED
5454
bool save_restore_location_fields;
5555
#endif
5656

@@ -67,7 +67,7 @@ stringToNodeInternal(const char *str, bool restore_loc_fields)
6767
/*
6868
* If enabled, likewise save/restore the location field handling flag.
6969
*/
70-
#ifdef WRITE_READ_PARSE_PLAN_TREES
70+
#ifdef DEBUG_NODE_TESTS_ENABLED
7171
save_restore_location_fields = restore_location_fields;
7272
restore_location_fields = restore_loc_fields;
7373
#endif
@@ -76,7 +76,7 @@ stringToNodeInternal(const char *str, bool restore_loc_fields)
7676

7777
pg_strtok_ptr = save_strtok;
7878

79-
#ifdef WRITE_READ_PARSE_PLAN_TREES
79+
#ifdef DEBUG_NODE_TESTS_ENABLED
8080
restore_location_fields = save_restore_location_fields;
8181
#endif
8282

@@ -92,7 +92,7 @@ stringToNode(const char *str)
9292
return stringToNodeInternal(str, false);
9393
}
9494

95-
#ifdef WRITE_READ_PARSE_PLAN_TREES
95+
#ifdef DEBUG_NODE_TESTS_ENABLED
9696

9797
void *
9898
stringToNodeWithLocations(const char *str)

‎src/backend/nodes/readfuncs.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
*
2020
* However, if restore_location_fields is true, we do restore location
2121
* fields from the string. This is currently intended only for use by the
22-
* WRITE_READ_PARSE_PLAN_TREES test code, which doesn't want to cause
22+
* debug_write_read_parse_plan_trees test code, which doesn't want to cause
2323
* any change in the node contents.
2424
*
2525
*-------------------------------------------------------------------------
@@ -118,7 +118,7 @@
118118
local_node->fldname = nullable_string(token, length)
119119

120120
/* Read a parse location field (and possibly throw away the value) */
121-
#ifdef WRITE_READ_PARSE_PLAN_TREES
121+
#ifdef DEBUG_NODE_TESTS_ENABLED
122122
#define READ_LOCATION_FIELD(fldname) \
123123
token = pg_strtok(&length); /* skip :fldname */ \
124124
token = pg_strtok(&length); /* get field value */ \

‎src/backend/parser/analyze.c

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
#include "parser/parsetree.h"
5151
#include "utils/backend_status.h"
5252
#include "utils/builtins.h"
53+
#include "utils/guc.h"
5354
#include "utils/rel.h"
5455
#include "utils/syscache.h"
5556

@@ -84,7 +85,7 @@ static Query *transformCallStmt(ParseState *pstate,
8485
CallStmt *stmt);
8586
static void transformLockingClause(ParseState *pstate, Query *qry,
8687
LockingClause *lc, bool pushedDown);
87-
#ifdef RAW_EXPRESSION_COVERAGE_TEST
88+
#ifdef DEBUG_NODE_TESTS_ENABLED
8889
static bool test_raw_expression_coverage(Node *node, void *context);
8990
#endif
9091

@@ -312,25 +313,30 @@ transformStmt(ParseState *pstate, Node *parseTree)
312313
{
313314
Query *result;
314315

316+
#ifdef DEBUG_NODE_TESTS_ENABLED
317+
315318
/*
316-
* We apply RAW_EXPRESSION_COVERAGE_TEST testing to basic DML statements;
317-
* we can't just run it on everything because raw_expression_tree_walker()
318-
* doesn't claim to handle utility statements.
319+
* We apply debug_raw_expression_coverage_test testing to basic DML
320+
* statements; we can't just run it on everything because
321+
* raw_expression_tree_walker() doesn't claim to handle utility
322+
* statements.
319323
*/
320-
#ifdef RAW_EXPRESSION_COVERAGE_TEST
321-
switch (nodeTag(parseTree))
324+
if (Debug_raw_expression_coverage_test)
322325
{
323-
case T_SelectStmt:
324-
case T_InsertStmt:
325-
case T_UpdateStmt:
326-
case T_DeleteStmt:
327-
case T_MergeStmt:
328-
(void) test_raw_expression_coverage(parseTree, NULL);
329-
break;
330-
default:
331-
break;
326+
switch (nodeTag(parseTree))
327+
{
328+
case T_SelectStmt:
329+
case T_InsertStmt:
330+
case T_UpdateStmt:
331+
case T_DeleteStmt:
332+
case T_MergeStmt:
333+
(void) test_raw_expression_coverage(parseTree, NULL);
334+
break;
335+
default:
336+
break;
337+
}
332338
}
333-
#endif /* RAW_EXPRESSION_COVERAGE_TEST */
339+
#endif /* DEBUG_NODE_TESTS_ENABLED */
334340

335341
/*
336342
* Caution: when changing the set of statement types that have non-default
@@ -3575,6 +3581,7 @@ applyLockingClause(Query *qry, Index rtindex,
35753581
qry->rowMarks = lappend(qry->rowMarks, rc);
35763582
}
35773583

3584+
#ifdef DEBUG_NODE_TESTS_ENABLED
35783585
/*
35793586
* Coverage testing for raw_expression_tree_walker().
35803587
*
@@ -3583,8 +3590,6 @@ applyLockingClause(Query *qry, Index rtindex,
35833590
* applied in limited cases involving CTEs, and we don't really want to have
35843591
* to test everything inside as well as outside a CTE.
35853592
*/
3586-
#ifdef RAW_EXPRESSION_COVERAGE_TEST
3587-
35883593
static bool
35893594
test_raw_expression_coverage(Node *node, void *context)
35903595
{
@@ -3594,5 +3599,4 @@ test_raw_expression_coverage(Node *node, void *context)
35943599
test_raw_expression_coverage,
35953600
context);
35963601
}
3597-
3598-
#endif /* RAW_EXPRESSION_COVERAGE_TEST */
3602+
#endif /* DEBUG_NODE_TESTS_ENABLED */

‎src/backend/tcop/postgres.c

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -622,8 +622,10 @@ pg_parse_query(const char *query_string)
622622
if (log_parser_stats)
623623
ShowUsage("PARSER STATISTICS");
624624

625-
#ifdef COPY_PARSE_PLAN_TREES
625+
#ifdef DEBUG_NODE_TESTS_ENABLED
626+
626627
/* Optional debugging check: pass raw parsetrees through copyObject() */
628+
if (Debug_copy_parse_plan_trees)
627629
{
628630
List *new_list = copyObject(raw_parsetree_list);
629631

@@ -633,13 +635,12 @@ pg_parse_query(const char *query_string)
633635
else
634636
raw_parsetree_list = new_list;
635637
}
636-
#endif
637638

638639
/*
639640
* Optional debugging check: pass raw parsetrees through
640641
* outfuncs/readfuncs
641642
*/
642-
#ifdef WRITE_READ_PARSE_PLAN_TREES
643+
if (Debug_write_read_parse_plan_trees)
643644
{
644645
char *str = nodeToStringWithLocations(raw_parsetree_list);
645646
List *new_list = stringToNodeWithLocations(str);
@@ -651,7 +652,8 @@ pg_parse_query(const char *query_string)
651652
else
652653
raw_parsetree_list = new_list;
653654
}
654-
#endif
655+
656+
#endif /* DEBUG_NODE_TESTS_ENABLED */
655657

656658
TRACE_POSTGRESQL_QUERY_PARSE_DONE(query_string);
657659

@@ -826,8 +828,10 @@ pg_rewrite_query(Query *query)
826828
if (log_parser_stats)
827829
ShowUsage("REWRITER STATISTICS");
828830

829-
#ifdef COPY_PARSE_PLAN_TREES
831+
#ifdef DEBUG_NODE_TESTS_ENABLED
832+
830833
/* Optional debugging check: pass querytree through copyObject() */
834+
if (Debug_copy_parse_plan_trees)
831835
{
832836
List *new_list;
833837

@@ -838,10 +842,9 @@ pg_rewrite_query(Query *query)
838842
else
839843
querytree_list = new_list;
840844
}
841-
#endif
842845

843-
#ifdef WRITE_READ_PARSE_PLAN_TREES
844846
/* Optional debugging check: pass querytree through outfuncs/readfuncs */
847+
if (Debug_write_read_parse_plan_trees)
845848
{
846849
List *new_list = NIL;
847850
ListCell *lc;
@@ -868,7 +871,8 @@ pg_rewrite_query(Query *query)
868871
else
869872
querytree_list = new_list;
870873
}
871-
#endif
874+
875+
#endif /* DEBUG_NODE_TESTS_ENABLED */
872876

873877
if (Debug_print_rewritten)
874878
elog_node_display(LOG, "rewritten parse tree", querytree_list,
@@ -906,8 +910,10 @@ pg_plan_query(Query *querytree, const char *query_string, int cursorOptions,
906910
if (log_planner_stats)
907911
ShowUsage("PLANNER STATISTICS");
908912

909-
#ifdef COPY_PARSE_PLAN_TREES
913+
#ifdef DEBUG_NODE_TESTS_ENABLED
914+
910915
/* Optional debugging check: pass plan tree through copyObject() */
916+
if (Debug_copy_parse_plan_trees)
911917
{
912918
PlannedStmt *new_plan = copyObject(plan);
913919

@@ -923,10 +929,9 @@ pg_plan_query(Query *querytree, const char *query_string, int cursorOptions,
923929
#endif
924930
plan = new_plan;
925931
}
926-
#endif
927932

928-
#ifdef WRITE_READ_PARSE_PLAN_TREES
929933
/* Optional debugging check: pass plan tree through outfuncs/readfuncs */
934+
if (Debug_write_read_parse_plan_trees)
930935
{
931936
char *str;
932937
PlannedStmt *new_plan;
@@ -947,7 +952,8 @@ pg_plan_query(Query *querytree, const char *query_string, int cursorOptions,
947952
#endif
948953
plan = new_plan;
949954
}
950-
#endif
955+
956+
#endif /* DEBUG_NODE_TESTS_ENABLED */
951957

952958
/*
953959
* Print plan if debugging.

0 commit comments

Comments
 (0)