Restore sane locking behavior during parallel query.

Commit 9a3cebeaa changed things so that parallel workers didn't obtain
any lock of their own on tables they access.  That was clearly a bad
idea, but I'd mistakenly supposed that it was the intended end result
of the series of patches for simplifying the executor's lock management.
Undo that change in relation_open(), and adjust ExecOpenScanRelation()
so that it gets the correct lock if inside a parallel worker.

In passing, clean up some more obsolete comments about when locks
are acquired.

Discussion: https://postgr.es/m/468c85d9-540e-66a2-1dde-fec2b741e688@lab.ntt.co.jp

1 files changed, 23 insertions, 9 deletions

diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index 650fd81ff1..d98e90e711 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -732,16 +732,30 @@ ExecGetRangeTableRelation(EState *estate, Index rti)
 
 		Assert(rte->rtekind == RTE_RELATION);
 
-		rel = estate->es_relations[rti - 1] = heap_open(rte->relid, NoLock);
+		if (!IsParallelWorker())
+		{
+			/*
+			 * In a normal query, we should already have the appropriate lock,
+			 * but verify that through an Assert.  Since there's already an
+			 * Assert inside heap_open that insists on holding some lock, it
+			 * seems sufficient to check this only when rellockmode is higher
+			 * than the minimum.
+			 */
+			rel = heap_open(rte->relid, NoLock);
+			Assert(rte->rellockmode == AccessShareLock ||
+				   CheckRelationLockedByMe(rel, rte->rellockmode, false));
+		}
+		else
+		{
+			/*
+			 * If we are a parallel worker, we need to obtain our own local
+			 * lock on the relation.  This ensures sane behavior in case the
+			 * parent process exits before we do.
+			 */
+			rel = heap_open(rte->relid, rte->rellockmode);
+		}
 
-		/*
-		 * Verify that appropriate lock was obtained before execution.
-		 *
-		 * In the case of parallel query, only the leader would've obtained
-		 * the lock (that needs to be fixed, though).
-		 */
-		Assert(IsParallelWorker() ||
-			   CheckRelationLockedByMe(rel, rte->rellockmode, false));
+		estate->es_relations[rti - 1] = rel;
 	}
 
 	return rel;