v12-0003-Move-result-rel-and-ExecRowMark-initilization-to.patch

From fadc610644cfa352485e2ee8134640b7954f4cef Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Thu, 19 Jul 2018 16:14:51 +0900
Subject: [PATCH v12 3/5] Move result rel and ExecRowMark initilization to
 ExecInitNode

InitPlan would do certain initiaization steps, such as creating
result rels, ExecRowMarks, etc. only because of the concerns about
locking order, which it needs not to do anymore, because we've
eliminated executor locking at all.  Instead create result rels and
ExecRowMarks, etc. in the ExecInit* routines of the respective nodes.
---
 src/backend/executor/execMain.c        | 238 +++++++++++----------------------
 src/backend/executor/nodeLockRows.c    |   4 +-
 src/backend/executor/nodeModifyTable.c |  27 +++-
 src/include/executor/executor.h        |   2 +-
 4 files changed, 103 insertions(+), 168 deletions(-)

diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index d5227ae129..0e77c3b66f 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -836,98 +836,44 @@ 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.
+	 * Allocate memory required for result relations.  ResultRelInfos
+	 * themselves are initialized during ExecInitModifyTable of respective
+	 * ModifyTable nodes.
 	 */
 	if (plannedstmt->resultRelations)
 	{
-		List	   *resultRelations = plannedstmt->resultRelations;
-		int			numResultRelations = list_length(resultRelations);
-		ResultRelInfo *resultRelInfos;
-		ResultRelInfo *resultRelInfo;
+		int		numResultRelations = list_length(plannedstmt->resultRelations);
+		int		num_roots = list_length(plannedstmt->rootResultRelations);
 
-		resultRelInfos = (ResultRelInfo *)
-			palloc(numResultRelations * sizeof(ResultRelInfo));
-		resultRelInfo = resultRelInfos;
-		foreach(l, resultRelations)
-		{
-			Index		resultRelationIndex = lfirst_int(l);
-			Relation	resultRelation;
-
-			Assert(rt_fetch(resultRelationIndex, rangeTable)->rellockmode == RowExclusiveLock);
-			resultRelation = ExecRangeTableRelation(estate, resultRelationIndex);
-			InitResultRelInfo(resultRelInfo,
-							  resultRelation,
-							  resultRelationIndex,
-							  NULL,
-							  estate->es_instrument);
-			resultRelInfo++;
-		}
-		estate->es_result_relations = resultRelInfos;
+		estate->es_result_relations = (ResultRelInfo *)
+				palloc(numResultRelations * sizeof(ResultRelInfo));
 		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.
-		 */
-		estate->es_root_result_relations = NULL;
-		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 *)
+		/* For partitioned tables that are roots of their partition trees. */
+		estate->es_root_result_relations = (ResultRelInfo *)
 				palloc(num_roots * sizeof(ResultRelInfo));
-			resultRelInfo = resultRelInfos;
-			foreach(l, plannedstmt->rootResultRelations)
-			{
-				Index		resultRelIndex = lfirst_int(l);
-				Relation	resultRelDesc;
+		estate->es_num_root_result_relations = num_roots;
 
-				Assert(rt_fetch(resultRelIndex, rangeTable)->rellockmode == RowExclusiveLock);
-				resultRelDesc = ExecRangeTableRelation(estate, resultRelIndex);
-				InitResultRelInfo(resultRelInfo,
-								  resultRelDesc,
-								  lfirst_int(l),
-								  NULL,
-								  estate->es_instrument);
-				resultRelInfo++;
-			}
-
-			estate->es_root_result_relations = resultRelInfos;
-			estate->es_num_root_result_relations = num_roots;
-
-			/* Simply check the rest of them are locked. */
+		/*
+		 * Since non-leaf tables of partition trees won't be processed by
+		 * ExecInitModifyTable, so check here that the appropriate lock is
+		 * held by upstream.
+		 */
 #ifdef USE_ASSERT_CHECKING
-			foreach(l, plannedstmt->nonleafResultRelations)
-			{
-				Index		resultRelIndex = lfirst_int(l);
+		foreach(l, plannedstmt->nonleafResultRelations)
+		{
+			Index		resultRelIndex = lfirst_int(l);
+			Relation	resultRelDesc;
+			Oid			reloid = getrelid(resultRelIndex,
+										  estate->es_range_table_array);
 
-				/* We locked the roots above. */
-				if (!list_member_int(plannedstmt->rootResultRelations,
-									 resultRelIndex))
-				{
-					Relation	resultRelDesc;
-					Oid		reloid = getrelid(resultRelIndex,
-											  estate->es_range_table_array);
-
-					resultRelDesc = heap_open(reloid, NoLock);
-					Assert(CheckRelationLockedByMe(resultRelDesc, RowExclusiveLock, true));
-					heap_close(resultRelDesc, NoLock);
-				}
-			}
-#endif
+			resultRelDesc = heap_open(reloid, NoLock);
+			Assert(CheckRelationLockedByMe(resultRelDesc, RowExclusiveLock, true));
+			heap_close(resultRelDesc, NoLock);
 		}
