From f2343653f5b2aecfc759f36dbb3fd2a61f36853e Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 6 Oct 2018 15:12:51 -0400 Subject: [PATCH] Remove more redundant relation locking during executor startup. We already have appropriate locks on every relation listed in the query's rangetable before we reach the executor. Take the next step in exploiting that knowledge by removing code that worries about taking locks on non-leaf result relations in a partitioned table. In particular, get rid of ExecLockNonLeafAppendTables and a stanza in InitPlan that asserts we already have locks on certain such tables. In passing, clean up some now-obsolete comments in InitPlan. Amit Langote, reviewed by David Rowley and Jesper Pedersen, and whacked around a bit more by me Discussion: https://postgr.es/m/468c85d9-540e-66a2-1dde-fec2b741e688@lab.ntt.co.jp --- src/backend/executor/execMain.c | 56 ++++++------------------ src/backend/executor/execUtils.c | 60 -------------------------- src/backend/executor/nodeAppend.c | 6 --- src/backend/executor/nodeMergeAppend.c | 6 --- src/include/executor/executor.h | 2 - 5 files changed, 14 insertions(+), 116 deletions(-) diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 71b720eec9..23e6749920 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -828,10 +828,7 @@ 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. + * Initialize ResultRelInfo data structures, and open the result rels. */ if (plannedstmt->resultRelations) { @@ -859,25 +856,19 @@ InitPlan(QueryDesc *queryDesc, int eflags) } 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, lock the non-leaf result - * relations too. A subset of these are the roots of respective - * partitioned tables, for which we also allocate ResultRelInfos. + * 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. */ - estate->es_root_result_relations = NULL; - estate->es_num_root_result_relations = 0; - if (plannedstmt->nonleafResultRelations) + if (plannedstmt->rootResultRelations) { 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 *) palloc(num_roots * sizeof(ResultRelInfo)); resultRelInfo = resultRelInfos; @@ -898,26 +889,11 @@ InitPlan(QueryDesc *queryDesc, int eflags) estate->es_root_result_relations = resultRelInfos; estate->es_num_root_result_relations = num_roots; - - /* Simply check the rest of them are locked. */ -#ifdef USE_ASSERT_CHECKING - foreach(l, plannedstmt->nonleafResultRelations) - { - Index resultRelIndex = lfirst_int(l); - - /* We locked the roots above. */ - if (!list_member_int(plannedstmt->rootResultRelations, - resultRelIndex)) - { - Relation resultRelDesc; - Oid reloid = exec_rt_fetch(resultRelIndex, estate)->relid; - - resultRelDesc = heap_open(reloid, NoLock); - Assert(CheckRelationLockedByMe(resultRelDesc, RowExclusiveLock, true)); - heap_close(resultRelDesc, NoLock); - } - } -#endif + } + else + { + estate->es_root_result_relations = NULL; + estate->es_num_root_result_relations = 0; } } else @@ -933,13 +909,7 @@ InitPlan(QueryDesc *queryDesc, int eflags) } /* - * 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.) + * Next, build the ExecRowMark list from the PlanRowMark(s), if any. */ estate->es_rowMarks = NIL; foreach(l, plannedstmt->rowMarks) @@ -956,6 +926,7 @@ InitPlan(QueryDesc *queryDesc, int eflags) /* get relation's OID (will produce InvalidOid if subquery) */ relid = exec_rt_fetch(rc->rti, estate)->relid; + /* open relation, if we need to access it for this mark type */ switch (rc->markType) { case ROW_MARK_EXCLUSIVE: @@ -991,6 +962,7 @@ InitPlan(QueryDesc *queryDesc, int eflags) erm->ermActive = false; ItemPointerSetInvalid(&(erm->curCtid)); erm->ermExtra = NULL; + estate->es_rowMarks = lappend(estate->es_rowMarks, erm); } diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c index b75062f74c..650fd81ff1 100644 --- a/src/backend/executor/execUtils.c +++ b/src/backend/executor/execUtils.c @@ -899,66 +899,6 @@ ShutdownExprContext(ExprContext *econtext, bool isCommit) MemoryContextSwitchTo(oldcontext); } -/* - * ExecLockNonLeafAppendTables - * - * Locks, if necessary, the tables indicated by the RT indexes contained in - * the partitioned_rels list. These are the non-leaf tables in the partition - * tree controlled by a given Append or MergeAppend node. - */ -void -ExecLockNonLeafAppendTables(List *partitioned_rels, EState *estate) -{ - PlannedStmt *stmt = estate->es_plannedstmt; - ListCell *lc; - - foreach(lc, partitioned_rels) - { - ListCell *l; - Index rti = lfirst_int(lc); - bool is_result_rel = false; - Oid relid = exec_rt_fetch(rti, estate)->relid; - - /* If this is a result relation, already locked in InitPlan */ - foreach(l, stmt->nonleafResultRelations) - { - if (rti == lfirst_int(l)) - { - is_result_rel = true; - break; - } - } - - /* - * Not a result relation; check if there is a RowMark that requires - * taking a RowShareLock on this rel. - */ - if (!is_result_rel) - { - PlanRowMark *rc = NULL; - LOCKMODE lockmode; - - foreach(l, stmt->rowMarks) - { - if (((PlanRowMark *) lfirst(l))->rti == rti) - { - rc = lfirst(l); - break; - } - } - - if (rc && RowMarkRequiresRowShareLock(rc->markType)) - lockmode = RowShareLock; - else - lockmode = AccessShareLock; - - Assert(lockmode == exec_rt_fetch(rti, estate)->rellockmode); - - LockRelationOid(relid, lockmode); - } - } -} - /* * GetAttributeByName * GetAttributeByNum diff --git a/src/backend/executor/nodeAppend.c b/src/backend/executor/nodeAppend.c index d44befd44e..a16b6da474 100644 --- a/src/backend/executor/nodeAppend.c +++ b/src/backend/executor/nodeAppend.c @@ -112,12 +112,6 @@ ExecInitAppend(Append *node, EState *estate, int eflags) /* check for unsupported flags */ Assert(!(eflags & EXEC_FLAG_MARK)); - /* - * Lock the non-leaf tables in the partition tree controlled by this node. - * It's a no-op for non-partitioned parent tables. - */ - ExecLockNonLeafAppendTables(node->partitioned_rels, estate); - /* * create new AppendState for our append node */ diff --git a/src/backend/executor/nodeMergeAppend.c b/src/backend/executor/nodeMergeAppend.c index f8aa50f77e..fde369d622 100644 --- a/src/backend/executor/nodeMergeAppend.c +++ b/src/backend/executor/nodeMergeAppend.c @@ -75,12 +75,6 @@ ExecInitMergeAppend(MergeAppend *node, EState *estate, int eflags) /* check for unsupported flags */ Assert(!(eflags & (EXEC_FLAG_BACKWARD | EXEC_FLAG_MARK))); - /* - * Lock the non-leaf tables in the partition tree controlled by this node. - * It's a no-op for non-partitioned parent tables. - */ - ExecLockNonLeafAppendTables(node->partitioned_rels, estate); - /* * create new MergeAppendState for our node */ diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 830f42dfe3..c9ed1988c5 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -534,8 +534,6 @@ extern void UnregisterExprContextCallback(ExprContext *econtext, ExprContextCallbackFunction function, Datum arg); -extern void ExecLockNonLeafAppendTables(List *partitioned_rels, EState *estate); - extern Datum GetAttributeByName(HeapTupleHeader tuple, const char *attname, bool *isNull); extern Datum GetAttributeByNum(HeapTupleHeader tuple, AttrNumber attrno, -- 2.40.0