aboutsummaryrefslogtreecommitdiff
path: root/src/backend/executor
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2017-09-16 13:20:32 -0400
committerTom Lane <tgl@sss.pgh.pa.us>2017-09-16 13:20:37 -0400
commit54d4d0ff6cd40638d026c01e46deb102e7951ba6 (patch)
tree2ec106b514e81cb463c388a4f644b128b7bfa375 /src/backend/executor
parentd2bbd6104096b1559c1683d39e2c4e4cfbcada1c (diff)
downloadpostgresql-54d4d0ff6cd40638d026c01e46deb102e7951ba6.tar.gz
postgresql-54d4d0ff6cd40638d026c01e46deb102e7951ba6.zip
Fix SQL-spec incompatibilities in new transition table feature.
The standard says that all changes of the same kind (insert, update, or delete) caused in one table by a single SQL statement should be reported in a single transition table; and by that, they mean to include foreign key enforcement actions cascading from the statement's direct effects. It's also reasonable to conclude that if the standard had wCTEs, they would say that effects of wCTEs applying to the same table as each other or the outer statement should be merged into one transition table. We weren't doing it like that. Hence, arrange to merge tuples from multiple update actions into a single transition table as much as we can. There is a problem, which is that if the firing of FK enforcement triggers and after-row triggers with transition tables is interspersed, we might need to report more tuples after some triggers have already seen the transition table. It seems like a bad idea for the transition table to be mutable between trigger calls. There's no good way around this without a major redesign of the FK logic, so for now, resolve it by opening a new transition table each time this happens. Also, ensure that AFTER STATEMENT triggers fire just once per statement, or once per transition table when we're forced to make more than one. Previous versions of Postgres have allowed each FK enforcement query to cause an additional firing of the AFTER STATEMENT triggers for the referencing table, but that's certainly not per spec. (We're still doing multiple firings of BEFORE STATEMENT triggers, though; is that something worth changing?) Also, forbid using transition tables with column-specific UPDATE triggers. The spec requires such transition tables to show only the tuples for which the UPDATE trigger would have fired, which means maintaining multiple transition tables or else somehow filtering the contents at readout. Maybe someday we'll bother to support that option, but it looks like a lot of trouble for a marginal feature. The transition tables are now managed by the AfterTriggers data structures, rather than being directly the responsibility of ModifyTable nodes. This removes a subtransaction-lifespan memory leak introduced by my previous band-aid patch 3c4359521. In passing, refactor the AfterTriggers data structures to reduce the management overhead for them, by using arrays of structs rather than several parallel arrays for per-query-level and per-subtransaction state. I failed to resist the temptation to do some copy-editing on the SGML docs about triggers, above and beyond merely documenting the effects of this patch. Back-patch to v10, because we don't want the semantics of transition tables to change post-release. Patch by me, with help and review from Thomas Munro. Discussion: https://postgr.es/m/20170909064853.25630.12825@wrigleys.postgresql.org
Diffstat (limited to 'src/backend/executor')
-rw-r--r--src/backend/executor/README2
-rw-r--r--src/backend/executor/execMain.c11
-rw-r--r--src/backend/executor/nodeModifyTable.c57
3 files changed, 44 insertions, 26 deletions
diff --git a/src/backend/executor/README b/src/backend/executor/README
index a0045067fb8..b3e74aa1a54 100644
--- a/src/backend/executor/README
+++ b/src/backend/executor/README
@@ -241,11 +241,11 @@ This is a sketch of control flow for full query processing:
CreateExecutorState
creates per-query context
switch to per-query context to run ExecInitNode
+ AfterTriggerBeginQuery
ExecInitNode --- recursively scans plan tree
CreateExprContext
creates per-tuple context
ExecInitExpr
- AfterTriggerBeginQuery
ExecutorRun
ExecProcNode --- recursively called in per-query context
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 9dcc358ec27..396b7a1e83f 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -252,17 +252,17 @@ standard_ExecutorStart(QueryDesc *queryDesc, int eflags)
estate->es_instrument = queryDesc->instrument_options;
/*
- * Initialize the plan state tree
- */
- InitPlan(queryDesc, eflags);
-
- /*
* Set up an AFTER-trigger statement context, unless told not to, or
* unless it's EXPLAIN-only mode (when ExecutorFinish won't be called).
*/
if (!(eflags & (EXEC_FLAG_SKIP_TRIGGERS | EXEC_FLAG_EXPLAIN_ONLY)))
AfterTriggerBeginQuery();
+ /*
+ * Initialize the plan state tree
+ */
+ InitPlan(queryDesc, eflags);
+
MemoryContextSwitchTo(oldcontext);
}
@@ -1174,6 +1174,7 @@ CheckValidResultRel(ResultRelInfo *resultRelInfo, CmdType operation)
switch (operation)
{
case CMD_INSERT:
+
/*
* If foreign partition to do tuple-routing for, skip the
* check; it's disallowed elsewhere.
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index d78e868154e..7b5214c9996 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -342,6 +342,9 @@ ExecInsert(ModifyTableState *mtstate,
mtstate->mt_transition_capture->tcs_map = NULL;
}
}
+ if (mtstate->mt_oc_transition_capture != NULL)
+ mtstate->mt_oc_transition_capture->tcs_map =
+ mtstate->mt_transition_tupconv_maps[leaf_part_index];
/*
* We might need to convert from the parent rowtype to the partition
@@ -1157,6 +1160,8 @@ lreplace:;
/* AFTER ROW UPDATE Triggers */
ExecARUpdateTriggers(estate, resultRelInfo, tupleid, oldtuple, tuple,
recheckIndexes,
+ mtstate->operation == CMD_INSERT ?
+ mtstate->mt_oc_transition_capture :
mtstate->mt_transition_capture);
list_free(recheckIndexes);
@@ -1443,7 +1448,7 @@ fireASTriggers(ModifyTableState *node)
if (node->mt_onconflict == ONCONFLICT_UPDATE)
ExecASUpdateTriggers(node->ps.state,
resultRelInfo,
- node->mt_transition_capture);
+ node->mt_oc_transition_capture);
ExecASInsertTriggers(node->ps.state, resultRelInfo,
node->mt_transition_capture);
break;
@@ -1473,14 +1478,24 @@ ExecSetupTransitionCaptureState(ModifyTableState *mtstate, EState *estate)
/* Check for transition tables on the directly targeted relation. */
mtstate->mt_transition_capture =
- MakeTransitionCaptureState(targetRelInfo->ri_TrigDesc);
+ MakeTransitionCaptureState(targetRelInfo->ri_TrigDesc,
+ RelationGetRelid(targetRelInfo->ri_RelationDesc),
+ mtstate->operation);
+ if (mtstate->operation == CMD_INSERT &&
+ mtstate->mt_onconflict == ONCONFLICT_UPDATE)
+ mtstate->mt_oc_transition_capture =
+ MakeTransitionCaptureState(targetRelInfo->ri_TrigDesc,
+ RelationGetRelid(targetRelInfo->ri_RelationDesc),
+ CMD_UPDATE);
/*
* If we found that we need to collect transition tuples then we may also
* need tuple conversion maps for any children that have TupleDescs that
- * aren't compatible with the tuplestores.
+ * aren't compatible with the tuplestores. (We can share these maps
+ * between the regular and ON CONFLICT cases.)
*/
- if (mtstate->mt_transition_capture != NULL)
+ if (mtstate->mt_transition_capture != NULL ||
+ mtstate->mt_oc_transition_capture != NULL)
{
ResultRelInfo *resultRelInfos;
int numResultRelInfos;
@@ -1521,10 +1536,12 @@ ExecSetupTransitionCaptureState(ModifyTableState *mtstate, EState *estate)
/*
* Install the conversion map for the first plan for UPDATE and DELETE
* operations. It will be advanced each time we switch to the next
- * plan. (INSERT operations set it every time.)
+ * plan. (INSERT operations set it every time, so we need not update
+ * mtstate->mt_oc_transition_capture here.)
*/
- mtstate->mt_transition_capture->tcs_map =
- mtstate->mt_transition_tupconv_maps[0];
+ if (mtstate->mt_transition_capture)
+ mtstate->mt_transition_capture->tcs_map =
+ mtstate->mt_transition_tupconv_maps[0];
}
}
@@ -1628,13 +1645,19 @@ ExecModifyTable(PlanState *pstate)
estate->es_result_relation_info = resultRelInfo;
EvalPlanQualSetPlan(&node->mt_epqstate, subplanstate->plan,
node->mt_arowmarks[node->mt_whichplan]);
+ /* Prepare to convert transition tuples from this child. */
if (node->mt_transition_capture != NULL)
{
- /* Prepare to convert transition tuples from this child. */
Assert(node->mt_transition_tupconv_maps != NULL);
node->mt_transition_capture->tcs_map =
node->mt_transition_tupconv_maps[node->mt_whichplan];
}
+ if (node->mt_oc_transition_capture != NULL)
+ {
+ Assert(node->mt_transition_tupconv_maps != NULL);
+ node->mt_oc_transition_capture->tcs_map =
+ node->mt_transition_tupconv_maps[node->mt_whichplan];
+ }
continue;
}
else
@@ -1933,8 +1956,12 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
mtstate->mt_partition_tuple_slot = partition_tuple_slot;
}
- /* Build state for collecting transition tuples */
- ExecSetupTransitionCaptureState(mtstate, estate);
+ /*
+ * Build state for collecting transition tuples. This requires having a
+ * valid trigger query context, so skip it in explain-only mode.
+ */
+ if (!(eflags & EXEC_FLAG_EXPLAIN_ONLY))
+ ExecSetupTransitionCaptureState(mtstate, estate);
/*
* Initialize any WITH CHECK OPTION constraints if needed.
@@ -2318,16 +2345,6 @@ ExecEndModifyTable(ModifyTableState *node)
int i;
/*
- * Free transition tables, unless this query is being run in
- * EXEC_FLAG_SKIP_TRIGGERS mode, which means that it may have queued AFTER
- * triggers that won't be run till later. In that case we'll just leak
- * the transition tables till end of (sub)transaction.
- */
- if (node->mt_transition_capture != NULL &&
- !(node->ps.state->es_top_eflags & EXEC_FLAG_SKIP_TRIGGERS))
- DestroyTransitionCaptureState(node->mt_transition_capture);
-
- /*
* Allow any FDWs to shut down
*/
for (i = 0; i < node->mt_nplans; i++)