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

@@ -828,10 +828,7 @@ InitPlan(QueryDesc *queryDesc, int eflags)
 	estate->es_plannedstmt = plannedstmt;
 
 	/*
-	 * initialize result relation stuff, and open/lock the result rels.
+	 * Initialize ResultRelInfo data structures, and open 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.
 	 */
 	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
+		 * In the partitioned result relation case, also build ResultRelInfos
-		 * relations too.  A subset of these are the roots of respective
+		 * for all the partitioned table roots, because we will need them to
-		 * partitioned tables, for which we also allocate ResultRelInfos.
+		 * fire statement-level triggers, if any.
 		 */
-		estate->es_root_result_relations = NULL;
+		if (plannedstmt->rootResultRelations)
-		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 *)
 				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. */
+		else
-#ifdef USE_ASSERT_CHECKING
+		{
-			foreach(l, plannedstmt->nonleafResultRelations)
+			estate->es_root_result_relations = NULL;
-			{
+			estate->es_num_root_result_relations = 0;
-				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
@@ -933,13 +909,7 @@ InitPlan(QueryDesc *queryDesc, int eflags)
 	}
 
 	/*
-	 * Similarly, we have to lock relations selected FOR [KEY] UPDATE/SHARE
+	 * Next, build the ExecRowMark list from the PlanRowMark(s), if any.
-	 * 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)
@@ -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);
 	}
 

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

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
 	 */

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
 	 */

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,