Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitd0cfc3d

Browse files
committed
Add a debugging option to stress-test outfuncs.c and readfuncs.c.
In the normal course of operation, query trees will be serialized only ifthey are stored as views or rules; and plan trees will be serialized onlyif they get passed to parallel-query workers. This leaves an awful lot ofopportunity for bugs/oversights to not get detected, as indeed we've justbeen reminded of the hard way.To improve matters, this patch adds a new compile optionWRITE_READ_PARSE_PLAN_TREES, which is modeled on the longstanding optionCOPY_PARSE_PLAN_TREES; but instead of passing all parse and plan treesthrough copyObject, it passes them through nodeToString + stringToNode.Enabling this option in a buildfarm animal or two will catch problemsat least for cases that are exercised by the regression tests.A small problem with this idea is that readfuncs.c historically hasdiscarded location fields, on the reasonable grounds that parselocations in a retrieved view are not relevant to the current query.But doing that in WRITE_READ_PARSE_PLAN_TREES breaks pg_stat_statements,and it could cause problems for future improvements that might try toreport error locations at runtime. To fix that, provide a variantbehavior in readfuncs.c that makes it restore location fields whentold to.In passing, const-ify the string arguments of stringToNode and itssubsidiary functions, just because it annoyed me that they weren'tconst already.Discussion:https://postgr.es/m/17114.1537138992@sss.pgh.pa.us
1 parentdb1071d commitd0cfc3d

File tree

7 files changed

+170
-29
lines changed

7 files changed

+170
-29
lines changed

‎src/backend/nodes/read.c

Lines changed: 56 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,18 +28,30 @@
2828

2929

3030
/* Static state for pg_strtok */
31-
staticchar*pg_strtok_ptr=NULL;
31+
staticconstchar*pg_strtok_ptr=NULL;
32+
33+
/* State flag that determines how readfuncs.c should treat location fields */
34+
#ifdefWRITE_READ_PARSE_PLAN_TREES
35+
boolrestore_location_fields= false;
36+
#endif
3237

3338

3439
/*
3540
* stringToNode -
36-
* returns a Node with a given legal ASCII representation
41+
* builds a Node tree from its string representation (assumed valid)
42+
*
43+
* restore_loc_fields instructs readfuncs.c whether to restore location
44+
* fields rather than set them to -1. This is currently only supported
45+
* in builds with the WRITE_READ_PARSE_PLAN_TREES debugging flag set.
3746
*/
38-
void*
39-
stringToNode(char*str)
47+
staticvoid*
48+
stringToNodeInternal(constchar*str,boolrestore_loc_fields)
4049
{
41-
char*save_strtok;
4250
void*retval;
51+
constchar*save_strtok;
52+
#ifdefWRITE_READ_PARSE_PLAN_TREES
53+
boolsave_restore_location_fields;
54+
#endif
4355

4456
/*
4557
* We save and restore the pre-existing state of pg_strtok. This makes the
@@ -51,13 +63,45 @@ stringToNode(char *str)
5163

5264
pg_strtok_ptr=str;/* point pg_strtok at the string to read */
5365

66+
/*
67+
* If enabled, likewise save/restore the location field handling flag.
68+
*/
69+
#ifdefWRITE_READ_PARSE_PLAN_TREES
70+
save_restore_location_fields=restore_location_fields;
71+
restore_location_fields=restore_loc_fields;
72+
#endif
73+
5474
retval=nodeRead(NULL,0);/* do the reading */
5575

5676
pg_strtok_ptr=save_strtok;
5777

78+
#ifdefWRITE_READ_PARSE_PLAN_TREES
79+
restore_location_fields=save_restore_location_fields;
80+
#endif
81+
5882
returnretval;
5983
}
6084

