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

diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 71b720eec9e0934a5fb9934a56fcbd4d3fdf7432..23e6749920a1bb96e8e6599c7aeb0158e62c84fb 100644 (file)
--- 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 b75062f74c4446050992de50f150fcc126d1d9fa..650fd81ff1729898d3fd9608bc7bbdb023c09b66 100644 (file)
--- 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 d44befd44e35f7a49bd32794c482f7236757bd04..a16b6da47404608bd372fd04c23f7ef37acb8360 100644 (file)
--- 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 f8aa50f77e778776ac30b28dbd43d4777328e920..fde369d62203d54e1129e1ade89c34f374157fc5 100644 (file)
--- 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 830f42dfe3c1fe41fa7f543439a8fab492d5cb96..c9ed1988c5fb0759810586a7845bfe8be32dc616 100644 (file)
--- 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,