From 807dde3d299a2d4ae89d96ea7422d0feb9c58e4b Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Fri, 9 Oct 2020 22:09:08 +0300 Subject: [PATCH 2/2] Merge ExecCleanUpTriggerState() and ExecCloseResultRelations(). And some other kibitzing. --- src/backend/commands/copy.c | 9 ++- src/backend/commands/explain.c | 2 +- src/backend/commands/trigger.c | 2 +- src/backend/executor/execMain.c | 83 ++++++++++-------------- src/backend/executor/execUtils.c | 7 +- src/backend/replication/logical/worker.c | 6 +- src/include/executor/executor.h | 1 - src/include/nodes/execnodes.h | 17 ++--- 8 files changed, 59 insertions(+), 68 deletions(-) diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index 6948214334..bfdd366139 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 @@ -3352,15 +3353,13 @@ CopyFrom(CopyState cstate) if (insertMethod != CIM_SINGLE) CopyMultiInsertInfoCleanup(&multiInsertInfo); - ExecCloseResultRelations(estate); - ExecCloseRangeTableRelations(estate); - /* 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 3210f90bd1..1cc47b0e77 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -786,7 +786,7 @@ ExplainPrintTriggers(ExplainState *es, QueryDesc *queryDesc) routerels != NIL || targrels != NIL); foreach(l, resultrels) { - rInfo = lfirst(l); + rInfo = (ResultRelInfo *) lfirst(l); report_triggers(rInfo, show_relname, es); } 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 f62fd8f7aa..403d62f2b6 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -1340,35 +1340,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 * @@ -1449,7 +1420,7 @@ ExecEndPlan(PlanState *planstate, EState *estate) */ ExecResetTupleTable(estate->es_tupleTable, false); - /* Close indexes of result relation(s), if any. */ + /* Close result relation(s), if any. */ ExecCloseResultRelations(estate); /* @@ -1457,22 +1428,19 @@ ExecEndPlan(PlanState *planstate, EState *estate) * release any locks we might hold on those rels. */ ExecCloseRangeTableRelations(estate); - - /* likewise close any trigger target relations */ - ExecCleanUpTriggerState(estate); } /* - * ExecCloseResultRelations + * Close any relations that have been opened for ResultRelInfos. */ void ExecCloseResultRelations(EState *estate) { - ListCell *l; + 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()) */ foreach(l, estate->es_opened_result_relations) { @@ -1480,15 +1448,36 @@ ExecCloseResultRelations(EState *estate) ExecCloseIndices(resultRelInfo); } + + /* 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); + } } /* - * ExecCloseRangeTableRelations + * Close all relations opened by ExecGetRangeTableRelation() */ void ExecCloseRangeTableRelations(EState *estate) { - int i; + int i; for (i = 0; i < estate->es_range_table_size; i++) { @@ -2689,9 +2678,9 @@ EvalPlanQualStart(EPQState *epqstate, Plan *planTree) /* * Child EPQ EStates share the parent's copy of unchanging state such as - * 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. + * 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; @@ -2704,9 +2693,10 @@ 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; + /* - * ResultRelInfos needed by subplans are initialized from scratch when - * the subplans themselves are initialized. + * ResultRelInfos needed by subplans are initialized from scratch when the + * subplans themselves are initialized. */ if (parentestate->es_result_relations) ExecInitResultRelationsArray(rcestate); @@ -2859,13 +2849,10 @@ EvalPlanQualEnd(EPQState *epqstate) ExecEndNode(subplanstate); } - ExecCloseResultRelations(estate); - /* 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); + ExecCloseResultRelations(estate); MemoryContextSwitchTo(oldcontext); diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c index 169dd925ad..eda84bca45 100644 --- a/src/backend/executor/execUtils.c +++ b/src/backend/executor/execUtils.c @@ -770,6 +770,7 @@ ExecInitRangeTable(EState *estate, List *rangeTable) * es_result_relations and es_rowmarks are also parallel to es_range_table, * but are only allocated if needed. */ + estate->es_result_relations = NULL; estate->es_rowmarks = NULL; } @@ -827,9 +828,9 @@ ExecGetRangeTableRelation(EState *estate, Index rti) * ExecInitResultRelationsArray * Allocate space to hold ResultRelInfo pointers of result relations * - * Although not relations in the range table may be result relations, we - * allocate that many pointers, because that allows to access individual - * entries by RT index (minus 1 to be accurate), which is convenient. + * Usually, only some relations in the range table are result relations, but + * we allocate an array with the same size as the range table, so that we + * can index it by the RT index (minus 1 to be accurate). */ void ExecInitResultRelationsArray(EState *estate) diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c index 77f71e52d4..07889f7a7b 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -357,9 +357,13 @@ create_estate_for_relation(LogicalRepRelMapEntry *rel) ExecInitRangeTable(estate, list_make1(rte)); ExecInitResultRelationsArray(estate); + /* + * Initialize a ResultRelInfo for the target relation. Note that we + * intentionally don't add it to the es_opened_result_relations list, + * because we do our own cleanup and don't use ExecCloseResultRelations(). + */ resultRelInfo = makeNode(ResultRelInfo); InitResultRelInfo(resultRelInfo, rel->localrel, 1, NULL, 0); - estate->es_result_relations[0] = resultRelInfo; estate->es_result_relation_info = resultRelInfo; diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 54455e1446..76ef5cd91c 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, diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 7eb5ca6a04..b6500abc24 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -508,14 +508,6 @@ typedef struct EState Index es_range_table_size; /* size of the range table arrays */ Relation *es_relations; /* Array of per-range-table-entry Relation * pointers, or NULL if not yet opened */ - 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 */ struct ExecRowMark **es_rowmarks; /* Array of per-range-table-entry * ExecRowMarks, or NULL if none */ PlannedStmt *es_plannedstmt; /* link to top of plan tree */ @@ -526,6 +518,15 @@ typedef struct EState /* If query can insert/delete tuples, the command ID to mark them with */ CommandId es_output_cid; + /* Info about target table(s) for insert/update/delete queries: */ + 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 */ PartitionDirectory es_partition_directory; /* for PartitionDesc lookup */ -- 2.20.1