On 12/10/2020 16:47, Amit Langote wrote:
On Mon, Oct 12, 2020 at 8:12 PM Heikki Linnakangas <hlinn...@iki.fi> wrote:
1. We have many different cleanup/close routines now:
ExecCloseResultRelations, ExecCloseRangeTableRelations, and
ExecCleanUpTriggerState. Do we need them all? It seems to me that we
could merge ExecCloseRangeTableRelations() and
ExecCleanUpTriggerState(), they seem to do roughly the same thing: close
relations that were opened for ResultRelInfos. They are always called
together, except in afterTriggerInvokeEvents(). And in
afterTriggerInvokeEvents() too, there would be no harm in doing both,
even though we know there aren't any entries in the es_result_relations
array at that point.
Hmm, I find trigger result relations to behave differently enough to
deserve a separate function. For example, unlike plan-specified
result relations, they don't point to range table relations and don't
open indices. Maybe the name could be revisited, say,
ExecCloseTriggerResultRelations().
Matter of perception I guess. I still prefer to club them together into
one Close call. It's true that they're slightly different, but they're
also pretty similar. And IMHO they're more similar than different.
Also, maybe call the other functions:
ExecInitPlanResultRelationsArray()
ExecInitPlanResultRelation()
ExecClosePlanResultRelations()
Thoughts?
Hmm. How about initializing the array lazily, on the first
ExecInitPlanResultRelation() call? It's not performance critical, and
that way there's one fewer initialization function that you need to
remember to call.
It occurred to me that if we do that (initialize the array lazily),
there's very little need for the PlannedStmt->resultRelations list
anymore. It's only used in ExecRelationIsTargetRelation(), but if we
assume that ExecRelationIsTargetRelation() is only called after InitPlan
has initialized the result relation for the relation, it can easily
check es_result_relations instead. I think that's a safe assumption.
ExecRelationIsTargetRelation() is only used in FDWs, and I believe the
FDWs initialization routine can only be called after ExecInitModifyTable
has been called on the relation.
The PlannedStmt->rootResultRelations field is even more useless.
Actually, maybe we don't need to be so paranoid about setting up
es_result_relations in worker.c, because none of the downstream
functionality invoked seems to rely on it, that is, no need to call
ExecInitResultRelationsArray() and ExecInitResultRelation().
ExecSimpleRelation* and downstream functionality assume a
single-relation operation and the ResultRelInfo is explicitly passed.
Hmm, yeah, I like that. Similarly in ExecuteTruncateGuts(), there isn't
actually any need to put the ResultRelInfos in the es_result_relations
array.
Putting all this together, I ended up with the attached. It doesn't
include the subsequent commits in this patch set yet, for removal of
es_result_relation_info et al.
- Heikki
>From c3f24ca219aa3d535df48c7add376ee3f8f4bc1e Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakan...@iki.fi>
Date: Mon, 12 Oct 2020 19:43:55 +0300
Subject: [PATCH v15 1/1] Make es_result_relations array indexable by RT index.
Create each ResultRelInfo on demand in ExecInitModifyTable, instead of
initializing the whole array at once in InitPlan. And instead of having
a separate concept of a "result rel index", access the array using the
range table indexes. This allows several simplifications. The arrays of
regular result rels and root result are merged into one array. And we
don't need the global resultRelations list in PlannedStmt anymore.
Author: Amit Langote
Discussion: https://www.postgresql.org/message-id/CA%2BHiwqGEmiib8FLiHMhKB%2BCH5dRgHSLc5N5wnvc4kym%2BZYpQEQ%40mail.gmail.com
---
src/backend/commands/copy.c | 24 +--
src/backend/commands/explain.c | 17 +-
src/backend/commands/tablecmds.c | 11 +-
src/backend/commands/trigger.c | 2 +-
src/backend/executor/execMain.c | 256 +++++++----------------
src/backend/executor/execParallel.c | 2 -
src/backend/executor/execUtils.c | 58 +++--
src/backend/executor/nodeModifyTable.c | 24 ++-
src/backend/nodes/copyfuncs.c | 4 -
src/backend/nodes/outfuncs.c | 6 -
src/backend/nodes/readfuncs.c | 4 -
src/backend/optimizer/plan/createplan.c | 2 -
src/backend/optimizer/plan/planner.c | 6 -
src/backend/optimizer/plan/setrefs.c | 30 +--
src/backend/replication/logical/worker.c | 32 +--
src/include/executor/executor.h | 5 +-
src/include/nodes/execnodes.h | 19 +-
src/include/nodes/pathnodes.h | 4 -
src/include/nodes/plannodes.h | 11 -
19 files changed, 188 insertions(+), 329 deletions(-)
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 3c7dbad27a..ca05702139 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -2727,6 +2727,7 @@ CopyFrom(CopyState cstate)
bool leafpart_use_multi_insert = false;
Assert(cstate->rel);
+ Assert(list_length(cstate->range_table) == 1);
/*
* The target must be a plain, foreign, or partitioned relation, or have
@@ -2829,25 +2830,17 @@ CopyFrom(CopyState cstate)
* index-entry-making machinery. (There used to be a huge amount of code
* here that basically duplicated execUtils.c ...)
*/
- resultRelInfo = makeNode(ResultRelInfo);
- InitResultRelInfo(resultRelInfo,
- cstate->rel,
- 1, /* must match rel's position in range_table */
- NULL,
- 0);
- target_resultRelInfo = resultRelInfo;
+ ExecInitRangeTable(estate, cstate->range_table);
+ resultRelInfo = target_resultRelInfo = makeNode(ResultRelInfo);
+ ExecInitResultRelation(estate, resultRelInfo, 1);
/* Verify the named relation is a valid target for INSERT */
CheckValidResultRel(resultRelInfo, CMD_INSERT);
ExecOpenIndices(resultRelInfo, false);
- estate->es_result_relations = resultRelInfo;
- estate->es_num_result_relations = 1;
estate->es_result_relation_info = resultRelInfo;
- ExecInitRangeTable(estate, cstate->range_table);
-
/*
* Set up a ModifyTableState so we can let FDW(s) init themselves for
* foreign-table result relation(s).
@@ -2856,7 +2849,7 @@ CopyFrom(CopyState cstate)
mtstate->ps.plan = NULL;
mtstate->ps.state = estate;
mtstate->operation = CMD_INSERT;
- mtstate->resultRelInfo = estate->es_result_relations;
+ mtstate->resultRelInfo = resultRelInfo;
if (resultRelInfo->ri_FdwRoutine != NULL &&
resultRelInfo->ri_FdwRoutine->BeginForeignInsert != NULL)
@@ -3359,14 +3352,13 @@ CopyFrom(CopyState cstate)
if (insertMethod != CIM_SINGLE)
CopyMultiInsertInfoCleanup(&multiInsertInfo);
- ExecCloseIndices(target_resultRelInfo);
-
/* Close all the partitioned tables, leaf partitions, and their indices */
if (proute)
ExecCleanupTupleRouting(mtstate, proute);
- /* Close any trigger target relations */
- ExecCleanUpTriggerState(estate);
+ /* Close the result relations */
+ ExecCloseResultRelations(estate);
+ ExecCloseRangeTableRelations(estate);
FreeExecutorState(estate);
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index c98c9b5547..c8e292adfa 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -769,27 +769,24 @@ ExplainPrintTriggers(ExplainState *es, QueryDesc *queryDesc)
{
ResultRelInfo *rInfo;
bool show_relname;
- int numrels = queryDesc->estate->es_num_result_relations;
- int numrootrels = queryDesc->estate->es_num_root_result_relations;
+ List *resultrels;
List *routerels;
List *targrels;
- int nr;
ListCell *l;
+ resultrels = queryDesc->estate->es_opened_result_relations;
routerels = queryDesc->estate->es_tuple_routing_result_relations;
targrels = queryDesc->estate->es_trig_target_relations;
ExplainOpenGroup("Triggers", "Triggers", false, es);
- show_relname = (numrels > 1 || numrootrels > 0 ||
+ show_relname = (list_length(resultrels) > 1 ||
routerels != NIL || targrels != NIL);
- rInfo = queryDesc->estate->es_result_relations;
- for (nr = 0; nr < numrels; rInfo++, nr++)
- report_triggers(rInfo, show_relname, es);
-
- rInfo = queryDesc->estate->es_root_result_relations;
- for (nr = 0; nr < numrootrels; rInfo++, nr++)
+ foreach(l, resultrels)
+ {
+ rInfo = (ResultRelInfo *) lfirst(l);
report_triggers(rInfo, show_relname, es);
+ }
foreach(l, routerels)
{
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index e0ac4e05e5..1f0b0f3ef9 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1787,11 +1787,18 @@ ExecuteTruncateGuts(List *explicit_rels, List *relids, List *relids_logged,
/*
* To fire triggers, we'll need an EState as well as a ResultRelInfo for
* each relation. We don't need to call ExecOpenIndices, though.
+ *
+ * We put the ResultRelInfos in the es_opened_result_relations list, even
+ * though we don't have a range table and don't populate the
+ * es_result_relations array. That's a big bogus, but it's enough to make
+ * ExecGetTriggerResultRel() find them.
*/
estate = CreateExecutorState();
resultRelInfos = (ResultRelInfo *)
palloc(list_length(rels) * sizeof(ResultRelInfo));
resultRelInfo = resultRelInfos;
+ estate->es_result_relations = (ResultRelInfo **)
+ palloc(list_length(rels) * sizeof(ResultRelInfo *));
foreach(cell, rels)
{
Relation rel = (Relation) lfirst(cell);
@@ -1801,10 +1808,10 @@ ExecuteTruncateGuts(List *explicit_rels, List *relids, List *relids_logged,
0, /* dummy rangetable index */
NULL,
0);
+ estate->es_opened_result_relations =
+ lappend(estate->es_opened_result_relations, resultRelInfo);
resultRelInfo++;
}
- estate->es_result_relations = resultRelInfos;
- estate->es_num_result_relations = list_length(rels);
/*
* Process all BEFORE STATEMENT TRUNCATE triggers before we begin
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 672fccff5b..3b4fbdadf4 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -4227,7 +4227,7 @@ afterTriggerInvokeEvents(AfterTriggerEventList *events,
if (local_estate)
{
- ExecCleanUpTriggerState(estate);
+ ExecCloseResultRelations(estate);
ExecResetTupleTable(estate->es_tupleTable, false);
FreeExecutorState(estate);
}
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 2e27e26ba4..e32f5984e5 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -827,86 +827,8 @@ InitPlan(QueryDesc *queryDesc, int eflags)
estate->es_plannedstmt = plannedstmt;
- /*
- * Initialize ResultRelInfo data structures, and open the result rels.
- */
- if (plannedstmt->resultRelations)
- {
- List *resultRelations = plannedstmt->resultRelations;
- int numResultRelations = list_length(resultRelations);
- ResultRelInfo *resultRelInfos;
- ResultRelInfo *resultRelInfo;
-
- resultRelInfos = (ResultRelInfo *)
- palloc(numResultRelations * sizeof(ResultRelInfo));
- resultRelInfo = resultRelInfos;
- foreach(l, resultRelations)
- {
- Index resultRelationIndex = lfirst_int(l);
- Relation resultRelation;
-
- resultRelation = ExecGetRangeTableRelation(estate,
- resultRelationIndex);
- InitResultRelInfo(resultRelInfo,
- resultRelation,
- resultRelationIndex,
- NULL,
- estate->es_instrument);
- resultRelInfo++;
- }
- estate->es_result_relations = resultRelInfos;
- estate->es_num_result_relations = numResultRelations;
-
- /* es_result_relation_info is NULL except when within ModifyTable */
- estate->es_result_relation_info = NULL;
-
- /*
- * In the partitioned result relation case, also build ResultRelInfos
- * for all the partitioned table roots, because we will need them to
- * fire statement-level triggers, if any.
- */
- if (plannedstmt->rootResultRelations)
- {
- int num_roots = list_length(plannedstmt->rootResultRelations);
-
- resultRelInfos = (ResultRelInfo *)
- palloc(num_roots * sizeof(ResultRelInfo));
- resultRelInfo = resultRelInfos;
- foreach(l, plannedstmt->rootResultRelations)
- {
- Index resultRelIndex = lfirst_int(l);
- Relation resultRelDesc;
-
- resultRelDesc = ExecGetRangeTableRelation(estate,
- resultRelIndex);
- InitResultRelInfo(resultRelInfo,
- resultRelDesc,
- resultRelIndex,
- NULL,
- estate->es_instrument);
- resultRelInfo++;
- }
-
- estate->es_root_result_relations = resultRelInfos;
- estate->es_num_root_result_relations = num_roots;
- }
- else
- {
- estate->es_root_result_relations = NULL;
- estate->es_num_root_result_relations = 0;
- }
- }
- else
- {
- /*
- * if no result relation, then set state appropriately
- */
- estate->es_result_relations = NULL;
- estate->es_num_result_relations = 0;
- estate->es_result_relation_info = NULL;
- estate->es_root_result_relations = NULL;
- estate->es_num_root_result_relations = 0;
- }
+ /* es_result_relation_info is NULL except when within ModifyTable */
+ estate->es_result_relation_info = NULL;
/*
* Next, build the ExecRowMark array from the PlanRowMark(s), if any.
@@ -1334,8 +1256,7 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo,
*
* Most of the time, triggers are fired on one of the result relations of the
* query, and so we can just return a member of the es_result_relations array,
- * or the es_root_result_relations array (if any), or the
- * es_tuple_routing_result_relations list (if any). (Note: in self-join
+ * or the es_tuple_routing_result_relations list (if any). (Note: in self-join
* situations there might be multiple members with the same OID; if so it
* doesn't matter which one we pick.)
*
@@ -1352,30 +1273,16 @@ ResultRelInfo *
ExecGetTriggerResultRel(EState *estate, Oid relid)
{
ResultRelInfo *rInfo;
- int nr;
ListCell *l;
Relation rel;
MemoryContext oldcontext;
/* First, search through the query result relations */
- rInfo = estate->es_result_relations;
- nr = estate->es_num_result_relations;
- while (nr > 0)
- {
- if (RelationGetRelid(rInfo->ri_RelationDesc) == relid)
- return rInfo;
- rInfo++;
- nr--;
- }
- /* Second, search through the root result relations, if any */
- rInfo = estate->es_root_result_relations;
- nr = estate->es_num_root_result_relations;
- while (nr > 0)
+ foreach(l, estate->es_opened_result_relations)
{
+ rInfo = lfirst(l);
if (RelationGetRelid(rInfo->ri_RelationDesc) == relid)
return rInfo;
- rInfo++;
- nr--;
}
/*
@@ -1428,35 +1335,6 @@ ExecGetTriggerResultRel(EState *estate, Oid relid)
return rInfo;
}
-/*
- * Close any relations that have been opened by ExecGetTriggerResultRel().
- */
-void
-ExecCleanUpTriggerState(EState *estate)
-{
- ListCell *l;
-
- foreach(l, estate->es_trig_target_relations)
- {
- ResultRelInfo *resultRelInfo = (ResultRelInfo *) lfirst(l);
-
- /*
- * Assert this is a "dummy" ResultRelInfo, see above. Otherwise we
- * might be issuing a duplicate close against a Relation opened by
- * ExecGetRangeTableRelation.
- */
- Assert(resultRelInfo->ri_RangeTableIndex == 0);
-
- /*
- * Since ExecGetTriggerResultRel doesn't call ExecOpenIndices for
- * these rels, we needn't call ExecCloseIndices either.
- */
- Assert(resultRelInfo->ri_NumIndices == 0);
-
- table_close(resultRelInfo->ri_RelationDesc, NoLock);
- }
-}
-
/* ----------------------------------------------------------------
* ExecPostprocessPlan
*
@@ -1512,9 +1390,6 @@ ExecPostprocessPlan(EState *estate)
static void
ExecEndPlan(PlanState *planstate, EState *estate)
{
- ResultRelInfo *resultRelInfo;
- Index num_relations;
- Index i;
ListCell *l;
/*
@@ -1540,30 +1415,70 @@ ExecEndPlan(PlanState *planstate, EState *estate)
*/
ExecResetTupleTable(estate->es_tupleTable, false);
+ /* Close result relation(s), if any. */
+ ExecCloseResultRelations(estate);
+
+ /*
+ * close whatever rangetable Relations have been opened. We do not
+ * release any locks we might hold on those rels.
+ */
+ ExecCloseRangeTableRelations(estate);
+}
+
+/*
+ * Close any relations that have been opened for ResultRelInfos.
+ */
+void
+ExecCloseResultRelations(EState *estate)
+{
+ ListCell *l;
+
/*
- * close indexes of result relation(s) if any. (Rels themselves get
- * closed next.)
+ * close indexes of result relation(s) if any. (Rels themselves are
+ * closed in ExecCloseRangeTableRelations())
*/
- resultRelInfo = estate->es_result_relations;
- for (i = estate->es_num_result_relations; i > 0; i--)
+ foreach(l, estate->es_opened_result_relations)
{
+ ResultRelInfo *resultRelInfo = lfirst(l);
+
ExecCloseIndices(resultRelInfo);
- resultRelInfo++;
}
- /*
- * close whatever rangetable Relations have been opened. We do not
- * release any locks we might hold on those rels.
- */
- num_relations = estate->es_range_table_size;
- for (i = 0; i < num_relations; i++)
+ /* Close any relations that have been opened by ExecGetTriggerResultRel(). */
+ foreach(l, estate->es_trig_target_relations)
+ {
+ ResultRelInfo *resultRelInfo = (ResultRelInfo *) lfirst(l);
+
+ /*
+ * Assert this is a "dummy" ResultRelInfo, see above. Otherwise we
+ * might be issuing a duplicate close against a Relation opened by
+ * ExecGetRangeTableRelation.
+ */
+ Assert(resultRelInfo->ri_RangeTableIndex == 0);
+
+ /*
+ * Since ExecGetTriggerResultRel doesn't call ExecOpenIndices for
+ * these rels, we needn't call ExecCloseIndices either.
+ */
+ Assert(resultRelInfo->ri_NumIndices == 0);
+
+ table_close(resultRelInfo->ri_RelationDesc, NoLock);
+ }
+}
+
+/*
+ * Close all relations opened by ExecGetRangeTableRelation()
+ */
+void
+ExecCloseRangeTableRelations(EState *estate)
+{
+ int i;
+
+ for (i = 0; i < estate->es_range_table_size; i++)
{
if (estate->es_relations[i])
table_close(estate->es_relations[i], NoLock);
}
-
- /* likewise close any trigger target relations */
- ExecCleanUpTriggerState(estate);
}
/* ----------------------------------------------------------------
@@ -2758,17 +2673,9 @@ EvalPlanQualStart(EPQState *epqstate, Plan *planTree)
/*
* Child EPQ EStates share the parent's copy of unchanging state such as
- * the snapshot, rangetable, result-rel info, and external Param info.
- * They need their own copies of local state, including a tuple table,
- * es_param_exec_vals, etc.
- *
- * The ResultRelInfo array management is trickier than it looks. We
- * create fresh arrays for the child but copy all the content from the
- * parent. This is because it's okay for the child to share any
- * per-relation state the parent has already created --- but if the child
- * sets up any ResultRelInfo fields, such as its own junkfilter, that
- * state must *not* propagate back to the parent. (For one thing, the
- * pointed-to data is in a memory context that won't last long enough.)
+ * the snapshot, rangetable, and external Param info. They need their own
+ * copies of local state, including a tuple table, es_param_exec_vals,
+ * result-rel info, etc.
*/
rcestate->es_direction = ForwardScanDirection;
rcestate->es_snapshot = parentestate->es_snapshot;
@@ -2781,30 +2688,12 @@ EvalPlanQualStart(EPQState *epqstate, Plan *planTree)
rcestate->es_plannedstmt = parentestate->es_plannedstmt;
rcestate->es_junkFilter = parentestate->es_junkFilter;
rcestate->es_output_cid = parentestate->es_output_cid;
- if (parentestate->es_num_result_relations > 0)
- {
- int numResultRelations = parentestate->es_num_result_relations;
- int numRootResultRels = parentestate->es_num_root_result_relations;
- ResultRelInfo *resultRelInfos;
-
- resultRelInfos = (ResultRelInfo *)
- palloc(numResultRelations * sizeof(ResultRelInfo));
- memcpy(resultRelInfos, parentestate->es_result_relations,
- numResultRelations * sizeof(ResultRelInfo));
- rcestate->es_result_relations = resultRelInfos;
- rcestate->es_num_result_relations = numResultRelations;
-
- /* Also transfer partitioned root result relations. */
- if (numRootResultRels > 0)
- {
- resultRelInfos = (ResultRelInfo *)
- palloc(numRootResultRels * sizeof(ResultRelInfo));
- memcpy(resultRelInfos, parentestate->es_root_result_relations,
- numRootResultRels * sizeof(ResultRelInfo));
- rcestate->es_root_result_relations = resultRelInfos;
- rcestate->es_num_root_result_relations = numRootResultRels;
- }
- }
+
+ /*
+ * ResultRelInfos needed by subplans are initialized from scratch when the
+ * subplans themselves are initialized.
+ */
+ parentestate->es_result_relations = NULL;
/* es_result_relation_info must NOT be copied */
/* es_trig_target_relations must NOT be copied */
rcestate->es_top_eflags = parentestate->es_top_eflags;
@@ -2914,8 +2803,9 @@ EvalPlanQualStart(EPQState *epqstate, Plan *planTree)
* This is a cut-down version of ExecutorEnd(); basically we want to do most
* of the normal cleanup, but *not* close result relations (which we are
* just sharing from the outer query). We do, however, have to close any
- * trigger target relations that got opened, since those are not shared.
- * (There probably shouldn't be any of the latter, but just in case...)
+ * result and trigger target relations that got opened, since those are not
+ * shared. (There probably shouldn't be any of the latter, but just in
+ * case...)
*/
void
EvalPlanQualEnd(EPQState *epqstate)
@@ -2957,8 +2847,8 @@ EvalPlanQualEnd(EPQState *epqstate)
/* throw away the per-estate tuple table, some node may have used it */
ExecResetTupleTable(estate->es_tupleTable, false);
- /* close any trigger target relations attached to this EState */
- ExecCleanUpTriggerState(estate);
+ /* Close any result and trigger target relations attached to this EState */
+ ExecCloseResultRelations(estate);
MemoryContextSwitchTo(oldcontext);
diff --git a/src/backend/executor/execParallel.c b/src/backend/executor/execParallel.c
index 382e78fb7f..d85d0fdb64 100644
--- a/src/backend/executor/execParallel.c
+++ b/src/backend/executor/execParallel.c
@@ -183,8 +183,6 @@ ExecSerializePlan(Plan *plan, EState *estate)
pstmt->parallelModeNeeded = false;
pstmt->planTree = plan;
pstmt->rtable = estate->es_range_table;
- pstmt->resultRelations = NIL;
- pstmt->rootResultRelations = NIL;
pstmt->appendRelations = NIL;
/*
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index d0e65b8647..c41cdc79e4 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -124,14 +124,9 @@ CreateExecutorState(void)
estate->es_output_cid = (CommandId) 0;
estate->es_result_relations = NULL;
- estate->es_num_result_relations = 0;
+ estate->es_opened_result_relations = NIL;
estate->es_result_relation_info = NULL;
-
- estate->es_root_result_relations = NULL;
- estate->es_num_root_result_relations = 0;
-
estate->es_tuple_routing_result_relations = NIL;
-
estate->es_trig_target_relations = NIL;
estate->es_param_list_info = NULL;
@@ -711,16 +706,10 @@ ExecCreateScanSlotFromOuterPlan(EState *estate,
bool
ExecRelationIsTargetRelation(EState *estate, Index scanrelid)
{
- ResultRelInfo *resultRelInfos;
- int i;
+ if (!estate->es_result_relations)
+ return false;
- resultRelInfos = estate->es_result_relations;
- for (i = 0; i < estate->es_num_result_relations; i++)
- {
- if (resultRelInfos[i].ri_RangeTableIndex == scanrelid)
- return true;
- }
- return false;
+ return estate->es_result_relations[scanrelid - 1] != NULL;
}
/* ----------------------------------------------------------------
@@ -779,9 +768,10 @@ ExecInitRangeTable(EState *estate, List *rangeTable)
palloc0(estate->es_range_table_size * sizeof(Relation));
/*
- * es_rowmarks is also parallel to the es_range_table, but it's allocated
- * only if needed.
+ * es_result_relations and es_rowmarks are also parallel to es_range_table,
+ * but are allocated only if needed.
*/
+ estate->es_result_relations = NULL;
estate->es_rowmarks = NULL;
}
@@ -835,6 +825,40 @@ ExecGetRangeTableRelation(EState *estate, Index rti)
return rel;
}
+/*
+ * ExecInitResultRelation
+ * Open result relation given by the passed-in RT index and fill its
+ * ResultRelInfo node
+ *
+ * Here, we also save the ResultRelInfo in estate->es_result_relations array
+ * such that it can be accessed later using the RT index.
+ */
+void
+ExecInitResultRelation(EState *estate, ResultRelInfo *resultRelInfo,
+ Index rti)
+{
+ Relation resultRelationDesc;
+
+ resultRelationDesc = ExecGetRangeTableRelation(estate, rti);
+ InitResultRelInfo(resultRelInfo,
+ resultRelationDesc,
+ rti,
+ NULL,
+ estate->es_instrument);
+
+ if (estate->es_result_relations == NULL)
+ estate->es_result_relations = (ResultRelInfo **)
+ palloc0(estate->es_range_table_size * sizeof(ResultRelInfo *));
+ estate->es_result_relations[rti - 1] = resultRelInfo;
+
+ /*
+ * Saving in the list allows to avoid needlessly traversing the whole
+ * array when only a few of its entries are possibly non-NULL.
+ */
+ estate->es_opened_result_relations =
+ lappend(estate->es_opened_result_relations, resultRelInfo);
+}
+
/*
* UpdateChangedParamSet
* Add changed parameters to a plan node's chgParam set
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 9812089161..9b27d34ba5 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -2301,7 +2301,8 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
ResultRelInfo *saved_resultRelInfo;
ResultRelInfo *resultRelInfo;
Plan *subplan;
- ListCell *l;
+ ListCell *l,
+ *l1;
int i;
Relation rel;
bool update_tuple_routing_needed = node->partColsUpdated;
@@ -2322,13 +2323,17 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
mtstate->mt_done = false;
mtstate->mt_plans = (PlanState **) palloc0(sizeof(PlanState *) * nplans);
- mtstate->resultRelInfo = estate->es_result_relations + node->resultRelIndex;
+ mtstate->resultRelInfo = (ResultRelInfo *)
+ palloc(nplans * sizeof(ResultRelInfo));
mtstate->mt_scans = (TupleTableSlot **) palloc0(sizeof(TupleTableSlot *) * nplans);
/* If modifying a partitioned table, initialize the root table info */
- if (node->rootResultRelIndex >= 0)
- mtstate->rootResultRelInfo = estate->es_root_result_relations +
- node->rootResultRelIndex;
+ if (node->rootRelation > 0)
+ {
+ mtstate->rootResultRelInfo = makeNode(ResultRelInfo);
+ ExecInitResultRelation(estate, mtstate->rootResultRelInfo,
+ node->rootRelation);
+ }
mtstate->mt_arowmarks = (List **) palloc0(sizeof(List *) * nplans);
mtstate->mt_nplans = nplans;
@@ -2351,9 +2356,14 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
resultRelInfo = mtstate->resultRelInfo;
i = 0;
- foreach(l, node->plans)
+ forboth(l, node->resultRelations, l1, node->plans)
{
- subplan = (Plan *) lfirst(l);
+ Index resultRelation = lfirst_int(l);
+
+ subplan = (Plan *) lfirst(l1);
+
+ /* This opens result relation and fills ResultRelInfo. */
+ ExecInitResultRelation(estate, resultRelInfo, resultRelation);
/* Initialize the usesFdwDirectModify flag */
resultRelInfo->ri_usesFdwDirectModify = bms_is_member(i,
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 0409a40b82..e6b48c73ff 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -90,8 +90,6 @@ _copyPlannedStmt(const PlannedStmt *from)
COPY_SCALAR_FIELD(jitFlags);
COPY_NODE_FIELD(planTree);
COPY_NODE_FIELD(rtable);
- COPY_NODE_FIELD(resultRelations);
- COPY_NODE_FIELD(rootResultRelations);
COPY_NODE_FIELD(appendRelations);
COPY_NODE_FIELD(subplans);
COPY_BITMAPSET_FIELD(rewindPlanIDs);
@@ -207,8 +205,6 @@ _copyModifyTable(const ModifyTable *from)
COPY_SCALAR_FIELD(rootRelation);
COPY_SCALAR_FIELD(partColsUpdated);
COPY_NODE_FIELD(resultRelations);
- COPY_SCALAR_FIELD(resultRelIndex);
- COPY_SCALAR_FIELD(rootResultRelIndex);
COPY_NODE_FIELD(plans);
COPY_NODE_FIELD(withCheckOptionLists);
COPY_NODE_FIELD(returningLists);
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index f0386480ab..fae01357d8 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -308,8 +308,6 @@ _outPlannedStmt(StringInfo str, const PlannedStmt *node)
WRITE_INT_FIELD(jitFlags);
WRITE_NODE_FIELD(planTree);
WRITE_NODE_FIELD(rtable);
- WRITE_NODE_FIELD(resultRelations);
- WRITE_NODE_FIELD(rootResultRelations);
WRITE_NODE_FIELD(appendRelations);
WRITE_NODE_FIELD(subplans);
WRITE_BITMAPSET_FIELD(rewindPlanIDs);
@@ -408,8 +406,6 @@ _outModifyTable(StringInfo str, const ModifyTable *node)
WRITE_UINT_FIELD(rootRelation);
WRITE_BOOL_FIELD(partColsUpdated);
WRITE_NODE_FIELD(resultRelations);
- WRITE_INT_FIELD(resultRelIndex);
- WRITE_INT_FIELD(rootResultRelIndex);
WRITE_NODE_FIELD(plans);
WRITE_NODE_FIELD(withCheckOptionLists);
WRITE_NODE_FIELD(returningLists);
@@ -2193,8 +2189,6 @@ _outPlannerGlobal(StringInfo str, const PlannerGlobal *node)
WRITE_BITMAPSET_FIELD(rewindPlanIDs);
WRITE_NODE_FIELD(finalrtable);
WRITE_NODE_FIELD(finalrowmarks);
- WRITE_NODE_FIELD(resultRelations);
- WRITE_NODE_FIELD(rootResultRelations);
WRITE_NODE_FIELD(appendRelations);
WRITE_NODE_FIELD(relationOids);
WRITE_NODE_FIELD(invalItems);
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index 42050ab719..867d1360b8 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -1541,8 +1541,6 @@ _readPlannedStmt(void)
READ_INT_FIELD(jitFlags);
READ_NODE_FIELD(planTree);
READ_NODE_FIELD(rtable);
- READ_NODE_FIELD(resultRelations);
- READ_NODE_FIELD(rootResultRelations);
READ_NODE_FIELD(appendRelations);
READ_NODE_FIELD(subplans);
READ_BITMAPSET_FIELD(rewindPlanIDs);
@@ -1639,8 +1637,6 @@ _readModifyTable(void)
READ_UINT_FIELD(rootRelation);
READ_BOOL_FIELD(partColsUpdated);
READ_NODE_FIELD(resultRelations);
- READ_INT_FIELD(resultRelIndex);
- READ_INT_FIELD(rootResultRelIndex);
READ_NODE_FIELD(plans);
READ_NODE_FIELD(withCheckOptionLists);
READ_NODE_FIELD(returningLists);
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 3d7a4e373f..881eaf4813 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -6808,8 +6808,6 @@ make_modifytable(PlannerInfo *root,
node->rootRelation = rootRelation;
node->partColsUpdated = partColsUpdated;
node->resultRelations = resultRelations;
- node->resultRelIndex = -1; /* will be set correctly in setrefs.c */
- node->rootResultRelIndex = -1; /* will be set correctly in setrefs.c */
node->plans = subplans;
if (!onconflict)
{
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index f331f82a6c..3e4de96b8e 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -304,8 +304,6 @@ standard_planner(Query *parse, const char *query_string, int cursorOptions,
glob->rewindPlanIDs = NULL;
glob->finalrtable = NIL;
glob->finalrowmarks = NIL;
- glob->resultRelations = NIL;
- glob->rootResultRelations = NIL;
glob->appendRelations = NIL;
glob->relationOids = NIL;
glob->invalItems = NIL;
@@ -492,8 +490,6 @@ standard_planner(Query *parse, const char *query_string, int cursorOptions,
/* final cleanup of the plan */
Assert(glob->finalrtable == NIL);
Assert(glob->finalrowmarks == NIL);
- Assert(glob->resultRelations == NIL);
- Assert(glob->rootResultRelations == NIL);
Assert(glob->appendRelations == NIL);
top_plan = set_plan_references(root, top_plan);
/* ... and the subplans (both regular subplans and initplans) */
@@ -519,8 +515,6 @@ standard_planner(Query *parse, const char *query_string, int cursorOptions,
result->parallelModeNeeded = glob->parallelModeNeeded;
result->planTree = top_plan;
result->rtable = glob->finalrtable;
- result->resultRelations = glob->resultRelations;
- result->rootResultRelations = glob->rootResultRelations;
result->appendRelations = glob->appendRelations;
result->subplans = glob->subplans;
result->rewindPlanIDs = glob->rewindPlanIDs;
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index dd8e2e966d..be151416af 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -235,9 +235,8 @@ static List *set_returning_clause_references(PlannerInfo *root,
* different when the passed-in Plan is a node we decide isn't needed.
*
* The flattened rangetable entries are appended to root->glob->finalrtable.
- * Also, rowmarks entries are appended to root->glob->finalrowmarks, and the
- * RT indexes of ModifyTable result relations to root->glob->resultRelations,
- * and flattened AppendRelInfos are appended to root->glob->appendRelations.
+ * Also, rowmarks entries are appended to root->glob->finalrowmarks, and
+ * flattened AppendRelInfos are appended to root->glob->appendRelations.
* Plan dependencies are appended to root->glob->relationOids (for relations)
* and root->glob->invalItems (for everything else).
*
@@ -972,31 +971,6 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
(Plan *) lfirst(l),
rtoffset);
}
-
- /*
- * Append this ModifyTable node's final result relation RT
- * index(es) to the global list for the plan, and set its
- * resultRelIndex to reflect their starting position in the
- * global list.
- */
- splan->resultRelIndex = list_length(root->glob->resultRelations);
- root->glob->resultRelations =
- list_concat(root->glob->resultRelations,
- splan->resultRelations);
-
- /*
- * If the main target relation is a partitioned table, also
- * add the partition root's RT index to rootResultRelations,
- * and remember its index in that list in rootResultRelIndex.
- */
- if (splan->rootRelation)
- {
- splan->rootResultRelIndex =
- list_length(root->glob->rootResultRelations);
- root->glob->rootResultRelations =
- lappend_int(root->glob->rootResultRelations,
- splan->rootRelation);
- }
}
break;
case T_Append:
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 9c6fdeeb56..8d5d9e05b3 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -344,7 +344,6 @@ static EState *
create_estate_for_relation(LogicalRepRelMapEntry *rel)
{
EState *estate;
- ResultRelInfo *resultRelInfo;
RangeTblEntry *rte;
estate = CreateExecutorState();
@@ -356,13 +355,6 @@ create_estate_for_relation(LogicalRepRelMapEntry *rel)
rte->rellockmode = AccessShareLock;
ExecInitRangeTable(estate, list_make1(rte));
- resultRelInfo = makeNode(ResultRelInfo);
- InitResultRelInfo(resultRelInfo, rel->localrel, 1, NULL, 0);
-
- estate->es_result_relations = resultRelInfo;
- estate->es_num_result_relations = 1;
- estate->es_result_relation_info = resultRelInfo;
-
estate->es_output_cid = GetCurrentCommandId(true);
/* Prepare to catch AFTER triggers. */
@@ -1150,6 +1142,7 @@ GetRelationIdentityOrPK(Relation rel)
static void
apply_handle_insert(StringInfo s)
{
+ ResultRelInfo *resultRelInfo;
LogicalRepRelMapEntry *rel;
LogicalRepTupleData newtup;
LogicalRepRelId relid;
@@ -1179,6 +1172,9 @@ apply_handle_insert(StringInfo s)
remoteslot = ExecInitExtraTupleSlot(estate,
RelationGetDescr(rel->localrel),
&TTSOpsVirtual);
+ resultRelInfo = makeNode(ResultRelInfo);
+ InitResultRelInfo(resultRelInfo, rel->localrel, 1, NULL, 0);
+ estate->es_result_relation_info = resultRelInfo;
/* Input functions may need an active snapshot, so get one */
PushActiveSnapshot(GetTransactionSnapshot());
@@ -1191,10 +1187,10 @@ apply_handle_insert(StringInfo s)
/* For a partitioned table, insert the tuple into a partition. */
if (rel->localrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
- apply_handle_tuple_routing(estate->es_result_relation_info, estate,
+ apply_handle_tuple_routing(resultRelInfo, estate,
remoteslot, NULL, rel, CMD_INSERT);
else
- apply_handle_insert_internal(estate->es_result_relation_info, estate,
+ apply_handle_insert_internal(resultRelInfo, estate,
remoteslot);
PopActiveSnapshot();
@@ -1265,6 +1261,7 @@ check_relation_updatable(LogicalRepRelMapEntry *rel)
static void
apply_handle_update(StringInfo s)
{
+ ResultRelInfo *resultRelInfo;
LogicalRepRelMapEntry *rel;
LogicalRepRelId relid;
EState *estate;
@@ -1301,6 +1298,9 @@ apply_handle_update(StringInfo s)
remoteslot = ExecInitExtraTupleSlot(estate,
RelationGetDescr(rel->localrel),
&TTSOpsVirtual);
+ resultRelInfo = makeNode(ResultRelInfo);
+ InitResultRelInfo(resultRelInfo, rel->localrel, 1, NULL, 0);
+ estate->es_result_relation_info = resultRelInfo;
/*
* Populate updatedCols so that per-column triggers can fire. This could
@@ -1337,10 +1337,10 @@ apply_handle_update(StringInfo s)
/* For a partitioned table, apply update to correct partition. */
if (rel->localrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
- apply_handle_tuple_routing(estate->es_result_relation_info, estate,
+ apply_handle_tuple_routing(resultRelInfo, estate,
remoteslot, &newtup, rel, CMD_UPDATE);
else
- apply_handle_update_internal(estate->es_result_relation_info, estate,
+ apply_handle_update_internal(resultRelInfo, estate,
remoteslot, &newtup, rel);
PopActiveSnapshot();
@@ -1420,6 +1420,7 @@ apply_handle_update_internal(ResultRelInfo *relinfo,
static void
apply_handle_delete(StringInfo s)
{
+ ResultRelInfo *resultRelInfo;
LogicalRepRelMapEntry *rel;
LogicalRepTupleData oldtup;
LogicalRepRelId relid;
@@ -1452,6 +1453,9 @@ apply_handle_delete(StringInfo s)
remoteslot = ExecInitExtraTupleSlot(estate,
RelationGetDescr(rel->localrel),
&TTSOpsVirtual);
+ resultRelInfo = makeNode(ResultRelInfo);
+ InitResultRelInfo(resultRelInfo, rel->localrel, 1, NULL, 0);
+ estate->es_result_relation_info = resultRelInfo;
PushActiveSnapshot(GetTransactionSnapshot());
@@ -1462,10 +1466,10 @@ apply_handle_delete(StringInfo s)
/* For a partitioned table, apply delete to correct partition. */
if (rel->localrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
- apply_handle_tuple_routing(estate->es_result_relation_info, estate,
+ apply_handle_tuple_routing(resultRelInfo, estate,
remoteslot, NULL, rel, CMD_DELETE);
else
- apply_handle_delete_internal(estate->es_result_relation_info, estate,
+ apply_handle_delete_internal(resultRelInfo, estate,
remoteslot, &rel->remoterel);
PopActiveSnapshot();
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index 415e117407..c283bf1454 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -191,7 +191,6 @@ extern void InitResultRelInfo(ResultRelInfo *resultRelInfo,
Relation partition_root,
int instrument_options);
extern ResultRelInfo *ExecGetTriggerResultRel(EState *estate, Oid relid);
-extern void ExecCleanUpTriggerState(EState *estate);
extern void ExecConstraints(ResultRelInfo *resultRelInfo,
TupleTableSlot *slot, EState *estate);
extern bool ExecPartitionCheck(ResultRelInfo *resultRelInfo,
@@ -538,6 +537,8 @@ extern bool ExecRelationIsTargetRelation(EState *estate, Index scanrelid);
extern Relation ExecOpenScanRelation(EState *estate, Index scanrelid, int eflags);
extern void ExecInitRangeTable(EState *estate, List *rangeTable);
+extern void ExecCloseRangeTableRelations(EState *estate);
+extern void ExecCloseResultRelations(EState *estate);
static inline RangeTblEntry *
exec_rt_fetch(Index rti, EState *estate)
@@ -546,6 +547,8 @@ exec_rt_fetch(Index rti, EState *estate)
}
extern Relation ExecGetRangeTableRelation(EState *estate, Index rti);
+extern void ExecInitResultRelation(EState *estate, ResultRelInfo *resultRelInfo,
+ Index rti);
extern int executor_errposition(EState *estate, int location);
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index ef448d67c7..8683484050 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -519,23 +519,20 @@ typedef struct EState
CommandId es_output_cid;
/* Info about target table(s) for insert/update/delete queries: */
- ResultRelInfo *es_result_relations; /* array of ResultRelInfos */
- int es_num_result_relations; /* length of array */
+ ResultRelInfo **es_result_relations; /* Array of per-range-table-entry
+ * ResultRelInfo pointers, or NULL
+ * if a given range table relation
+ * not a target table */
+ List *es_opened_result_relations; /* List of non-NULL entries in
+ * es_result_relations added in no
+ * specific order */
ResultRelInfo *es_result_relation_info; /* currently active array elt */
- /*
- * Info about the partition root table(s) for insert/update/delete queries
- * targeting partitioned tables. Only leaf partitions are mentioned in
- * es_result_relations, but we need access to the roots for firing
- * triggers and for runtime tuple routing.
- */
- ResultRelInfo *es_root_result_relations; /* array of ResultRelInfos */
- int es_num_root_result_relations; /* length of the array */
PartitionDirectory es_partition_directory; /* for PartitionDesc lookup */
/*
* The following list contains ResultRelInfos created by the tuple routing
- * code for partitions that don't already have one.
+ * code for partitions that aren't found in the es_result_relations array.
*/
List *es_tuple_routing_result_relations;
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index dbe86e7af6..c3ed0ee8d4 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -118,10 +118,6 @@ typedef struct PlannerGlobal
List *finalrowmarks; /* "flat" list of PlanRowMarks */
- List *resultRelations; /* "flat" list of integer RT indexes */
-
- List *rootResultRelations; /* "flat" list of integer RT indexes */
-
List *appendRelations; /* "flat" list of AppendRelInfos */
List *relationOids; /* OIDs of relations the plan depends on */
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index 83e01074ed..1943ec4f48 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -65,15 +65,6 @@ typedef struct PlannedStmt
List *rtable; /* list of RangeTblEntry nodes */
- /* rtable indexes of target relations for INSERT/UPDATE/DELETE */
- List *resultRelations; /* integer list of RT indexes, or NIL */
-
- /*
- * rtable indexes of partitioned table roots that are UPDATE/DELETE
- * targets; needed for trigger firing.
- */
- List *rootResultRelations;
-
List *appendRelations; /* list of AppendRelInfo nodes */
List *subplans; /* Plan trees for SubPlan expressions; note
@@ -224,8 +215,6 @@ typedef struct ModifyTable
Index rootRelation; /* Root RT index, if target is partitioned */
bool partColsUpdated; /* some part key in hierarchy updated */
List *resultRelations; /* integer list of RT indexes */
- int resultRelIndex; /* index of first resultRel in plan's list */
- int rootResultRelIndex; /* index of the partitioned table root */
List *plans; /* plan(s) producing source data */
List *withCheckOptionLists; /* per-target-table WCO lists */
List *returningLists; /* per-target-table RETURNING tlists */
--
2.20.1