+#endif
 	}
 	else
 	{
@@ -941,67 +887,7 @@ InitPlan(QueryDesc *queryDesc, int eflags)
 		estate->es_num_root_result_relations = 0;
 	}
 
-	/*
-	 * 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.)
-	 */
 	estate->es_rowMarks = NIL;
-	foreach(l, plannedstmt->rowMarks)
-	{
-		PlanRowMark *rc = (PlanRowMark *) lfirst(l);
-		Oid			relid;
-		Relation	relation;
-		ExecRowMark *erm;
-
-		/* ignore "parent" rowmarks; they are irrelevant at runtime */
-		if (rc->isParent)
-			continue;
-
-		/* get relation's OID (will produce InvalidOid if subquery) */
-		relid = getrelid(rc->rti, estate->es_range_table_array);
-
-		switch (rc->markType)
-		{
-			case ROW_MARK_EXCLUSIVE:
-			case ROW_MARK_NOKEYEXCLUSIVE:
-			case ROW_MARK_SHARE:
-			case ROW_MARK_KEYSHARE:
-			case ROW_MARK_REFERENCE:
-				relation = ExecRangeTableRelation(estate, rc->rti);
-				break;
-			case ROW_MARK_COPY:
-				/* no physical table access is required */
-				relation = NULL;
-				break;
-			default:
-				elog(ERROR, "unrecognized markType: %d", rc->markType);
-				relation = NULL;	/* keep compiler quiet */
-				break;
-		}
-
-		/* Check that relation is a legal target for marking */
-		if (relation)
-			CheckValidRowMarkRel(relation, rc->markType);
-
-		erm = (ExecRowMark *) palloc(sizeof(ExecRowMark));
-		erm->relation = relation;
-		erm->relid = relid;
-		erm->rti = rc->rti;
-		erm->prti = rc->prti;
-		erm->rowmarkId = rc->rowmarkId;
-		erm->markType = rc->markType;
-		erm->strength = rc->strength;
-		erm->waitPolicy = rc->waitPolicy;
-		erm->ermActive = false;
-		ItemPointerSetInvalid(&(erm->curCtid));
-		erm->ermExtra = NULL;
-		estate->es_rowMarks = lappend(estate->es_rowMarks, erm);
-	}
 
 	/*
 	 * Initialize the executor's tuple table to empty.
@@ -1609,7 +1495,6 @@ static void
 ExecEndPlan(PlanState *planstate, EState *estate)
 {
 	int			num_relations = list_length(estate->es_range_table);
-	ResultRelInfo *resultRelInfo;
 	int			i;
 	ListCell   *l;
 
@@ -1643,17 +1528,6 @@ ExecEndPlan(PlanState *planstate, EState *estate)
 	 */
 	ExecResetTupleTable(estate->es_tupleTable, false);
 
-	/*
-	 * close indexes of result relation(s) if any
-	 */
-	resultRelInfo = estate->es_result_relations;
-	for (i = estate->es_num_result_relations; i > 0; i--)
-	{
-		/* Close indices; the relation itself already closed above */
-		ExecCloseIndices(resultRelInfo);
-		resultRelInfo++;
-	}
-
 	/* likewise close any trigger target relations */
 	ExecCleanUpTriggerState(estate);
 }
@@ -2409,25 +2283,63 @@ ExecUpdateLockMode(EState *estate, ResultRelInfo *relinfo)
 }
 
 /*
- * ExecFindRowMark -- find the ExecRowMark struct for given rangetable index
+ * ExecBuildRowMark -- create and return an ExecRowMark struct for the given
+ * PlanRowMark.
  *
- * If no such struct, either return NULL or throw error depending on missing_ok
+ * The created ExecRowMark is also added to the list of rowmarks in the given
+ * EState.
  */
 ExecRowMark *
