From fadc610644cfa352485e2ee8134640b7954f4cef Mon Sep 17 00:00:00 2001 From: amit Date: Thu, 19 Jul 2018 16:14:51 +0900 Subject: [PATCH v12 3/5] Move result rel and ExecRowMark initilization to ExecInitNode InitPlan would do certain initiaization steps, such as creating result rels, ExecRowMarks, etc. only because of the concerns about locking order, which it needs not to do anymore, because we've eliminated executor locking at all. Instead create result rels and ExecRowMarks, etc. in the ExecInit* routines of the respective nodes. --- src/backend/executor/execMain.c | 238 +++++++++++---------------------- src/backend/executor/nodeLockRows.c | 4 +- src/backend/executor/nodeModifyTable.c | 27 +++- src/include/executor/executor.h | 2 +- 4 files changed, 103 insertions(+), 168 deletions(-) diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index d5227ae129..0e77c3b66f 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -836,98 +836,44 @@ InitPlan(QueryDesc *queryDesc, int eflags) estate->es_plannedstmt = plannedstmt; /* - * initialize result relation stuff, and open/lock the result rels. - * - * We must do this before initializing the plan tree, else we might try to - * do a lock upgrade if a result rel is also a source rel. + * Allocate memory required for result relations. ResultRelInfos + * themselves are initialized during ExecInitModifyTable of respective + * ModifyTable nodes. */ if (plannedstmt->resultRelations) { - List *resultRelations = plannedstmt->resultRelations; - int numResultRelations = list_length(resultRelations); - ResultRelInfo *resultRelInfos; - ResultRelInfo *resultRelInfo; + int numResultRelations = list_length(plannedstmt->resultRelations); + int num_roots = list_length(plannedstmt->rootResultRelations); - resultRelInfos = (ResultRelInfo *) - palloc(numResultRelations * sizeof(ResultRelInfo)); - resultRelInfo = resultRelInfos; - foreach(l, resultRelations) - { - Index resultRelationIndex = lfirst_int(l); - Relation resultRelation; - - Assert(rt_fetch(resultRelationIndex, rangeTable)->rellockmode == RowExclusiveLock); - resultRelation = ExecRangeTableRelation(estate, resultRelationIndex); - InitResultRelInfo(resultRelInfo, - resultRelation, - resultRelationIndex, - NULL, - estate->es_instrument); - resultRelInfo++; - } - estate->es_result_relations = resultRelInfos; + estate->es_result_relations = (ResultRelInfo *) + palloc(numResultRelations * sizeof(ResultRelInfo)); 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, lock the non-leaf result - * relations too. A subset of these are the roots of respective - * partitioned tables, for which we also allocate ResultRelInfos. - */ - estate->es_root_result_relations = NULL; - estate->es_num_root_result_relations = 0; - if (plannedstmt->nonleafResultRelations) - { - int num_roots = list_length(plannedstmt->rootResultRelations); - - /* - * Firstly, build ResultRelInfos for all the partitioned table - * roots, because we will need them to fire the statement-level - * triggers, if any. - */ - resultRelInfos = (ResultRelInfo *) + /* For partitioned tables that are roots of their partition trees. */ + estate->es_root_result_relations = (ResultRelInfo *) palloc(num_roots * sizeof(ResultRelInfo)); - resultRelInfo = resultRelInfos; - foreach(l, plannedstmt->rootResultRelations) - { - Index resultRelIndex = lfirst_int(l); - Relation resultRelDesc; + estate->es_num_root_result_relations = num_roots; - Assert(rt_fetch(resultRelIndex, rangeTable)->rellockmode == RowExclusiveLock); - resultRelDesc = ExecRangeTableRelation(estate, resultRelIndex); - InitResultRelInfo(resultRelInfo, - resultRelDesc, - lfirst_int(l), - NULL, - estate->es_instrument); - resultRelInfo++; - } - - estate->es_root_result_relations = resultRelInfos; - estate->es_num_root_result_relations = num_roots; - - /* Simply check the rest of them are locked. */ + /* + * Since non-leaf tables of partition trees won't be processed by + * ExecInitModifyTable, so check here that the appropriate lock is + * held by upstream. + */ #ifdef USE_ASSERT_CHECKING - foreach(l, plannedstmt->nonleafResultRelations) - { - Index resultRelIndex = lfirst_int(l); + foreach(l, plannedstmt->nonleafResultRelations) + { + Index resultRelIndex = lfirst_int(l); + Relation resultRelDesc; + Oid reloid = getrelid(resultRelIndex, + estate->es_range_table_array); - /* We locked the roots above. */ - if (!list_member_int(plannedstmt->rootResultRelations, - resultRelIndex)) - { - Relation resultRelDesc; - Oid reloid = getrelid(resultRelIndex, - estate->es_range_table_array); - - resultRelDesc = heap_open(reloid, NoLock); - Assert(CheckRelationLockedByMe(resultRelDesc, RowExclusiveLock, true)); - heap_close(resultRelDesc, NoLock); - } - } -#endif + resultRelDesc = heap_open(reloid, NoLock); + Assert(CheckRelationLockedByMe(resultRelDesc, RowExclusiveLock, true)); + heap_close(resultRelDesc, NoLock); } +#endif } else { @@ -941,67 +887,7 @@ InitPlan(QueryDesc *queryDesc, int eflags) estate->es_num_root_result_relations = 0; } - /* - * Similarly, we have to lock relations selected FOR [KEY] UPDATE/SHARE - * before we initialize the plan tree, else we'd be risking lock upgrades. - * While we are at it, build the ExecRowMark list. Any partitioned child - * tables are ignored here (because isParent=true) and will be locked by - * the first Append or MergeAppend node that references them. (Note that - * the RowMarks corresponding to partitioned child tables are present in - * the same list as the rest, i.e., plannedstmt->rowMarks.) - */ estate->es_rowMarks = NIL; - foreach(l, plannedstmt->rowMarks) - { - PlanRowMark *rc = (PlanRowMark *) lfirst(l); - Oid relid; - Relation relation; - ExecRowMark *erm; - - /* ignore "parent" rowmarks; they are irrelevant at runtime */ - if (rc->isParent) - continue; - - /* get relation's OID (will produce InvalidOid if subquery) */ - relid = getrelid(rc->rti, estate->es_range_table_array); - - switch (rc->markType) - { - case ROW_MARK_EXCLUSIVE: - case ROW_MARK_NOKEYEXCLUSIVE: - case ROW_MARK_SHARE: - case ROW_MARK_KEYSHARE: - case ROW_MARK_REFERENCE: - relation = ExecRangeTableRelation(estate, rc->rti); - break; - case ROW_MARK_COPY: - /* no physical table access is required */ - relation = NULL; - break; - default: - elog(ERROR, "unrecognized markType: %d", rc->markType); - relation = NULL; /* keep compiler quiet */ - break; - } - - /* Check that relation is a legal target for marking */ - if (relation) - CheckValidRowMarkRel(relation, rc->markType); - - erm = (ExecRowMark *) palloc(sizeof(ExecRowMark)); - erm->relation = relation; - erm->relid = relid; - erm->rti = rc->rti; - erm->prti = rc->prti; - erm->rowmarkId = rc->rowmarkId; - erm->markType = rc->markType; - erm->strength = rc->strength; - erm->waitPolicy = rc->waitPolicy; - erm->ermActive = false; - ItemPointerSetInvalid(&(erm->curCtid)); - erm->ermExtra = NULL; - estate->es_rowMarks = lappend(estate->es_rowMarks, erm); - } /* * Initialize the executor's tuple table to empty. @@ -1609,7 +1495,6 @@ static void ExecEndPlan(PlanState *planstate, EState *estate) { int num_relations = list_length(estate->es_range_table); - ResultRelInfo *resultRelInfo; int i; ListCell *l; @@ -1643,17 +1528,6 @@ ExecEndPlan(PlanState *planstate, EState *estate) */ ExecResetTupleTable(estate->es_tupleTable, false); - /* - * close indexes of result relation(s) if any - */ - resultRelInfo = estate->es_result_relations; - for (i = estate->es_num_result_relations; i > 0; i--) - { - /* Close indices; the relation itself already closed above */ - ExecCloseIndices(resultRelInfo); - resultRelInfo++; - } - /* likewise close any trigger target relations */ ExecCleanUpTriggerState(estate); } @@ -2409,25 +2283,63 @@ ExecUpdateLockMode(EState *estate, ResultRelInfo *relinfo) } /* - * ExecFindRowMark -- find the ExecRowMark struct for given rangetable index + * ExecBuildRowMark -- create and return an ExecRowMark struct for the given + * PlanRowMark. * - * If no such struct, either return NULL or throw error depending on missing_ok + * The created ExecRowMark is also added to the list of rowmarks in the given + * EState. */ ExecRowMark * -ExecFindRowMark(EState *estate, Index rti, bool missing_ok) +ExecBuildRowMark(EState *estate, PlanRowMark *rc) { - ListCell *lc; + Oid relid; + ExecRowMark *erm; + Relation relation; - foreach(lc, estate->es_rowMarks) + Assert(rc != NULL && rc->rti > 0); + Assert(!rc->isParent); + relid = getrelid(rc->rti, estate->es_range_table_array); + + /* get relation's OID (will produce InvalidOid if subquery) */ + + switch (rc->markType) { - ExecRowMark *erm = (ExecRowMark *) lfirst(lc); - - if (erm->rti == rti) - return erm; + case ROW_MARK_EXCLUSIVE: + case ROW_MARK_NOKEYEXCLUSIVE: + case ROW_MARK_SHARE: + case ROW_MARK_KEYSHARE: + case ROW_MARK_REFERENCE: + relation = ExecRangeTableRelation(estate, rc->rti); + break; + case ROW_MARK_COPY: + /* no physical table access is required */ + relation = NULL; + break; + default: + elog(ERROR, "unrecognized markType: %d", rc->markType); + relation = NULL; /* keep compiler quiet */ + break; } - if (!missing_ok) - elog(ERROR, "failed to find ExecRowMark for rangetable index %u", rti); - return NULL; + + /* Check that relation is a legal target for marking */ + if (relation) + CheckValidRowMarkRel(relation, rc->markType); + + erm = (ExecRowMark *) palloc(sizeof(ExecRowMark)); + erm->relation = relation; + erm->relid = relid; + erm->rti = rc->rti; + erm->prti = rc->prti; + erm->rowmarkId = rc->rowmarkId; + erm->markType = rc->markType; + erm->strength = rc->strength; + erm->waitPolicy = rc->waitPolicy; + erm->ermActive = false; + ItemPointerSetInvalid(&(erm->curCtid)); + erm->ermExtra = NULL; + estate->es_rowMarks = lappend(estate->es_rowMarks, erm); + + return erm; } /* diff --git a/src/backend/executor/nodeLockRows.c b/src/backend/executor/nodeLockRows.c index 6db345ae7a..ff9606342b 100644 --- a/src/backend/executor/nodeLockRows.c +++ b/src/backend/executor/nodeLockRows.c @@ -424,8 +424,8 @@ ExecInitLockRows(LockRows *node, EState *estate, int eflags) /* safety check on size of lr_curtuples array */ Assert(rc->rti > 0 && rc->rti <= lrstate->lr_ntables); - /* find ExecRowMark and build ExecAuxRowMark */ - erm = ExecFindRowMark(estate, rc->rti, false); + /* build the ExecRowMark and ExecAuxRowMark */ + erm = ExecBuildRowMark(estate, rc); aerm = ExecBuildAuxRowMark(erm, outerPlan->targetlist); /* diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 07ac3de5e0..9b584097d7 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -2242,12 +2242,33 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) mtstate->mt_plans = (PlanState **) palloc0(sizeof(PlanState *) * nplans); mtstate->resultRelInfo = estate->es_result_relations + node->resultRelIndex; + resultRelInfo = mtstate->resultRelInfo; + foreach(l, node->resultRelations) + { + Index resultRelationIndex = lfirst_int(l); + Relation rel; + + rel = ExecRangeTableRelation(estate, resultRelationIndex); + InitResultRelInfo(resultRelInfo, rel, resultRelationIndex, NULL, + estate->es_instrument); + resultRelInfo++; + } /* If modifying a partitioned table, initialize the root table info */ if (node->rootResultRelIndex >= 0) + { + Index root_rt_index = linitial_int(node->partitioned_rels); + Relation rel; + mtstate->rootResultRelInfo = estate->es_root_result_relations + node->rootResultRelIndex; + Assert(root_rt_index > 0); + rel = ExecRangeTableRelation(estate, root_rt_index); + InitResultRelInfo(mtstate->rootResultRelInfo, rel, root_rt_index, + NULL, estate->es_instrument); + } + mtstate->mt_arowmarks = (List **) palloc0(sizeof(List *) * nplans); mtstate->mt_nplans = nplans; @@ -2533,8 +2554,8 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) if (rc->isParent) continue; - /* find ExecRowMark (same for all subplans) */ - erm = ExecFindRowMark(estate, rc->rti, false); + /* build the ExecRowMark (same for all subplans) */ + erm = ExecBuildRowMark(estate, rc); /* build ExecAuxRowMark for each subplan */ for (i = 0; i < nplans; i++) @@ -2700,6 +2721,8 @@ ExecEndModifyTable(ModifyTableState *node) resultRelInfo->ri_FdwRoutine->EndForeignModify != NULL) resultRelInfo->ri_FdwRoutine->EndForeignModify(node->ps.state, resultRelInfo); + /* closes indices */ + ExecCloseIndices(resultRelInfo); } /* Close all the partitioned tables, leaf partitions, and their indices */ diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index fdfd48d897..4c3a78b1a7 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -188,7 +188,7 @@ extern void ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo, extern void ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo, TupleTableSlot *slot, EState *estate); extern LockTupleMode ExecUpdateLockMode(EState *estate, ResultRelInfo *relinfo); -extern ExecRowMark *ExecFindRowMark(EState *estate, Index rti, bool missing_ok); +extern ExecRowMark *ExecBuildRowMark(EState *estate, PlanRowMark *rc); extern ExecAuxRowMark *ExecBuildAuxRowMark(ExecRowMark *erm, List *targetlist); extern TupleTableSlot *EvalPlanQual(EState *estate, EPQState *epqstate, Relation relation, Index rti, int lockmode, -- 2.11.0