Improve representation of PlanRowMark.

This patch fixes two inadequacies of the PlanRowMark representation.

First, that the original LockingClauseStrength isn't stored (and cannot be
inferred for foreign tables, which always get ROW_MARK_COPY).  Since some
PlanRowMarks are created out of whole cloth and don't actually have an
ancestral RowMarkClause, this requires adding a dummy LCS_NONE value to
enum LockingClauseStrength, which is fairly annoying but the alternatives
seem worse.  This fix allows getting rid of the use of get_parse_rowmark()
in FDWs (as per the discussion around commits 462bd95705a0c23b and
8ec8760fc87ecde0), and it simplifies some things elsewhere.

Second, that the representation assumed that all child tables in an
inheritance hierarchy would use the same RowMarkType.  That's true today
but will soon not be true.  We add an "allMarkTypes" field that identifies
the union of mark types used in all a parent table's children, and use
that where appropriate (currently, only in preprocess_targetlist()).

In passing fix a couple of minor infelicities left over from the SKIP
LOCKED patch, notably that _outPlanRowMark still thought waitPolicy
is a bool.

Catversion bump is required because the numeric values of enum
LockingClauseStrength can appear in on-disk rules.

Extracted from a much larger patch to support foreign table inheritance;
it seemed worth breaking this out, since it's a separable concern.

Shigeru Hanada and Etsuro Fujita, somewhat modified by me

4 files changed, 47 insertions, 52 deletions

diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 88b91f1b205..05687a48c9a 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -2219,35 +2219,42 @@ preprocess_rowmarks(PlannerInfo *root)
 		if (rte->rtekind != RTE_RELATION)
 			continue;
 
-		/*
-		 * Similarly, ignore RowMarkClauses for foreign tables; foreign tables
-		 * will instead get ROW_MARK_COPY items in the next loop.  (FDWs might
-		 * choose to do something special while fetching their rows, but that
-		 * is of no concern here.)
-		 */
-		if (rte->relkind == RELKIND_FOREIGN_TABLE)
-			continue;
-
 		rels = bms_del_member(rels, rc->rti);
 
 		newrc = makeNode(PlanRowMark);
 		newrc->rti = newrc->prti = rc->rti;
 		newrc->rowmarkId = ++(root->glob->lastRowMarkId);
-		switch (rc->strength)
+		if (rte->relkind == RELKIND_FOREIGN_TABLE)
 		{
-			case LCS_FORUPDATE:
-				newrc->markType = ROW_MARK_EXCLUSIVE;
-				break;
-			case LCS_FORNOKEYUPDATE:
-				newrc->markType = ROW_MARK_NOKEYEXCLUSIVE;
-				break;
-			case LCS_FORSHARE:
-				newrc->markType = ROW_MARK_SHARE;
-				break;
-			case LCS_FORKEYSHARE:
-				newrc->markType = ROW_MARK_KEYSHARE;
-				break;
+			/* For now, we force all foreign tables to use ROW_MARK_COPY */
+			newrc->markType = ROW_MARK_COPY;
+		}
+		else
+		{
+			/* regular table, apply the appropriate lock type */
+			switch (rc->strength)
+			{
+				case LCS_NONE:
+					/* we intentionally throw an error for LCS_NONE */
+					elog(ERROR, "unrecognized LockClauseStrength %d",
+						 (int) rc->strength);
+					break;
+				case LCS_FORKEYSHARE:
+					newrc->markType = ROW_MARK_KEYSHARE;
+					break;
+				case LCS_FORSHARE:
+					newrc->markType = ROW_MARK_SHARE;
+					break;
+				case LCS_FORNOKEYUPDATE:
+					newrc->markType = ROW_MARK_NOKEYEXCLUSIVE;
+					break;
+				case LCS_FORUPDATE:
+					newrc->markType = ROW_MARK_EXCLUSIVE;
+					break;
+			}
 		}
+		newrc->allMarkTypes = (1 << newrc->markType);
+		newrc->strength = rc->strength;
 		newrc->waitPolicy = rc->waitPolicy;
 		newrc->isParent = false;
 
@@ -2276,6 +2283,8 @@ preprocess_rowmarks(PlannerInfo *root)
 			newrc->markType = ROW_MARK_REFERENCE;
 		else
 			newrc->markType = ROW_MARK_COPY;
+		newrc->allMarkTypes = (1 << newrc->markType);
+		newrc->strength = LCS_NONE;
 		newrc->waitPolicy = LockWaitBlock;		/* doesn't matter */
 		newrc->isParent = false;
 
