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

Commita874fe7

Browse files
committed
Refactor the executor's API to support data-modifying CTEs better.
The originally committed patch for modifying CTEs didn't interact wellwith EXPLAIN, as noted by myself, and also had corner-case problems withtriggers, as noted by Dean Rasheed. Those problems show it is really notpractical for ExecutorEnd to call any user-defined code; so split thecleanup duties out into a new function ExecutorFinish, which must be calledbetween the last ExecutorRun call and ExecutorEnd. Some Asserts have beenadded to these functions to help verify correct usage.It is no longer necessary for callers of the executor to callAfterTriggerBeginQuery/AfterTriggerEndQuery for themselves, as this is nowdone by ExecutorStart/ExecutorFinish respectively. If you really need tosuppress that and do it for yourself, pass EXEC_FLAG_SKIP_TRIGGERS toExecutorStart.Also, refactor portal commit processing to allow for the possibility thatPortalDrop will invoke user-defined code. I think this is not actuallynecessary just yet, since the portal-execution-strategy logic forces anynon-pure-SELECT query to be run to completion before we will considercommitting. But it seems like good future-proofing.
1 parent67a5e72 commita874fe7

File tree

21 files changed

+350
-222
lines changed

21 files changed

+350
-222
lines changed

‎contrib/auto_explain/auto_explain.c

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ static intnesting_level = 0;
4040
/* Saved hook values in case of unload */
4141
staticExecutorStart_hook_typeprev_ExecutorStart=NULL;
4242
staticExecutorRun_hook_typeprev_ExecutorRun=NULL;
43+
staticExecutorFinish_hook_typeprev_ExecutorFinish=NULL;
4344
staticExecutorEnd_hook_typeprev_ExecutorEnd=NULL;
4445

4546
#defineauto_explain_enabled() \
@@ -53,6 +54,7 @@ static void explain_ExecutorStart(QueryDesc *queryDesc, int eflags);
5354
staticvoidexplain_ExecutorRun(QueryDesc*queryDesc,
5455
ScanDirectiondirection,
5556
longcount);
57+
staticvoidexplain_ExecutorFinish(QueryDesc*queryDesc);
5658
staticvoidexplain_ExecutorEnd(QueryDesc*queryDesc);
5759

5860

@@ -132,6 +134,8 @@ _PG_init(void)
132134
ExecutorStart_hook=explain_ExecutorStart;
133135
prev_ExecutorRun=ExecutorRun_hook;
134136
ExecutorRun_hook=explain_ExecutorRun;
137+
prev_ExecutorFinish=ExecutorFinish_hook;
138+
ExecutorFinish_hook=explain_ExecutorFinish;
135139
prev_ExecutorEnd=ExecutorEnd_hook;
136140
ExecutorEnd_hook=explain_ExecutorEnd;
137141
}
@@ -145,6 +149,7 @@ _PG_fini(void)
145149
/* Uninstall hooks. */
146150
ExecutorStart_hook=prev_ExecutorStart;
147151
ExecutorRun_hook=prev_ExecutorRun;
152+
ExecutorFinish_hook=prev_ExecutorFinish;
148153
ExecutorEnd_hook=prev_ExecutorEnd;
149154
}
150155

@@ -211,6 +216,29 @@ explain_ExecutorRun(QueryDesc *queryDesc, ScanDirection direction, long count)
211216
PG_END_TRY();
212217
}
213218

219+
/*
220+
* ExecutorFinish hook: all we need do is track nesting depth
221+
*/
222+
staticvoid
223+
explain_ExecutorFinish(QueryDesc*queryDesc)
224+
{
225+
nesting_level++;
226+
PG_TRY();
227+
{
228+
if (prev_ExecutorFinish)
229+
prev_ExecutorFinish(queryDesc);
230+
else
231+
standard_ExecutorFinish(queryDesc);
232+
nesting_level--;
233+
}
234+
PG_CATCH();
235+
{
236+
nesting_level--;
237+
PG_RE_THROW();
238+
}
239+
PG_END_TRY();
240+
}
241+
214242
/*
215243
* ExecutorEnd hook: log results if needed
216244
*/

‎contrib/pg_stat_statements/pg_stat_statements.c

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ static intnested_level = 0;
122122
staticshmem_startup_hook_typeprev_shmem_startup_hook=NULL;
123123
staticExecutorStart_hook_typeprev_ExecutorStart=NULL;
124124
staticExecutorRun_hook_typeprev_ExecutorRun=NULL;
125+
staticExecutorFinish_hook_typeprev_ExecutorFinish=NULL;
125126
staticExecutorEnd_hook_typeprev_ExecutorEnd=NULL;
126127
staticProcessUtility_hook_typeprev_ProcessUtility=NULL;
127128

@@ -173,6 +174,7 @@ static void pgss_ExecutorStart(QueryDesc *queryDesc, int eflags);
173174
staticvoidpgss_ExecutorRun(QueryDesc*queryDesc,
174175
ScanDirectiondirection,
175176
longcount);
177+
staticvoidpgss_ExecutorFinish(QueryDesc*queryDesc);
176178
staticvoidpgss_ExecutorEnd(QueryDesc*queryDesc);
177179
staticvoidpgss_ProcessUtility(Node*parsetree,
178180
constchar*queryString,ParamListInfoparams,boolisTopLevel,
@@ -269,6 +271,8 @@ _PG_init(void)
269271
ExecutorStart_hook=pgss_ExecutorStart;
270272
prev_ExecutorRun=ExecutorRun_hook;
271273
ExecutorRun_hook=pgss_ExecutorRun;
274+
prev_ExecutorFinish=ExecutorFinish_hook;
275+
ExecutorFinish_hook=pgss_ExecutorFinish;
272276
prev_ExecutorEnd=ExecutorEnd_hook;
273277
ExecutorEnd_hook=pgss_ExecutorEnd;
274278
prev_ProcessUtility=ProcessUtility_hook;
@@ -285,6 +289,7 @@ _PG_fini(void)
285289
shmem_startup_hook=prev_shmem_startup_hook;
286290
ExecutorStart_hook=prev_ExecutorStart;
287291
ExecutorRun_hook=prev_ExecutorRun;
292+
ExecutorFinish_hook=prev_ExecutorFinish;
288293
ExecutorEnd_hook=prev_ExecutorEnd;
289294
ProcessUtility_hook=prev_ProcessUtility;
290295
}
@@ -549,6 +554,29 @@ pgss_ExecutorRun(QueryDesc *queryDesc, ScanDirection direction, long count)
549554
PG_END_TRY();
550555
}
551556

557+
/*
558+
* ExecutorFinish hook: all we need do is track nesting depth
559+
*/
560+
staticvoid
561+
pgss_ExecutorFinish(QueryDesc*queryDesc)
562+
{
563+
nested_level++;
564+
PG_TRY();
565+
{
566+
if (prev_ExecutorFinish)
567+
prev_ExecutorFinish(queryDesc);
568+
else
569+
standard_ExecutorFinish(queryDesc);
570+
nested_level--;
571+
}
572+
PG_CATCH();
573+
{
574+
nested_level--;
575+
PG_RE_THROW();
576+
}
577+
PG_END_TRY();
578+
}
579+
552580
/*
553581
* ExecutorEnd hook: store results if needed
554582
*/

‎src/backend/access/transam/xact.c

Lines changed: 32 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1753,12 +1753,10 @@ CommitTransaction(void)
17531753
Assert(s->parent==NULL);
17541754

17551755
/*
1756-
* Do pre-commit processing (most of this stuff requires database access,
1757-
* and in fact could still cause an error...)
1758-
*
1759-
* It is possible for CommitHoldablePortals to invoke functions that queue
1760-
* deferred triggers, and it's also possible that triggers create holdable
1761-
* cursors. So we have to loop until there's nothing left to do.
1756+
* Do pre-commit processing that involves calling user-defined code, such
1757+
* as triggers. Since closing cursors could queue trigger actions,
1758+
* triggers could open cursors, etc, we have to keep looping until there's
1759+
* nothing left to do.
17621760
*/
17631761
for (;;)
17641762
{
@@ -1768,19 +1766,23 @@ CommitTransaction(void)
17681766
AfterTriggerFireDeferred();
17691767

17701768
/*
1771-
*Convert anyopen holdablecursors into static portals. If there
1772-
* weren't any, we are done ... otherwise loop back to check if they
1773-
* queued deferred triggers. Lather, rinse, repeat.
1769+
*Closeopenportals (convertingholdableones into static portals).
1770+
*If thereweren't any, we are done ... otherwise loop back to check
1771+
*if theyqueued deferred triggers. Lather, rinse, repeat.
17741772
*/
1775-
if (!CommitHoldablePortals())
1773+
if (!PreCommit_Portals(false))
17761774
break;
17771775
}
17781776

1779-
/* Now we can shut down the deferred-trigger manager */
1780-
AfterTriggerEndXact(true);
1777+
/*
1778+
* The remaining actions cannot call any user-defined code, so it's
1779+
* safe to start shutting down within-transaction services. But note
1780+
* that most of this stuff could still throw an error, which would
1781+
* switch us into the transaction-abort path.
1782+
*/
17811783

1782-
/*Close any open regular cursors */
1783-
AtCommit_Portals();
1784+
/*Shut down the deferred-trigger manager */
1785+
AfterTriggerEndXact(true);
17841786

17851787
/*
17861788
* Let ON COMMIT management do its thing (must happen after closing
@@ -1954,12 +1956,10 @@ PrepareTransaction(void)
19541956
Assert(s->parent==NULL);
19551957

19561958
/*
1957-
* Do pre-commit processing (most of this stuff requires database access,
1958-
* and in fact could still cause an error...)
1959-
*
1960-
* It is possible for PrepareHoldablePortals to invoke functions that
1961-
* queue deferred triggers, and it's also possible that triggers create
1962-
* holdable cursors. So we have to loop until there's nothing left to do.
1959+
* Do pre-commit processing that involves calling user-defined code, such
1960+
* as triggers. Since closing cursors could queue trigger actions,
1961+
* triggers could open cursors, etc, we have to keep looping until there's
1962+
* nothing left to do.
19631963
*/
19641964
for (;;)
19651965
{
@@ -1969,19 +1969,23 @@ PrepareTransaction(void)
19691969
AfterTriggerFireDeferred();
19701970

19711971
/*
1972-
*Convert anyopen holdablecursors into static portals. If there
1973-
* weren't any, we are done ... otherwise loop back to check if they
1974-
* queued deferred triggers. Lather, rinse, repeat.
1972+
*Closeopenportals (convertingholdableones into static portals).
1973+
*If thereweren't any, we are done ... otherwise loop back to check
1974+
*if theyqueued deferred triggers. Lather, rinse, repeat.
19751975
*/
1976-
if (!PrepareHoldablePortals())
1976+
if (!PreCommit_Portals(true))
19771977
break;
19781978
}
19791979

1980-
/* Now we can shut down the deferred-trigger manager */
1981-
AfterTriggerEndXact(true);
1980+
/*
1981+
* The remaining actions cannot call any user-defined code, so it's
1982+
* safe to start shutting down within-transaction services. But note
1983+
* that most of this stuff could still throw an error, which would
1984+
* switch us into the transaction-abort path.
1985+
*/
19821986

1983-
/*Close any open regular cursors */
1984-
AtCommit_Portals();
1987+
/*Shut down the deferred-trigger manager */
1988+
AfterTriggerEndXact(true);
19851989

19861990
/*
19871991
* Let ON COMMIT management do its thing (must happen after closing

‎src/backend/commands/copy.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1463,6 +1463,7 @@ EndCopyTo(CopyState cstate)
14631463
if (cstate->queryDesc!=NULL)
14641464
{
14651465
/* Close down the query and free resources. */
1466+
ExecutorFinish(cstate->queryDesc);
14661467
ExecutorEnd(cstate->queryDesc);
14671468
FreeQueryDesc(cstate->queryDesc);
14681469
PopActiveSnapshot();

‎src/backend/commands/discard.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,11 @@ DiscardAll(bool isTopLevel)
6161
*/
6262
PreventTransactionChain(isTopLevel,"DISCARD ALL");
6363

64+
/* Closing portals might run user-defined code, so do that first. */
65+
PortalHashTableDeleteAll();
6466
SetPGVariable("session_authorization",NIL, false);
6567
ResetAllOptions();
6668
DropAllPreparedStatements();
67-
PortalHashTableDeleteAll();
6869
Async_UnlistenAll();
6970
LockReleaseAll(USER_LOCKMETHOD, true);
7071
ResetPlanCache();

‎src/backend/commands/explain.c

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,8 @@ ExplainOnePlan(PlannedStmt *plannedstmt, ExplainState *es,
360360
if (es->buffers)
361361
instrument_option |=INSTRUMENT_BUFFERS;
362362

363+
INSTR_TIME_SET_CURRENT(starttime);
364+
363365
/*
364366
* Use a snapshot with an updated command ID to ensure this query sees
365367
* results of any previously executed queries.
@@ -371,12 +373,6 @@ ExplainOnePlan(PlannedStmt *plannedstmt, ExplainState *es,
371373
GetActiveSnapshot(),InvalidSnapshot,
372374
None_Receiver,params,instrument_option);
373375

374-
INSTR_TIME_SET_CURRENT(starttime);
375-
376-
/* If analyzing, we need to cope with queued triggers */
377-
if (es->analyze)
378-
AfterTriggerBeginQuery();
379-
380376
/* Select execution options */
381377
if (es->analyze)
382378
eflags=0;/* default run-to-completion flags */
@@ -392,7 +388,10 @@ ExplainOnePlan(PlannedStmt *plannedstmt, ExplainState *es,
392388
/* run the plan */
393389
ExecutorRun(queryDesc,ForwardScanDirection,0L);
394390

395-
/* We can't clean up 'till we're done printing the stats... */
391+
/* run cleanup too */
392+
ExecutorFinish(queryDesc);
393+
394+
/* We can't run ExecutorEnd 'till we're done printing the stats... */
396395
totaltime+=elapsed_time(&starttime);
397396
}
398397

@@ -401,18 +400,6 @@ ExplainOnePlan(PlannedStmt *plannedstmt, ExplainState *es,
401400
/* Create textual dump of plan tree */
402401
ExplainPrintPlan(es,queryDesc);
403402

404-
/*
405-
* If we ran the command, run any AFTER triggers it queued. (Note this
406-
* will not include DEFERRED triggers; since those don't run until end of
407-
* transaction, we can't measure them.) Include into total runtime.
408-
*/
409-
if (es->analyze)
410-
{
411-
INSTR_TIME_SET_CURRENT(starttime);
412-
AfterTriggerEndQuery(queryDesc->estate);
413-
totaltime+=elapsed_time(&starttime);
414-
}
415-
416403
/* Print info about runtime of triggers */
417404
if (es->analyze)
418405
{

‎src/backend/commands/extension.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -726,10 +726,9 @@ execute_sql_string(const char *sql, const char *filename)
726726
GetActiveSnapshot(),NULL,
727727
dest,NULL,0);
728728

729-
AfterTriggerBeginQuery();
730729
ExecutorStart(qdesc,0);
731730
ExecutorRun(qdesc,ForwardScanDirection,0);
732-
AfterTriggerEndQuery(qdesc->estate);
731+
ExecutorFinish(qdesc);
733732
ExecutorEnd(qdesc);
734733

735734
FreeQueryDesc(qdesc);

‎src/backend/commands/portalcmds.c

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,14 @@ PortalCleanup(Portal portal)
254254
queryDesc=PortalGetQueryDesc(portal);
255255
if (queryDesc)
256256
{
257+
/*
258+
* Reset the queryDesc before anything else. This prevents us
259+
* from trying to shut down the executor twice, in case of an
260+
* error below. The transaction abort mechanisms will take care
261+
* of resource cleanup in such a case.
262+
*/
257263
portal->queryDesc=NULL;
264+
258265
if (portal->status!=PORTAL_FAILED)
259266
{
260267
ResourceOwnersaveResourceOwner;
@@ -264,7 +271,7 @@ PortalCleanup(Portal portal)
264271
PG_TRY();
265272
{
266273
CurrentResourceOwner=portal->resowner;
267-
/* we do not need AfterTriggerEndQuery() here */
274+
ExecutorFinish(queryDesc);
268275
ExecutorEnd(queryDesc);
269276
FreeQueryDesc(queryDesc);
270277
}
@@ -371,7 +378,7 @@ PersistHoldablePortal(Portal portal)
371378
* Now shut down the inner executor.
372379
*/
373380
portal->queryDesc=NULL;/* prevent double shutdown */
374-
/* we do not need AfterTriggerEndQuery() here */
381+
ExecutorFinish(queryDesc);
375382
ExecutorEnd(queryDesc);
376383
FreeQueryDesc(queryDesc);
377384

‎src/backend/commands/trigger.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3676,9 +3676,9 @@ AfterTriggerBeginQuery(void)
36763676
*we invoke all AFTER IMMEDIATE trigger events queued by the query, and
36773677
*transfer deferred trigger events to the global deferred-trigger list.
36783678
*
3679-
*Note that thisshould be called just BEFORE closing down the executor
3679+
*Note that thismust be called BEFORE closing down the executor
36803680
*with ExecutorEnd, because we make use of the EState's info about
3681-
*target relations.
3681+
*target relations. Normally it is called from ExecutorFinish.
36823682
* ----------
36833683
*/
36843684
void

‎src/backend/executor/README

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,12 +95,17 @@ This is a sketch of control flow for full query processing:
9595
CreateExprContext
9696
creates per-tuple context
9797
ExecInitExpr
98+
AfterTriggerBeginQuery
9899

99100
ExecutorRun
100101
ExecProcNode --- recursively called in per-query context
101102
ExecEvalExpr --- called in per-tuple context
102103
ResetExprContext --- to free memory
103104

105+
ExecutorFinish
106+
ExecPostprocessPlan --- run any unfinished ModifyTable nodes
107+
AfterTriggerEndQuery
108+
104109
ExecutorEnd
105110
ExecEndNode --- recursively releases resources
106111
FreeExecutorState

0 commit comments

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