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
Appearance settings

Commit 6777080

Browse filesBrowse files
tglsfdctglsfdc
committed
Explicitly support the case that a plancache's raw_parse_tree is NULL.
This only happens if a client issues a Parse message with an empty query string, which is a bit odd; but since it is explicitly called out as legal by our FE/BE protocol spec, we'd probably better continue to allow it. Fix by adding tests everywhere that the raw_parse_tree field is passed to functions that don't or shouldn't accept NULL. Also make it clear in the relevant comments that NULL is an expected case. This reverts commits a73c9db and 2e9650c, which fixed specific crash symptoms by hacking things at what now seems to be the wrong end, ie the callee functions. Making the callees allow NULL is superficially more robust, but it's not always true that there is a defensible thing for the callee to do in such cases. The caller has more context and is better able to decide what the empty-query case ought to do. Per followup discussion of bug #11335. Back-patch to 9.2. The code before that is sufficiently different that it would require development of a separate patch, which doesn't seem worthwhile for what is believed to be an essentially cosmetic change.
1 parent ec5896a commit 6777080
Copy full SHA for 6777080

File tree

Expand file treeCollapse file tree

6 files changed

+15
-15
lines changed
Filter options
Expand file treeCollapse file tree

6 files changed

+15
-15
lines changed

‎src/backend/executor/spi.c

Copy file name to clipboardExpand all lines: src/backend/executor/spi.c
+3-1Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2037,7 +2037,9 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,
20372037
* Parameter datatypes are driven by parserSetup hook if provided,
20382038
* otherwise we use the fixed parameter list.
20392039
*/
2040-
if (plan->parserSetup != NULL)
2040+
if (parsetree == NULL)
2041+
stmt_list = NIL;
2042+
else if (plan->parserSetup != NULL)
20412043
{
20422044
Assert(plan->nargs == 0);
20432045
stmt_list = pg_analyze_and_rewrite_params(parsetree,

‎src/backend/parser/analyze.c

Copy file name to clipboardExpand all lines: src/backend/parser/analyze.c
+1-5Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -288,17 +288,13 @@ transformStmt(ParseState *pstate, Node *parseTree)
288288
* Returns true if a snapshot must be set before doing parse analysis
289289
* on the given raw parse tree.
290290
*
291-
* Classification here should match transformStmt(); but we also have to
292-
* allow a NULL input (for Parse/Bind of an empty query string).
291+
* Classification here should match transformStmt().
293292
*/
294293
bool
295294
analyze_requires_snapshot(Node *parseTree)
296295
{
297296
bool result;
298297

299-
if (parseTree == NULL)
300-
return false;
301-
302298
switch (nodeTag(parseTree))
303299
{
304300
/*

‎src/backend/tcop/postgres.c

Copy file name to clipboardExpand all lines: src/backend/tcop/postgres.c
+3-1Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1546,7 +1546,9 @@ exec_bind_message(StringInfo input_message)
15461546
* snapshot active till we're done, so that plancache.c doesn't have to
15471547
* take new ones.
15481548
*/
1549-
if (numParams > 0 || analyze_requires_snapshot(psrc->raw_parse_tree))
1549+
if (numParams > 0 ||
1550+
(psrc->raw_parse_tree &&
1551+
analyze_requires_snapshot(psrc->raw_parse_tree)))
15501552
{
15511553
PushActiveSnapshot(GetTransactionSnapshot());
15521554
snapshot_set = true;

‎src/backend/tcop/utility.c

Copy file name to clipboardExpand all lines: src/backend/tcop/utility.c
+1-4Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2485,9 +2485,6 @@ GetCommandLogLevel(Node *parsetree)
24852485
{
24862486
LogStmtLevel lev;
24872487

2488-
if (parsetree == NULL)
2489-
return LOGSTMT_ALL;
2490-
24912488
switch (nodeTag(parsetree))
24922489
{
24932490
/* raw plannable queries */
@@ -2592,7 +2589,7 @@ GetCommandLogLevel(Node *parsetree)
25922589

25932590
/* Look through an EXECUTE to the referenced stmt */
25942591
ps = FetchPreparedStatement(stmt->name, false);
2595-
if (ps)
2592+
if (ps && ps->plansource->raw_parse_tree)
25962593
lev = GetCommandLogLevel(ps->plansource->raw_parse_tree);
25972594
else
25982595
lev = LOGSTMT_ALL;

‎src/backend/utils/cache/plancache.c

Copy file name to clipboardExpand all lines: src/backend/utils/cache/plancache.c
+6-3Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ InitPlanCache(void)
141141
* Once constructed, the cached plan can be made longer-lived, if needed,
142142
* by calling SaveCachedPlan.
143143
*
144-
* raw_parse_tree: output of raw_parser()
144+
* raw_parse_tree: output of raw_parser(), or NULL if empty query
145145
* query_string: original query text
146146
* commandTag: compile-time-constant tag for query, or NULL if empty query
147147
*/
@@ -232,7 +232,7 @@ CreateCachedPlan(Node *raw_parse_tree,
232232
* invalidation, so plan use must be completed in the current transaction,
233233
* and DDL that might invalidate the querytree_list must be avoided as well.
234234
*
235-
* raw_parse_tree: output of raw_parser()
235+
* raw_parse_tree: output of raw_parser(), or NULL if empty query
236236
* query_string: original query text
237237
* commandTag: compile-time-constant tag for query, or NULL if empty query
238238
*/
@@ -699,7 +699,9 @@ RevalidateCachedQuery(CachedPlanSource *plansource)
699699
* the cache.
700700
*/
701701
rawtree = copyObject(plansource->raw_parse_tree);
702-
if (plansource->parserSetup != NULL)
702+
if (rawtree == NULL)
703+
tlist = NIL;
704+
else if (plansource->parserSetup != NULL)
703705
tlist = pg_analyze_and_rewrite_params(rawtree,
704706
plansource->query_string,
705707
plansource->parserSetup,
@@ -928,6 +930,7 @@ BuildCachedPlan(CachedPlanSource *plansource, List *qlist,
928930
*/
929931
snapshot_set = false;
930932
if (!ActiveSnapshotSet() &&
933+
plansource->raw_parse_tree &&
931934
analyze_requires_snapshot(plansource->raw_parse_tree))
932935
{
933936
PushActiveSnapshot(GetTransactionSnapshot());

‎src/include/utils/plancache.h

Copy file name to clipboardExpand all lines: src/include/utils/plancache.h
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@
7676
typedef struct CachedPlanSource
7777
{
7878
int magic; /* should equal CACHEDPLANSOURCE_MAGIC */
79-
Node *raw_parse_tree; /* output of raw_parser() */
79+
Node *raw_parse_tree; /* output of raw_parser(), or NULL */
8080
const char *query_string; /* source text of query */
8181
const char *commandTag; /* command tag (a constant!), or NULL */
8282
Oid *param_types; /* array of parameter type OIDs, or NULL */

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.