diff --git a/src/backend/optimizer/prep/prepsecurity.c b/src/backend/optimizer/prep/prepsecurity.c
index b382f13504e..0be44c1c2f7 100644
--- a/src/backend/optimizer/prep/prepsecurity.c
+++ b/src/backend/optimizer/prep/prepsecurity.c
@@ -73,8 +73,8 @@ expand_security_quals(PlannerInfo *root, List *tlist)
 	rt_index = 0;
 	foreach(cell, parse->rtable)
 	{
-		bool			targetRelation = false;
-		RangeTblEntry  *rte = (RangeTblEntry *) lfirst(cell);
+		bool		targetRelation = false;
+		RangeTblEntry *rte = (RangeTblEntry *) lfirst(cell);
 
 		rt_index++;
 
@@ -241,30 +241,10 @@ expand_security_qual(PlannerInfo *root, List *tlist, int rt_index,
 			rc = get_plan_rowmark(root->rowMarks, rt_index);
 			if (rc != NULL)
 			{
-				switch (rc->markType)
-				{
-					case ROW_MARK_EXCLUSIVE:
-						applyLockingClause(subquery, 1, LCS_FORUPDATE,
-										   rc->waitPolicy, false);
-						break;
-					case ROW_MARK_NOKEYEXCLUSIVE:
-						applyLockingClause(subquery, 1, LCS_FORNOKEYUPDATE,
-										   rc->waitPolicy, false);
-						break;
-					case ROW_MARK_SHARE:
-						applyLockingClause(subquery, 1, LCS_FORSHARE,
-										   rc->waitPolicy, false);
-						break;
-					case ROW_MARK_KEYSHARE:
-						applyLockingClause(subquery, 1, LCS_FORKEYSHARE,
-										   rc->waitPolicy, false);
-						break;
-					case ROW_MARK_REFERENCE:
-					case ROW_MARK_COPY:
-						/* No locking needed */
-						break;
-				}
-				root->rowMarks = list_delete(root->rowMarks, rc);
+				if (rc->strength != LCS_NONE)
+					applyLockingClause(subquery, 1, rc->strength,
+									   rc->waitPolicy, false);
+				root->rowMarks = list_delete_ptr(root->rowMarks, rc);
 			}
 
 			/*
@@ -276,6 +256,7 @@ expand_security_qual(PlannerInfo *root, List *tlist, int rt_index,
 			if (targetRelation)
 				applyLockingClause(subquery, 1, LCS_FORUPDATE,
 								   LockWaitBlock, false);
+
 			/*
 			 * Replace any variables in the outer query that refer to the
 			 * original relation RTE with references to columns that we will
diff --git a/src/backend/optimizer/prep/preptlist.c b/src/backend/optimizer/prep/preptlist.c
index 8a6c3cc5e5c..08e7c446d88 100644
--- a/src/backend/optimizer/prep/preptlist.c
+++ b/src/backend/optimizer/prep/preptlist.c
@@ -92,9 +92,9 @@ preprocess_targetlist(PlannerInfo *root, List *tlist)
 		if (rc->rti != rc->prti)
 			continue;
 
-		if (rc->markType != ROW_MARK_COPY)
+		if (rc->allMarkTypes & ~(1 << ROW_MARK_COPY))
 		{
-			/* It's a regular table, so fetch its TID */
+			/* Need to fetch TID */
 			var = makeVar(rc->rti,
 						  SelfItemPointerAttributeNumber,
 						  TIDOID,
@@ -125,9 +125,9 @@ preprocess_targetlist(PlannerInfo *root, List *tlist)
 				tlist = lappend(tlist, tle);
 			}
 		}
-		else
+		if (rc->allMarkTypes & (1 << ROW_MARK_COPY))
 		{
-			/* Not a table, so we need the whole row as a junk var */
+			/* Need the whole row as a junk var */
 			var = makeWholeRowVar(rt_fetch(rc->rti, range_table),
 								  rc->rti,
 								  0,
diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c
index b90fee387b4..cd40afce767 100644
--- a/src/backend/optimizer/prep/prepunion.c
+++ b/src/backend/optimizer/prep/prepunion.c
@@ -1389,9 +1389,14 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti)
 			newrc->prti = rti;
 			newrc->rowmarkId = oldrc->rowmarkId;
 			newrc->markType = oldrc->markType;
+			newrc->allMarkTypes = (1 << newrc->markType);
+			newrc->strength = oldrc->strength;
 			newrc->waitPolicy = oldrc->waitPolicy;
 			newrc->isParent = false;
 
+			/* Include child's rowmark type in parent's allMarkTypes */
+			oldrc->allMarkTypes |= newrc->allMarkTypes;
+
 			root->rowMarks = lappend(root->rowMarks, newrc);
 		}