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

Commit7c337b6

Browse files
committed
Centralize the logic for protective copying of utility statements.
In the "simple Query" code path, it's fine for parse analysis orexecution of a utility statement to scribble on the statement's nodetree, since that'll just be thrown away afterwards. However it'snot fine if the node tree is in the plan cache, as then it'd becorrupted for subsequent executions. Up to now we've dealt withthat by having individual utility-statement functions applycopyObject() if they were going to modify the tree. But that'sprone to errors of omission. Bug #17053 from Charles Samborskishows that CREATE/ALTER DOMAIN didn't get this memo, and cancrash if executed repeatedly from plan cache.In the back branches, we'll just apply a narrow band-aid for that,but in HEAD it seems prudent to have a more principled fix thatwill close off the possibility of other similar bugs in future.Hence, let's hoist the responsibility for doing copyObject up intoProcessUtility from its children, thus ensuring that it happens forall utility statement types.Also, modify ProcessUtility's API so that its callers can tell itwhether a copy step is necessary. It turns out that in all cases,the immediate caller knows whether the node tree is transient, sothis doesn't involve a huge amount of code thrashing. In this way,while we lose a little bit in the execute-from-cache code path dueto sometimes copying node trees that wouldn't be mutated anyway,we gain something in the simple-Query code path by not copyingthrowaway node trees. Statements that are complex enough to beexpensive to copy are almost certainly ones that would have to becopied anyway, so the loss in the cache code path shouldn't be much.(Note that this whole problem applies only to utility statements.Optimizable statements don't have the issue because we long ago madethe executor treat Plan trees as read-only. Perhaps someday we willmake utility statement execution act likewise, but I'm not holdingmy breath.)Discussion:https://postgr.es/m/931771.1623893989@sss.pgh.pa.usDiscussion:https://postgr.es/m/17053-3ca3f501bbc212b4@postgresql.org
1 parent0a4efdc commit7c337b6

File tree

19 files changed

+56
-91
lines changed

19 files changed

+56
-91
lines changed