85+
/*
86+
* Externally visible entry points
87+
*/
88+
void*
89+
stringToNode(constchar*str)
90+
{
91+
returnstringToNodeInternal(str, false);
92+
}
93+
94+
#ifdefWRITE_READ_PARSE_PLAN_TREES
95+
96+
void*
97+
stringToNodeWithLocations(constchar*str)
98+
{
99+
returnstringToNodeInternal(str, true);
100+
}
101+
102+
#endif
103+
104+
61105
/*****************************************************************************
62106
*
63107
* the lisp token parser
@@ -104,11 +148,11 @@ stringToNode(char *str)
104148
* code should add backslashes to a string constant to ensure it is treated
105149
* as a single token.
106150
*/
107-
char*
151+
constchar*
108152
pg_strtok(int*length)
109153
{
110-
char*local_str;/* working pointer to string */
111-
char*ret_str;/* start of token to return */
154+
constchar*local_str;/* working pointer to string */
155+
constchar*ret_str;/* start of token to return */
112156

113157
local_str=pg_strtok_ptr;
114158

@@ -166,7 +210,7 @@ pg_strtok(int *length)
166210
* any protective backslashes in the token are removed.
167211
*/
168212
char*
169-
debackslash(char*token,intlength)
213+
debackslash(constchar*token,intlength)
170214
{
171215
char*result=palloc(length+1);
172216
char*ptr=result;
@@ -198,10 +242,10 @@ debackslash(char *token, int length)
198242
* Assumption: the ascii representation is legal
199243
*/
200244
staticNodeTag
201-
nodeTokenType(char*token,intlength)
245+
nodeTokenType(constchar*token,intlength)
202246
{
203247
NodeTagretval;
204-
char*numptr;
248+
constchar*numptr;
205249
intnumlen;
206250

207251
/*
@@ -269,7 +313,7 @@ nodeTokenType(char *token, int length)
269313
* this should only be invoked from within a stringToNode operation).
270314
*/
271315
void*
272-
nodeRead(char*token,inttok_len)
316+
nodeRead(constchar*token,inttok_len)
273317
{
274318
Node*result;
275319
NodeTagtype;

‎src/backend/nodes/readfuncs.c

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,14 @@
1717
* never read executor state trees, either.
1818
*
1919
* Parse location fields are written out by outfuncs.c, but only for
20-
*possibledebugging use. When reading a location field, we discard
20+
* debugging use. When reading a location field, we normally discard
2121
* the stored value and set the location field to -1 (ie, "unknown").
2222
* This is because nodes coming from a stored rule should not be thought
2323
* to have a known location in the current query's text.
24+
* However, if restore_location_fields is true, we do restore location
25+
* fields from the string. This is currently intended only for use by the
26+
* WRITE_READ_PARSE_PLAN_TREES test code, which doesn't want to cause
27+
* any change in the node contents.
2428
*
2529
*-------------------------------------------------------------------------
2630
*/
@@ -51,7 +55,7 @@
5155

5256
/* And a few guys need only the pg_strtok support fields */
5357
#defineREAD_TEMP_LOCALS()\
54-
char *token;\
58+
const char *token;\
5559
intlength
5660

5761
/* ... but most need both */
@@ -120,12 +124,19 @@
120124
token=pg_strtok(&length);/* get field value */ \
121125
local_node->fldname=nullable_string(token,length)
122126

123-
/* Read a parse location field (and throw away the value, per notes above) */
127+
/* Read a parse location field (and possibly throw away the value) */
128+
#ifdefWRITE_READ_PARSE_PLAN_TREES
129+
#defineREAD_LOCATION_FIELD(fldname) \
130+
token = pg_strtok(&length);/* skip :fldname */ \
131+
token=pg_strtok(&length);/* get field value */ \
132+
local_node->fldname=restore_location_fields ?atoi(token) :-1
133+
#else
124134
#defineREAD_LOCATION_FIELD(fldname) \
125135
token = pg_strtok(&length);/* skip :fldname */ \
126136
token=pg_strtok(&length);/* get field value */ \
127137
(void)token;/* in case not used elsewhere */ \
128138
local_node->fldname=-1/* set field to "unknown" */
139+
#endif
129140

130141
/* Read a Node field */
131142
#defineREAD_NODE_FIELD(fldname) \
@@ -2804,7 +2815,7 @@ readDatum(bool typbyval)
28042815
Sizelength,
28052816
i;
28062817
inttokenLength;
2807-
char*token;
2818+
constchar*token;
28082819
Datumres;
28092820
char*s;
28102821

@@ -2817,7 +2828,7 @@ readDatum(bool typbyval)
28172828
token=pg_strtok(&tokenLength);/* read the '[' */
28182829
if (token==NULL||token[0]!='[')
28192830
elog(ERROR,"expected \"[\" to start datum, but got \"%s\"; length = %zu",
2820-
token ?(constchar*)token :"[NULL]",length);
2831+
token ?token :"[NULL]",length);
28212832

28222833
if (typbyval)
28232834
{
@@ -2847,7 +2858,7 @@ readDatum(bool typbyval)
28472858
token=pg_strtok(&tokenLength);/* read the ']' */
28482859
if (token==NULL||token[0]!=']')
28492860
elog(ERROR,"expected \"]\" to end datum, but got \"%s\"; length = %zu",
2850-
token ?(constchar*)token :"[NULL]",length);
2861+
token ?token :"[NULL]",length);
28512862

28522863
returnres;
28532864
}
@@ -2860,7 +2871,7 @@ readAttrNumberCols(int numCols)
28602871
{
28612872
inttokenLength,
28622873
i;
2863-
char*token;
2874+
constchar*token;
28642875
AttrNumber*attr_vals;
28652876

28662877
if (numCols <=0)
@@ -2884,7 +2895,7 @@ readOidCols(int numCols)
28842895
{
28852896
inttokenLength,
28862897
i;
2887-
char*token;
2898+
constchar*token;
28882899
Oid*oid_vals;
28892900

28902901
if (numCols <=0)
@@ -2908,7 +2919,7 @@ readIntCols(int numCols)
29082919
{
29092920
inttokenLength,
29102921
i;
2911-
char*token;
2922+
constchar*token;
29122923
int*int_vals;
29132924

29142925
if (numCols <=0)
@@ -2932,7 +2943,7 @@ readBoolCols(int numCols)
29322943
{
29332944
inttokenLength,
29342945
i;
2935-
char*token;
2946+
constchar*token;
29362947
bool*bool_vals;
29372948

29382949
if (numCols <=0)

‎src/backend/parser/parse_relation.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1335,7 +1335,6 @@ addRangeTableEntryForSubquery(ParseState *pstate,
13351335
Assert(pstate!=NULL);
13361336

13371337
rte->rtekind=RTE_SUBQUERY;
1338-
rte->relid=InvalidOid;
13391338
rte->subquery=subquery;
13401339
rte->alias=alias;
13411340

‎src/backend/tcop/postgres.c

Lines changed: 72 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -633,6 +633,12 @@ pg_parse_query(const char *query_string)
633633
}
634634
#endif
635635

636+
/*
637+
* Currently, outfuncs/readfuncs support is missing for many raw parse
638+
* tree nodes, so we don't try to implement WRITE_READ_PARSE_PLAN_TREES
639+
* here.
640+
*/
641+
636642
TRACE_POSTGRESQL_QUERY_PARSE_DONE(query_string);
637643

638644
returnraw_parsetree_list;
@@ -763,7 +769,7 @@ pg_rewrite_query(Query *query)
763769
ShowUsage("REWRITER STATISTICS");
764770

765771
#ifdefCOPY_PARSE_PLAN_TREES
766-
/* Optional debugging check: pass querytreeoutputthrough copyObject() */
772+
/* Optional debugging check: pass querytree through copyObject() */
767773
{
768774
List*new_list;
769775

@@ -776,6 +782,46 @@ pg_rewrite_query(Query *query)
776782
}
777783
#endif
778784

785+
#ifdefWRITE_READ_PARSE_PLAN_TREES
786+
/* Optional debugging check: pass querytree through outfuncs/readfuncs */
787+
{
788+
List*new_list=NIL;
789+
ListCell*lc;
790+
791+
/*
792+
* We currently lack outfuncs/readfuncs support for most utility
793+
* statement types, so only attempt to write/read non-utility queries.
794+
*/
795+
foreach(lc,querytree_list)
796+
{
797+
Query*query=castNode(Query,lfirst(lc));
798+
799+
if (query->commandType!=CMD_UTILITY)
800+
{
801+
char*str=nodeToString(query);
802+
Query*new_query=stringToNodeWithLocations(str);
803+
804+
/*
805+
* queryId is not saved in stored rules, but we must preserve
806+
* it here to avoid breaking pg_stat_statements.
807+
*/
808+
new_query->queryId=query->queryId;
809+
810+
new_list=lappend(new_list,new_query);
811+
pfree(str);
812+
}
813+
else
814+
new_list=lappend(new_list,query);
815+
}
816+
817+
/* This checks both outfuncs/readfuncs and the equal() routines... */
818+
if (!equal(new_list,querytree_list))
819+
elog(WARNING,"outfuncs/readfuncs failed to produce equal parse tree");
820+
else
821+
querytree_list=new_list;
822+
}
823+
#endif
824+
779825
if (Debug_print_rewritten)
780826
elog_node_display(LOG,"rewritten parse tree",querytree_list,
781827
Debug_pretty_print);
@@ -812,7 +858,7 @@ pg_plan_query(Query *querytree, int cursorOptions, ParamListInfo boundParams)
812858
ShowUsage("PLANNER STATISTICS");
813859

814860
#ifdefCOPY_PARSE_PLAN_TREES
815-
/* Optional debugging check: pass planoutput through copyObject() */
861+
/* Optional debugging check: pass plantree through copyObject() */
816862
{
817863
PlannedStmt*new_plan=copyObject(plan);
818864

@@ -830,6 +876,30 @@ pg_plan_query(Query *querytree, int cursorOptions, ParamListInfo boundParams)
830876
}
831877
#endif
832878

879+
#ifdefWRITE_READ_PARSE_PLAN_TREES
880+
/* Optional debugging check: pass plan tree through outfuncs/readfuncs */
881+
{
882+
char*str;
883+
PlannedStmt*new_plan;
884+
885+
str=nodeToString(plan);
886+
new_plan=stringToNodeWithLocations(str);
887+
pfree(str);
888+
889+
/*
890+
* equal() currently does not have routines to compare Plan nodes, so
891+
* don't try to test equality here. Perhaps fix someday?
892+
*/
893+
#ifdefNOT_USED
894+
/* This checks both outfuncs/readfuncs and the equal() routines... */
895+
if (!equal(new_plan,plan))
896+
elog(WARNING,"outfuncs/readfuncs failed to produce an equal plan tree");
897+
else
898+
#endif
899+
plan=new_plan;
900+
}
901+
#endif
902+
833903
/*
834904
* Print plan if debugging.
835905
*/

‎src/include/nodes/nodes.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -610,7 +610,10 @@ extern char *bmsToString(const struct Bitmapset *bms);
610610
/*
611611
* nodes/{readfuncs.c,read.c}
612612
*/
613-
externvoid*stringToNode(char*str);
613+
externvoid*stringToNode(constchar*str);
614+
#ifdefWRITE_READ_PARSE_PLAN_TREES
615+
externvoid*stringToNodeWithLocations(constchar*str);
616+
#endif
614617
externstructBitmapset*readBitmapset(void);
615618
externuintptr_treadDatum(booltypbyval);
616619
externbool*readBoolCols(intnumCols);

‎src/include/nodes/readfuncs.h

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,19 @@
1616

1717
#include"nodes/nodes.h"
1818

19+
/*
20+
* variable in read.c that needs to be accessible to readfuncs.c
21+
*/
22+
#ifdefWRITE_READ_PARSE_PLAN_TREES
23+
externboolrestore_location_fields;
24+
#endif
25+
1926
/*
2027
* prototypes for functions in read.c (the lisp token parser)
2128
*/
22-
externchar*pg_strtok(int*length);
23-
externchar*debackslash(char*token,intlength);
24-
externvoid*nodeRead(char*token,inttok_len);
29+
externconstchar*pg_strtok(int*length);
30+
externchar*debackslash(constchar*token,intlength);
31+
externvoid*nodeRead(constchar*token,inttok_len);
2532

2633
/*
2734
* prototypes for functions in readfuncs.c

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp