summaryrefslogtreecommitdiff
path: root/src/backend/commands
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2011-02-27 13:43:29 -0500
committerTom Lane <tgl@sss.pgh.pa.us>2011-02-27 13:44:12 -0500
commita874fe7b4c890d1fe3455215a83ca777867beadd (patch)
tree04b7870100a76bb79470abaf0f971550043590d7 /src/backend/commands
parent67a5e727c8655496013b007d2fb6137fcc244b18 (diff)
downloadpostgresql-a874fe7b4c890d1fe3455215a83ca777867beadd.tar.gz
Refactor the executor's API to support data-modifying CTEs better.
The originally committed patch for modifying CTEs didn't interact well with EXPLAIN, as noted by myself, and also had corner-case problems with triggers, as noted by Dean Rasheed. Those problems show it is really not practical for ExecutorEnd to call any user-defined code; so split the cleanup duties out into a new function ExecutorFinish, which must be called between the last ExecutorRun call and ExecutorEnd. Some Asserts have been added to these functions to help verify correct usage. It is no longer necessary for callers of the executor to call AfterTriggerBeginQuery/AfterTriggerEndQuery for themselves, as this is now done by ExecutorStart/ExecutorFinish respectively. If you really need to suppress that and do it for yourself, pass EXEC_FLAG_SKIP_TRIGGERS to ExecutorStart. Also, refactor portal commit processing to allow for the possibility that PortalDrop will invoke user-defined code. I think this is not actually necessary just yet, since the portal-execution-strategy logic forces any non-pure-SELECT query to be run to completion before we will consider committing. But it seems like good future-proofing.
Diffstat (limited to 'src/backend/commands')
-rw-r--r--src/backend/commands/copy.c1
-rw-r--r--src/backend/commands/discard.c3
-rw-r--r--src/backend/commands/explain.c25
-rw-r--r--src/backend/commands/extension.c3
-rw-r--r--src/backend/commands/portalcmds.c11
-rw-r--r--src/backend/commands/trigger.c4
6 files changed, 21 insertions, 26 deletions
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 3c504e96be..4b32c5dc5c 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -1463,6 +1463,7 @@ EndCopyTo(CopyState cstate)
if (cstate->queryDesc != NULL)
{
/* Close down the query and free resources. */
+ ExecutorFinish(cstate->queryDesc);
ExecutorEnd(cstate->queryDesc);
FreeQueryDesc(cstate->queryDesc);
PopActiveSnapshot();
diff --git a/src/backend/commands/discard.c b/src/backend/commands/discard.c
index cc8ad9ae07..85a2ef8582 100644
--- a/src/backend/commands/discard.c
+++ b/src/backend/commands/discard.c
@@ -61,10 +61,11 @@ DiscardAll(bool isTopLevel)
*/
PreventTransactionChain(isTopLevel, "DISCARD ALL");
+ /* Closing portals might run user-defined code, so do that first. */
+ PortalHashTableDeleteAll();
SetPGVariable("session_authorization", NIL, false);
ResetAllOptions();
DropAllPreparedStatements();
- PortalHashTableDeleteAll();
Async_UnlistenAll();
LockReleaseAll(USER_LOCKMETHOD, true);
ResetPlanCache();
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index ccae1f9d84..23819a0fdb 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -360,6 +360,8 @@ ExplainOnePlan(PlannedStmt *plannedstmt, ExplainState *es,
if (es->buffers)
instrument_option |= INSTRUMENT_BUFFERS;
+ INSTR_TIME_SET_CURRENT(starttime);
+
/*
* Use a snapshot with an updated command ID to ensure this query sees
* results of any previously executed queries.
@@ -371,12 +373,6 @@ ExplainOnePlan(PlannedStmt *plannedstmt, ExplainState *es,
GetActiveSnapshot(), InvalidSnapshot,
None_Receiver, params, instrument_option);
- INSTR_TIME_SET_CURRENT(starttime);
-
- /* If analyzing, we need to cope with queued triggers */
- if (es->analyze)
- AfterTriggerBeginQuery();
-
/* Select execution options */
if (es->analyze)
eflags = 0; /* default run-to-completion flags */
@@ -392,7 +388,10 @@ ExplainOnePlan(PlannedStmt *plannedstmt, ExplainState *es,
/* run the plan */
ExecutorRun(queryDesc, ForwardScanDirection, 0L);
- /* We can't clean up 'till we're done printing the stats... */
+ /* run cleanup too */
+ ExecutorFinish(queryDesc);
+
+ /* We can't run ExecutorEnd 'till we're done printing the stats... */
totaltime += elapsed_time(&starttime);
}
@@ -401,18 +400,6 @@ ExplainOnePlan(PlannedStmt *plannedstmt, ExplainState *es,
/* Create textual dump of plan tree */
ExplainPrintPlan(es, queryDesc);
- /*
- * If we ran the command, run any AFTER triggers it queued. (Note this
- * will not include DEFERRED triggers; since those don't run until end of
- * transaction, we can't measure them.) Include into total runtime.
- */
- if (es->analyze)
- {
- INSTR_TIME_SET_CURRENT(starttime);
- AfterTriggerEndQuery(queryDesc->estate);
- totaltime += elapsed_time(&starttime);
- }
-
/* Print info about runtime of triggers */
if (es->analyze)
{
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 4bb79d4921..8c812c21e2 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -726,10 +726,9 @@ execute_sql_string(const char *sql, const char *filename)
GetActiveSnapshot(), NULL,
dest, NULL, 0);
- AfterTriggerBeginQuery();
ExecutorStart(qdesc, 0);
ExecutorRun(qdesc, ForwardScanDirection, 0);
- AfterTriggerEndQuery(qdesc->estate);
+ ExecutorFinish(qdesc);
ExecutorEnd(qdesc);
FreeQueryDesc(qdesc);
diff --git a/src/backend/commands/portalcmds.c b/src/backend/commands/portalcmds.c
index dda592c03a..60aca3ce8e 100644
--- a/src/backend/commands/portalcmds.c
+++ b/src/backend/commands/portalcmds.c
@@ -254,7 +254,14 @@ PortalCleanup(Portal portal)
queryDesc = PortalGetQueryDesc(portal);
if (queryDesc)
{
+ /*
+ * Reset the queryDesc before anything else. This prevents us
+ * from trying to shut down the executor twice, in case of an
+ * error below. The transaction abort mechanisms will take care
+ * of resource cleanup in such a case.
+ */
portal->queryDesc = NULL;
+
if (portal->status != PORTAL_FAILED)
{
ResourceOwner saveResourceOwner;
@@ -264,7 +271,7 @@ PortalCleanup(Portal portal)
PG_TRY();
{
CurrentResourceOwner = portal->resowner;
- /* we do not need AfterTriggerEndQuery() here */
+ ExecutorFinish(queryDesc);
ExecutorEnd(queryDesc);
FreeQueryDesc(queryDesc);
}
@@ -371,7 +378,7 @@ PersistHoldablePortal(Portal portal)
* Now shut down the inner executor.
*/
portal->queryDesc = NULL; /* prevent double shutdown */
- /* we do not need AfterTriggerEndQuery() here */
+ ExecutorFinish(queryDesc);
ExecutorEnd(queryDesc);
FreeQueryDesc(queryDesc);
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index dc6ee9c266..0af8a11b0a 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -3676,9 +3676,9 @@ AfterTriggerBeginQuery(void)
* we invoke all AFTER IMMEDIATE trigger events queued by the query, and
* transfer deferred trigger events to the global deferred-trigger list.
*
- * Note that this should be called just BEFORE closing down the executor
+ * Note that this must be called BEFORE closing down the executor
* with ExecutorEnd, because we make use of the EState's info about
- * target relations.
+ * target relations. Normally it is called from ExecutorFinish.
* ----------
*/
void