‎contrib/pg_stat_statements/pg_stat_statements.c

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,7 @@ static void pgss_ExecutorRun(QueryDesc *queryDesc,
320320
staticvoidpgss_ExecutorFinish(QueryDesc*queryDesc);
321321
staticvoidpgss_ExecutorEnd(QueryDesc*queryDesc);
322322
staticvoidpgss_ProcessUtility(PlannedStmt*pstmt,constchar*queryString,
323+
boolreadOnlyTree,
323324
ProcessUtilityContextcontext,ParamListInfoparams,
324325
QueryEnvironment*queryEnv,
325326
DestReceiver*dest,QueryCompletion*qc);
@@ -1069,6 +1070,7 @@ pgss_ExecutorEnd(QueryDesc *queryDesc)
10691070
*/
10701071
staticvoid
10711072
pgss_ProcessUtility(PlannedStmt*pstmt,constchar*queryString,
1073+
boolreadOnlyTree,
10721074
ProcessUtilityContextcontext,
10731075
ParamListInfoparams,QueryEnvironment*queryEnv,
10741076
DestReceiver*dest,QueryCompletion*qc)
@@ -1126,11 +1128,11 @@ pgss_ProcessUtility(PlannedStmt *pstmt, const char *queryString,
11261128
PG_TRY();
11271129
{
11281130
if (prev_ProcessUtility)
1129-
prev_ProcessUtility(pstmt,queryString,
1131+
prev_ProcessUtility(pstmt,queryString,readOnlyTree,
11301132
context,params,queryEnv,
11311133
dest,qc);
11321134
else
1133-
standard_ProcessUtility(pstmt,queryString,
1135+
standard_ProcessUtility(pstmt,queryString,readOnlyTree,
11341136
context,params,queryEnv,
11351137
dest,qc);
11361138
}
@@ -1176,11 +1178,11 @@ pgss_ProcessUtility(PlannedStmt *pstmt, const char *queryString,
11761178
else
11771179
{
11781180
if (prev_ProcessUtility)
1179-
prev_ProcessUtility(pstmt,queryString,
1181+
prev_ProcessUtility(pstmt,queryString,readOnlyTree,
11801182
context,params,queryEnv,
11811183
dest,qc);
11821184
else
1183-
standard_ProcessUtility(pstmt,queryString,
1185+
standard_ProcessUtility(pstmt,queryString,readOnlyTree,
11841186
context,params,queryEnv,
11851187
dest,qc);
11861188
}

‎contrib/sepgsql/hooks.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,7 @@ sepgsql_exec_check_perms(List *rangeTabls, bool abort)
313313
staticvoid
314314
sepgsql_utility_command(PlannedStmt*pstmt,
315315
constchar*queryString,
316+
boolreadOnlyTree,
316317
ProcessUtilityContextcontext,
317318
ParamListInfoparams,
318319
QueryEnvironment*queryEnv,
@@ -378,11 +379,11 @@ sepgsql_utility_command(PlannedStmt *pstmt,
378379
}
379380

380381
if (next_ProcessUtility_hook)
381-
(*next_ProcessUtility_hook) (pstmt,queryString,
382+
(*next_ProcessUtility_hook) (pstmt,queryString,readOnlyTree,
382383
context,params,queryEnv,
383384
dest,qc);
384385
else
385-
standard_ProcessUtility(pstmt,queryString,
386+
standard_ProcessUtility(pstmt,queryString,readOnlyTree,
386387
context,params,queryEnv,
387388
dest,qc);
388389
}

‎src/backend/commands/copyto.c

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -438,14 +438,8 @@ BeginCopyTo(ParseState *pstate,
438438
/*
439439
* Run parse analysis and rewrite. Note this also acquires sufficient
440440
* locks on the source table(s).
441-
*
442-
* Because the parser and planner tend to scribble on their input, we
443-
* make a preliminary copy of the source querytree. This prevents
444-
* problems in the case that the COPY is in a portal or plpgsql
445-
* function and is executed repeatedly. (See also the same hack in
446-
* DECLARE CURSOR and PREPARE.) XXX FIXME someday.
447441
*/
448-
rewritten=pg_analyze_and_rewrite(copyObject(raw_query),
442+
rewritten=pg_analyze_and_rewrite(raw_query,
449443
pstate->p_sourcetext,NULL,0,
450444
NULL);
451445

‎src/backend/commands/createas.c

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -299,14 +299,8 @@ ExecCreateTableAs(ParseState *pstate, CreateTableAsStmt *stmt,
299299
* rewriter. We do not do AcquireRewriteLocks: we assume the query
300300
* either came straight from the parser, or suitable locks were
301301
* acquired by plancache.c.
302-
*
303-
* Because the rewriter and planner tend to scribble on the input, we
304-
* make a preliminary copy of the source querytree. This prevents
305-
* problems in the case that CTAS is in a portal or plpgsql function
306-
* and is executed repeatedly. (See also the same hack in EXPLAIN and
307-
* PREPARE.)
308302
*/
309-
rewritten=QueryRewrite(copyObject(query));
303+
rewritten=QueryRewrite(query);
310304

311305
/* SELECT should never rewrite to more or less than one SELECT query */
312306
if (list_length(rewritten)!=1)

‎src/backend/commands/explain.c

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -256,14 +256,8 @@ ExplainQuery(ParseState *pstate, ExplainStmt *stmt,
256256
* rewriter. We do not do AcquireRewriteLocks: we assume the query either
257257
* came straight from the parser, or suitable locks were acquired by
258258
* plancache.c.
259-
*
260-
* Because the rewriter and planner tend to scribble on the input, we make
261-
* a preliminary copy of the source querytree. This prevents problems in
262-
* the case that the EXPLAIN is in a portal or plpgsql function and is
263-
* executed repeatedly. (See also the same hack in DECLARE CURSOR and
264-
* PREPARE.) XXX FIXME someday.
265259
*/
266-
rewritten=QueryRewrite(castNode(Query,copyObject(stmt->query)));
260+
rewritten=QueryRewrite(castNode(Query,stmt->query));
267261

268262
/* emit opening boilerplate */
269263
ExplainBeginOutput(es);
@@ -427,7 +421,8 @@ ExplainOneQuery(Query *query, int cursorOptions,
427421
* "into" is NULL unless we are explaining the contents of a CreateTableAsStmt.
428422
*
429423
* This is exported because it's called back from prepare.c in the
430-
* EXPLAIN EXECUTE case.
424+
* EXPLAIN EXECUTE case. In that case, we'll be dealing with a statement
425+
* that's in the plan cache, so we have to ensure we don't modify it.
431426
*/
432427
void
433428
ExplainOneUtility(Node*utilityStmt,IntoClause*into,ExplainState*es,
@@ -441,8 +436,7 @@ ExplainOneUtility(Node *utilityStmt, IntoClause *into, ExplainState *es,
441436
{
442437
/*
443438
* We have to rewrite the contained SELECT and then pass it back to
444-
* ExplainOneQuery. It's probably not really necessary to copy the
445-
* contained parsetree another time, but let's be safe.
439+
* ExplainOneQuery. Copy to be safe in the EXPLAIN EXECUTE case.
446440
*/
447441
CreateTableAsStmt*ctas= (CreateTableAsStmt*)utilityStmt;
448442
List*rewritten;

‎src/backend/commands/extension.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -786,6 +786,7 @@ execute_sql_string(const char *sql)
786786

787787
ProcessUtility(stmt,
788788
sql,
789+
false,
789790
PROCESS_UTILITY_QUERY,
790791
NULL,
791792
NULL,

‎src/backend/commands/foreigncmds.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1570,8 +1570,7 @@ ImportForeignSchema(ImportForeignSchemaStmt *stmt)
15701570
pstmt->stmt_len=rs->stmt_len;
15711571

15721572
/* Execute statement */
1573-
ProcessUtility(pstmt,
1574-
cmd,
1573+
ProcessUtility(pstmt,cmd, false,
15751574
PROCESS_UTILITY_SUBCOMMAND,NULL,NULL,
15761575
None_Receiver,NULL);
15771576

‎src/backend/commands/policy.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -747,12 +747,12 @@ CreatePolicy(CreatePolicyStmt *stmt)
747747
addNSItemToQuery(with_check_pstate,nsitem, false, true, true);
748748

749749
qual=transformWhereClause(qual_pstate,
750-
copyObject(stmt->qual),
750+
stmt->qual,
751751
EXPR_KIND_POLICY,
752752
"POLICY");
753753

754754
with_check_qual=transformWhereClause(with_check_pstate,
755-
copyObject(stmt->with_check),
755+
stmt->with_check,
756756
EXPR_KIND_POLICY,
757757
"POLICY");
758758

@@ -922,7 +922,7 @@ AlterPolicy(AlterPolicyStmt *stmt)
922922

923923
addNSItemToQuery(qual_pstate,nsitem, false, true, true);
924924

925-
qual=transformWhereClause(qual_pstate,copyObject(stmt->qual),
925+
qual=transformWhereClause(qual_pstate,stmt->qual,
926926
EXPR_KIND_POLICY,
927927
"POLICY");
928928

@@ -946,7 +946,7 @@ AlterPolicy(AlterPolicyStmt *stmt)
946946
addNSItemToQuery(with_check_pstate,nsitem, false, true, true);
947947

948948
with_check_qual=transformWhereClause(with_check_pstate,
949-
copyObject(stmt->with_check),
949+
stmt->with_check,
950950
EXPR_KIND_POLICY,
951951
"POLICY");
952952

‎src/backend/commands/portalcmds.c

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -76,14 +76,8 @@ PerformCursorOpen(ParseState *pstate, DeclareCursorStmt *cstmt, ParamListInfo pa
7676
* rewriter. We do not do AcquireRewriteLocks: we assume the query either
7777
* came straight from the parser, or suitable locks were acquired by
7878
* plancache.c.
79-
*
80-
* Because the rewriter and planner tend to scribble on the input, we make
81-
* a preliminary copy of the source querytree. This prevents problems in
82-
* the case that the DECLARE CURSOR is in a portal or plpgsql function and
83-
* is executed repeatedly. (See also the same hack in EXPLAIN and
84-
* PREPARE.) XXX FIXME someday.
8579
*/
86-
rewritten=QueryRewrite((Query*)copyObject(query));
80+
rewritten=QueryRewrite(query);
8781

8882
/* SELECT should never rewrite to more or less than one query */
8983
if (list_length(rewritten)!=1)

‎src/backend/commands/prepare.c

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,12 +78,9 @@ PrepareQuery(ParseState *pstate, PrepareStmt *stmt,
7878
/*
7979
* Need to wrap the contained statement in a RawStmt node to pass it to
8080
* parse analysis.
81-
*
82-
* Because parse analysis scribbles on the raw querytree, we must make a
83-
* copy to ensure we don't modify the passed-in tree. FIXME someday.
8481
*/
8582
rawstmt=makeNode(RawStmt);
86-
rawstmt->stmt=(Node*)copyObject(stmt->query);
83+
rawstmt->stmt=stmt->query;
8784
rawstmt->stmt_location=stmt_location;
8885
rawstmt->stmt_len=stmt_len;
8986

‎src/backend/commands/schemacmds.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,7 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString,
191191
/* do this step */
192192
ProcessUtility(wrapper,
193193
queryString,
194+
false,
194195
PROCESS_UTILITY_SUBCOMMAND,
195196
NULL,
196197
NULL,

‎src/backend/commands/tablecmds.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4408,8 +4408,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
44084408
* Copy the original subcommand for each table. This avoids conflicts
44094409
* when different child tables need to make different parse
44104410
* transformations (for example, the same column may have different column
4411-
* numbers in different children). It also ensures that we don't corrupt
4412-
* the original parse tree, in case it is saved in plancache.
4411+
* numbers in different children).
44134412
*/
44144413
cmd = copyObject(cmd);
44154414

‎src/backend/commands/view.c

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -417,12 +417,9 @@ DefineView(ViewStmt *stmt, const char *queryString,
417417
/*
418418
* Run parse analysis to convert the raw parse tree to a Query. Note this
419419
* also acquires sufficient locks on the source table(s).
420-
*
421-
* Since parse analysis scribbles on its input, copy the raw parse tree;
422-
* this ensures we don't corrupt a prepared statement, for example.
423420
*/
424421
rawstmt=makeNode(RawStmt);
425-
rawstmt->stmt=(Node*)copyObject(stmt->query);
422+
rawstmt->stmt=stmt->query;
426423
rawstmt->stmt_location=stmt_location;
427424
rawstmt->stmt_len=stmt_len;
428425

‎src/backend/executor/functions.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -886,6 +886,7 @@ postquel_getnext(execution_state *es, SQLFunctionCachePtr fcache)
886886
{
887887
ProcessUtility(es->qd->plannedstmt,
888888
fcache->src,
889+
false,
889890
PROCESS_UTILITY_QUERY,
890891
es->qd->params,
891892
es->qd->queryEnv,

‎src/backend/executor/spi.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2545,6 +2545,7 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,
25452545
InitializeQueryCompletion(&qc);
25462546
ProcessUtility(stmt,
25472547
plansource->query_string,
2548+
true,/* protect plancache's node tree */
25482549
context,
25492550
paramLI,
25502551
_SPI_current->queryEnv,

‎src/backend/parser/parse_utilcmd.c

Lines changed: 2 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,6 @@
1111
* Hence these functions are now called at the start of execution of their
1212
* respective utility commands.
1313
*
14-
* NOTE: in general we must avoid scribbling on the passed-in raw parse
15-
* tree, since it might be in a plan cache. The simplest solution is
16-
* a quick copyObject() call before manipulating the query tree.
17-
*
1814
*
1915
* Portions Copyright (c) 1996-2021, PostgreSQL Global Development Group
2016
* Portions Copyright (c) 1994, Regents of the University of California
@@ -177,12 +173,6 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
177173
Oidexisting_relid;
178174
ParseCallbackStatepcbstate;
179175

180-
/*
181-
* We must not scribble on the passed-in CreateStmt, so copy it. (This is
182-
* overkill, but easy.)
183-
*/
184-
stmt=copyObject(stmt);
185-
186176
/* Set up pstate */
187177
pstate=make_parsestate(NULL);
188178
pstate->p_sourcetext=queryString;
@@ -2824,12 +2814,6 @@ transformIndexStmt(Oid relid, IndexStmt *stmt, const char *queryString)
28242814
if (stmt->transformed)
28252815
returnstmt;
28262816

2827-
/*
2828-
* We must not scribble on the passed-in IndexStmt, so copy it. (This is
2829-
* overkill, but easy.)
2830-
*/
2831-
stmt=copyObject(stmt);
2832-
28332817
/* Set up pstate */
28342818
pstate=make_parsestate(NULL);
28352819
pstate->p_sourcetext=queryString;
@@ -2925,12 +2909,6 @@ transformStatsStmt(Oid relid, CreateStatsStmt *stmt, const char *queryString)
29252909
if (stmt->transformed)
29262910
returnstmt;
29272911

2928-
/*
2929-
* We must not scribble on the passed-in CreateStatsStmt, so copy it.
2930-
* (This is overkill, but easy.)
2931-
*/
2932-
stmt=copyObject(stmt);
2933-
29342912
/* Set up pstate */
29352913
pstate=make_parsestate(NULL);
29362914
pstate->p_sourcetext=queryString;
@@ -2993,9 +2971,6 @@ transformStatsStmt(Oid relid, CreateStatsStmt *stmt, const char *queryString)
29932971
*
29942972
* actions and whereClause are output parameters that receive the
29952973
* transformed results.
2996-
*
2997-
* Note that we must not scribble on the passed-in RuleStmt, so we do
2998-
* copyObject() on the actions and WHERE clause.
29992974
*/
30002975
void
30012976
transformRuleStmt(RuleStmt*stmt,constchar*queryString,
@@ -3070,7 +3045,7 @@ transformRuleStmt(RuleStmt *stmt, const char *queryString,
30703045

30713046
/* take care of the where clause */
30723047
*whereClause=transformWhereClause(pstate,
3073-
(Node*)copyObject(stmt->whereClause),
3048+
stmt->whereClause,
30743049
EXPR_KIND_WHERE,
30753050
"WHERE");
30763051
/* we have to fix its collations too */
@@ -3142,8 +3117,7 @@ transformRuleStmt(RuleStmt *stmt, const char *queryString,
31423117
addNSItemToQuery(sub_pstate,newnsitem, false, true, false);
31433118

31443119
/* Transform the rule action statement */
3145-
top_subqry=transformStmt(sub_pstate,
3146-
(Node*)copyObject(action));
3120+
top_subqry=transformStmt(sub_pstate,action);
31473121

31483122
/*
31493123
* We cannot support utility-statement actions (eg NOTIFY) with
@@ -3325,12 +3299,6 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
33253299
AlterTableCmd*newcmd;
33263300
ParseNamespaceItem*nsitem;
33273301

3328-
/*
3329-
* We must not scribble on the passed-in AlterTableStmt, so copy it. (This
3330-
* is overkill, but easy.)
3331-
*/
3332-
stmt=copyObject(stmt);
3333-
33343302
/* Caller is responsible for locking the relation */
33353303
rel=relation_open(relid,NoLock);
33363304
tupdesc=RelationGetDescr(rel);

‎src/backend/tcop/pquery.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1146,6 +1146,7 @@ PortalRunUtility(Portal portal, PlannedStmt *pstmt,
11461146

11471147
ProcessUtility(pstmt,
11481148
portal->sourceText,
1149+
(portal->cplan!=NULL),/* protect tree if in plancache */
11491150
isTopLevel ?PROCESS_UTILITY_TOPLEVEL :PROCESS_UTILITY_QUERY,
11501151
portal->portalParams,
11511152
portal->queryEnv,

0 commit comments

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