-ExecFindRowMark(EState *estate, Index rti, bool missing_ok)
+ExecBuildRowMark(EState *estate, PlanRowMark *rc)
 {
-	ListCell   *lc;
+	Oid			relid;
+	ExecRowMark *erm;
+	Relation	relation;
 
-	foreach(lc, estate->es_rowMarks)
+	Assert(rc != NULL && rc->rti > 0);
+	Assert(!rc->isParent);
+	relid = getrelid(rc->rti, estate->es_range_table_array);
+
+	/* get relation's OID (will produce InvalidOid if subquery) */
+
+	switch (rc->markType)
 	{
-		ExecRowMark *erm = (ExecRowMark *) lfirst(lc);
-
-		if (erm->rti == rti)
-			return erm;
+		case ROW_MARK_EXCLUSIVE:
+		case ROW_MARK_NOKEYEXCLUSIVE:
+		case ROW_MARK_SHARE:
+		case ROW_MARK_KEYSHARE:
+		case ROW_MARK_REFERENCE:
+			relation = ExecRangeTableRelation(estate, rc->rti);
+			break;
+		case ROW_MARK_COPY:
+			/* no physical table access is required */
+			relation = NULL;
+			break;
+		default:
+			elog(ERROR, "unrecognized markType: %d", rc->markType);
+			relation = NULL;	/* keep compiler quiet */
+			break;
 	}
-	if (!missing_ok)
-		elog(ERROR, "failed to find ExecRowMark for rangetable index %u", rti);
-	return NULL;
+
+	/* Check that relation is a legal target for marking */
+	if (relation)
+		CheckValidRowMarkRel(relation, rc->markType);
+
+	erm = (ExecRowMark *) palloc(sizeof(ExecRowMark));
+	erm->relation = relation;
+	erm->relid = relid;
+	erm->rti = rc->rti;
+	erm->prti = rc->prti;
+	erm->rowmarkId = rc->rowmarkId;
+	erm->markType = rc->markType;
+	erm->strength = rc->strength;
+	erm->waitPolicy = rc->waitPolicy;
+	erm->ermActive = false;
+	ItemPointerSetInvalid(&(erm->curCtid));
+	erm->ermExtra = NULL;
+	estate->es_rowMarks = lappend(estate->es_rowMarks, erm);
+
+	return erm;
 }
 
 /*
diff --git a/src/backend/executor/nodeLockRows.c b/src/backend/executor/nodeLockRows.c
index 6db345ae7a..ff9606342b 100644
--- a/src/backend/executor/nodeLockRows.c
+++ b/src/backend/executor/nodeLockRows.c
@@ -424,8 +424,8 @@ ExecInitLockRows(LockRows *node, EState *estate, int eflags)
 		/* safety check on size of lr_curtuples array */
 		Assert(rc->rti > 0 && rc->rti <= lrstate->lr_ntables);
 
-		/* find ExecRowMark and build ExecAuxRowMark */
-		erm = ExecFindRowMark(estate, rc->rti, false);
+		/* build the ExecRowMark and ExecAuxRowMark */
+		erm = ExecBuildRowMark(estate, rc);
 		aerm = ExecBuildAuxRowMark(erm, outerPlan->targetlist);
 
 		/*
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 07ac3de5e0..9b584097d7 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -2242,12 +2242,33 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
 
 	mtstate->mt_plans = (PlanState **) palloc0(sizeof(PlanState *) * nplans);
 	mtstate->resultRelInfo = estate->es_result_relations + node->resultRelIndex;
+	resultRelInfo = mtstate->resultRelInfo;
+	foreach(l, node->resultRelations)
+	{
+		Index		resultRelationIndex = lfirst_int(l);
+		Relation	rel;
+
+		rel = ExecRangeTableRelation(estate, resultRelationIndex);
+		InitResultRelInfo(resultRelInfo, rel, resultRelationIndex, NULL,
+						  estate->es_instrument);
+		resultRelInfo++;
+	}
 
 	/* If modifying a partitioned table, initialize the root table info */
 	if (node->rootResultRelIndex >= 0)
+	{
+		Index			root_rt_index = linitial_int(node->partitioned_rels);
+		Relation		rel;
+
 		mtstate->rootResultRelInfo = estate->es_root_result_relations +
 			node->rootResultRelIndex;
 
+		Assert(root_rt_index > 0);
+		rel = ExecRangeTableRelation(estate, root_rt_index);
+		InitResultRelInfo(mtstate->rootResultRelInfo, rel, root_rt_index,
+						  NULL, estate->es_instrument);
+	}
+
 	mtstate->mt_arowmarks = (List **) palloc0(sizeof(List *) * nplans);
 	mtstate->mt_nplans = nplans;
 
@@ -2533,8 +2554,8 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
 		if (rc->isParent)
 			continue;
 
-		/* find ExecRowMark (same for all subplans) */
-		erm = ExecFindRowMark(estate, rc->rti, false);
+		/* build the ExecRowMark (same for all subplans) */
+		erm = ExecBuildRowMark(estate, rc);
 
 		/* build ExecAuxRowMark for each subplan */
 		for (i = 0; i < nplans; i++)
@@ -2700,6 +2721,8 @@ ExecEndModifyTable(ModifyTableState *node)
 			resultRelInfo->ri_FdwRoutine->EndForeignModify != NULL)
 			resultRelInfo->ri_FdwRoutine->EndForeignModify(node->ps.state,
 														   resultRelInfo);
+		/* closes indices */
+		ExecCloseIndices(resultRelInfo);
 	}
 
 	/* Close all the partitioned tables, leaf partitions, and their indices */
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index fdfd48d897..4c3a78b1a7 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -188,7 +188,7 @@ extern void ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo,
 extern void ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
 					 TupleTableSlot *slot, EState *estate);
 extern LockTupleMode ExecUpdateLockMode(EState *estate, ResultRelInfo *relinfo);
-extern ExecRowMark *ExecFindRowMark(EState *estate, Index rti, bool missing_ok);
+extern ExecRowMark *ExecBuildRowMark(EState *estate, PlanRowMark *rc);
 extern ExecAuxRowMark *ExecBuildAuxRowMark(ExecRowMark *erm, List *targetlist);
 extern TupleTableSlot *EvalPlanQual(EState *estate, EPQState *epqstate,
 			 Relation relation, Index rti, int lockmode,
-- 
2.11.0