From 18260f490a3d8355e09c18e216fc1ec743df1432 Mon Sep 17 00:00:00 2001
From: "dgrowley@gmail.com" <dgrowley@gmail.com>
Date: Mon, 9 Apr 2018 14:49:01 +1200
Subject: [PATCH 1/2] Fix incorrect logic for choosing the next Parallel Append
 subplan

In 499be013de support for pruning unneeded Append subnodes was added. The
logic in that commit was not correct and was not correctly checking if the
next subplan was in fact a valid subplan. This could cause parallel workers
processes to be given a subplan to work on which didn't require any work.

In passing also add a comment to explain what first_partial_plan means.
---
 src/backend/executor/nodeAppend.c | 54 ++++++++++++++++++++++++++++++---------
 src/include/nodes/plannodes.h     |  5 ++++
 2 files changed, 47 insertions(+), 12 deletions(-)

diff --git a/src/backend/executor/nodeAppend.c b/src/backend/executor/nodeAppend.c
index b135b61324..ddca1b2a2a 100644
--- a/src/backend/executor/nodeAppend.c
+++ b/src/backend/executor/nodeAppend.c
@@ -547,6 +547,11 @@ choose_next_subplan_for_leader(AppendState *node)
 			LWLockRelease(&pstate->pa_lock);
 			return false;
 		}
+
+		/*
+		 * We needn't pay attention to as_valid_subplans here as all invalid
+		 * plans have been marked as finished.
+		 */
 		node->as_whichplan--;
 	}
 
@@ -612,18 +617,32 @@ choose_next_subplan_for_worker(AppendState *node)
 	/* Save the plan from which we are starting the search. */
 	node->as_whichplan = pstate->pa_next_plan;
 
-	/* Loop until we find a subplan to execute. */
+	/* Loop until we find a valid subplan to execute. */
 	while (pstate->pa_finished[pstate->pa_next_plan])
 	{
-		if (pstate->pa_next_plan < node->as_nplans - 1)
+		int			nextplan = bms_next_member(node->as_valid_subplans,
+											   pstate->pa_next_plan);
+
+		if (nextplan >= 0)
 		{
-			/* Advance to next plan. */
-			pstate->pa_next_plan++;
+			/* Advance to the next valid plan. */
+			pstate->pa_next_plan = nextplan;
 		}
 		else if (node->as_whichplan > append->first_partial_plan)
 		{
-			/* Loop back to first partial plan. */
-			pstate->pa_next_plan = append->first_partial_plan;
+			/* Try looping back to first valid partial plan. */
+			nextplan = bms_next_member(node->as_valid_subplans,
+									   append->first_partial_plan - 1);
+
+			/*
+			 * There mightn't be any valid partial plans.  If there's none
+			 * then set pa_next_plan to as_whichplan so that we exit in the
+			 * test below, otherwise just select the first valid partial plan.
+			 */
+			if (nextplan < 0)
+				pstate->pa_next_plan = node->as_whichplan;
+			else
+				pstate->pa_next_plan = nextplan;
 		}
 		else
 		{
@@ -644,16 +663,27 @@ choose_next_subplan_for_worker(AppendState *node)
 	}
 
 	/* Pick the plan we found, and advance pa_next_plan one more time. */
-	node->as_whichplan = pstate->pa_next_plan++;
-	if (pstate->pa_next_plan >= node->as_nplans)
+	node->as_whichplan = pstate->pa_next_plan;
+	pstate->pa_next_plan = bms_next_member(node->as_valid_subplans,
+										   pstate->pa_next_plan);
+
+	/*
+	 * If there are no more valid plans then try setting the next plan to the
+	 * first valid partial plan.
+	 */
+	if (pstate->pa_next_plan < 0)
 	{
-		if (append->first_partial_plan < node->as_nplans)
-			pstate->pa_next_plan = append->first_partial_plan;
+		int			nextplan = bms_next_member(node->as_valid_subplans,
+											   append->first_partial_plan - 1);
+
+		if (nextplan >= 0)
+			pstate->pa_next_plan = nextplan;
 		else
 		{
 			/*
-			 * We have only non-partial plans, and we already chose the last
-			 * one; so arrange for the other workers to immediately bail out.
+			 * There are no valid partial plans, and we already chose the last
+			 * non-partial plan; so flag that there's nothing more for our
+			 * fellow workers to do.
 			 */
 			pstate->pa_next_plan = INVALID_SUBPLAN_INDEX;
 		}
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index c3e5c2c79f..c5c33cd336 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -255,6 +255,11 @@ typedef struct Append
 	/* RT indexes of non-leaf tables in a partition tree */
 	List	   *partitioned_rels;
 	List	   *appendplans;
+
+	/*
+	 * All 'appendplans' preceding this index are non-partial plans. All
+	 * 'appendplans' from this index onwards are partial plans.
+	 */
 	int			first_partial_plan;
 
 	/*
-- 
2.16.2.windows.1