misc_fixes.patch.txt

text/plain

Filename: misc_fixes.patch.txt
Type: text/plain
Part: 0
Message: Re: generic plans and "initial" pruning
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 05cc99df8f..5ee978937d 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -121,6 +121,8 @@ static void EvalPlanQualStart(EPQState *epqstate, Plan *planTree);
  *
  * Note: Partitioned tables mentioned in PartitionedRelPruneInfo nodes that
  * drive the pruning will be locked before doing the pruning.
+ *
+ * ----------------------------------------------------------------
  */
 PartitionPruneResult *
 ExecutorDoInitialPruning(PlannedStmt *plannedstmt, ParamListInfo params)
diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c
index 3037742b8d..e9ca6bc55f 100644
--- a/src/backend/executor/execPartition.c
+++ b/src/backend/executor/execPartition.c
@@ -1707,6 +1707,7 @@ ExecInitPartitionPruning(PlanState *planstate,
 		Assert(n_total_subplans > 0);
 		*initially_valid_subplans = bms_add_range(NULL, 0,
 												  n_total_subplans - 1);
+		return prunestate;
 	}
 
 	/*
@@ -1714,14 +1715,15 @@ ExecInitPartitionPruning(PlanState *planstate,
 	 * that were removed above due to initial pruning.  No need to do this if
 	 * no steps were removed.
 	 */
-	if (bms_num_members(*initially_valid_subplans) < n_total_subplans)
+	if (prunestate &&
+		bms_num_members(*initially_valid_subplans) < n_total_subplans)
 	{
 		/*
 		 * We can safely skip this when !do_exec_prune, even though that
 		 * leaves invalid data in prunestate, because that data won't be
 		 * consulted again (cf initial Assert in ExecFindMatchingSubPlans).
 		 */
-		if (prunestate && prunestate->do_exec_prune)
+		if (prunestate->do_exec_prune)
 			PartitionPruneFixSubPlanMap(prunestate,
 										*initially_valid_subplans,
 										n_total_subplans);
@@ -1751,7 +1753,8 @@ ExecPartitionDoInitialPruning(PlannedStmt *plannedstmt, ParamListInfo params,
 	Bitmapset	 *valid_subplan_offs;
 
 	/*
-	 * A temporary context to allocate stuff needded to run the pruning steps.
+	 * A temporary context to for memory allocations required while execution
+	 * partition pruning steps.
 	 */
 	tmpcontext = AllocSetContextCreate(CurrentMemoryContext,
 									   "initial pruning working data",
@@ -1765,11 +1768,12 @@ ExecPartitionDoInitialPruning(PlannedStmt *plannedstmt, ParamListInfo params,
 	pdir = CreatePartitionDirectory(CurrentMemoryContext, false);
 
 	/*
-	 * We don't yet have a PlanState for the parent plan node, so must create
-	 * a standalone ExprContext to evaluate pruning expressions, equipped with
-	 * the information about the EXTERN parameters that the caller passed us.
-	 * Note that that's okay because the initial pruning steps do not contain
-	 * anything that requires the execution to have started.
+	 * We don't yet have a PlanState for the parent plan node, so we must
+	 * create a standalone ExprContext to evaluate pruning expressions,
+	 * equipped with the information about the EXTERN parameters that the
+	 * caller passed us.  Note that that's okay because the initial pruning
+	 * steps do not contain anything that requires the execution to have
+	 * started.
 	 */
 	econtext = CreateStandaloneExprContext();
 	econtext->ecxt_param_list_info = params;
@@ -1874,7 +1878,6 @@ CreatePartitionPruneState(PlanState *planstate,
 			PartitionedRelPruneInfo *pinfo = lfirst_node(PartitionedRelPruneInfo, lc2);
 			PartitionedRelPruningData *pprune = &prunedata->partrelprunedata[j];
 			Relation	partrel;
-			bool		close_partrel = false;
 			PartitionDesc partdesc;
 			PartitionKey partkey;
 
@@ -1894,7 +1897,6 @@ CreatePartitionPruneState(PlanState *planstate,
 				int		lockmode = (j == 0) ? NoLock : rte->rellockmode;
 
 				partrel = table_open(rte->relid, lockmode);
-				close_partrel = true;
 			}
 			else
 				partrel = ExecGetRangeTableRelation(estate, pinfo->rtindex);
@@ -1914,7 +1916,7 @@ CreatePartitionPruneState(PlanState *planstate,
 			 * Must close partrel, keeping the lock taken, if we're not using
 			 * EState's entry.
 			 */
-			if (close_partrel)
+			if (estate == NULL)
 				table_close(partrel, NoLock);
 
 			/*
@@ -2367,6 +2369,8 @@ find_matching_subplans_recurse(PartitionPruningData *prunedata,
 		{
 			*validsubplans = bms_add_member(*validsubplans,
 											pprune->subplan_map[i]);
+			/* XXX why would pprune->rti_map[i] ever be zero here??? */
+			Assert(pprune->rti_map[i] > 0);
 			if (scan_leafpart_rtis && pprune->rti_map[i] > 0)
 				*scan_leafpart_rtis = bms_add_member(*scan_leafpart_rtis,
 													 pprune->rti_map[i]);
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index 639145abe9..f9c7976ff2 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -119,6 +119,7 @@ CreateExecutorState(void)
 	estate->es_relations = NULL;
 	estate->es_rowmarks = NULL;
 	estate->es_plannedstmt = NULL;
+	estate->es_part_prune_infos = NIL;
 	estate->es_part_prune_result = NULL;
 
 	estate->es_junkFilter = NULL;
diff --git a/src/backend/executor/nodeAppend.c b/src/backend/executor/nodeAppend.c
index 09f26658e2..96880e122a 100644
--- a/src/backend/executor/nodeAppend.c
+++ b/src/backend/executor/nodeAppend.c
@@ -94,7 +94,6 @@ static bool ExecAppendAsyncRequest(AppendState *node, TupleTableSlot **result);
 static void ExecAppendAsyncEventWait(AppendState *node);
 static void classify_matching_subplans(AppendState *node);
 
-
 /* ----------------------------------------------------------------
  *		ExecInitAppend
  *
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index ec6b1f1fc0..fe0df2f1d1 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -1184,7 +1184,6 @@ create_append_plan(PlannerInfo *root, AppendPath *best_path, int flags)
 	ListCell   *subpaths;
 	int			nasyncplans = 0;
 	RelOptInfo *rel = best_path->path.parent;
-	int			part_prune_index = -1;
 	int			nodenumsortkeys = 0;
 	AttrNumber *nodeSortColIdx = NULL;
 	Oid		   *nodeSortOperators = NULL;
@@ -1335,6 +1334,9 @@ create_append_plan(PlannerInfo *root, AppendPath *best_path, int flags)
 		subplans = lappend(subplans, subplan);
 	}
 
+	/* Set below if we find quals that we can use to run-time prune */
+	plan->part_prune_index = -1;
+
 	/*
 	 * If any quals exist, they may be useful to perform further partition
 	 * pruning during execution.  Gather information needed by the executor to
@@ -1358,18 +1360,15 @@ create_append_plan(PlannerInfo *root, AppendPath *best_path, int flags)
 		}
 
 		if (prunequal != NIL)
-			part_prune_index= make_partition_pruneinfo(root, rel,
-													   best_path->subpaths,
-													   prunequal);
+			plan->part_prune_index = make_partition_pruneinfo(root, rel,
+															  best_path->subpaths,
+															  prunequal);
 	}
 
 	plan->appendplans = subplans;
 	plan->nasyncplans = nasyncplans;
 	plan->first_partial_plan = best_path->first_partial_path;
 
-	/* Will be updated later in set_plan_references(). */
-	plan->part_prune_index = part_prune_index;
-
 	copy_generic_path_info(&plan->plan, (Path *) best_path);
 
 	/*
@@ -1408,7 +1407,6 @@ create_merge_append_plan(PlannerInfo *root, MergeAppendPath *best_path,
 	List	   *subplans = NIL;
 	ListCell   *subpaths;
 	RelOptInfo *rel = best_path->path.parent;
-	int			part_prune_index = -1;
 
 	/*
 	 * We don't have the actual creation of the MergeAppend node split out
@@ -1501,6 +1499,9 @@ create_merge_append_plan(PlannerInfo *root, MergeAppendPath *best_path,
 		subplans = lappend(subplans, subplan);
 	}
 
+	/* Set below if we find quals that we can use to run-time prune */
+	node->part_prune_index = -1;
+
 	/*
 	 * If any quals exist, they may be useful to perform further partition
 	 * pruning during execution.  Gather information needed by the executor to
@@ -1524,15 +1525,13 @@ create_merge_append_plan(PlannerInfo *root, MergeAppendPath *best_path,
 		}
 
 		if (prunequal != NIL)
-			part_prune_index= make_partition_pruneinfo(root, rel,
-													   best_path->subpaths,
-													   prunequal);
+			node->part_prune_index = make_partition_pruneinfo(root, rel,
+															  best_path->subpaths,
+															  prunequal);
 	}
 
 	node->mergeplans = subplans;
 
-	/* Will be updated later in set_plan_references(). */
-	node->part_prune_index = part_prune_index;
 
 	/*
 	 * If prepare_sort_from_pathkeys added sort columns, but we were told to
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index c88e5bacac..63ec8a98fc 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -408,6 +408,13 @@ set_plan_references(PlannerInfo *root, Plan *plan)
 		glob->partPruneInfos = lappend(glob->partPruneInfos, pruneinfo);
 	}
 
+	/*
+	 * XXX is it worth doing a bms_copy() on glob->minLockRelids if
+	 * glob->containsInitialPruning is true?. I'm slighly worried that the
+	 * Bitmapset could have a very long empty tail resulting in excessive
+	 * looping during AcquireExecutorLocks().
+	 */
+
 	return result;
 }
 
diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.c
index 5a5f5dee46..952c5b8327 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -212,12 +212,12 @@ static void partkey_datum_from_expr(PartitionPruneContext *context,
 /*
  * make_partition_pruneinfo
  *		Checks if the given set of quals can be used to build pruning steps
- *		that the executor will use to prune useless ones from given set of
- *		child paths, and if so builds a PartitionPruneInfo that will allow the
- *		executor to do do and append it to root->partPruneInfos.
+ *		that the executor can use to prune away unneeded partitions.  If
+ *		suitable quals are found then a PartitionPruneInfo is built and tagged
+ *		onto the PlannerInfo's partPruneInfos list.
  *
- * Return value is 0-based index of the added PartitionPruneInfo or -1 if one
- * was not built after all.
+ * The return value is the 0-based index of the item added to the
+ * partPruneInfos list or -1 if nothing was added.
  *
  * 'parentrel' is the RelOptInfo for an appendrel, and 'subpaths' is the list
  * of scan paths for its child rels.
@@ -335,10 +335,9 @@ make_partition_pruneinfo(PlannerInfo *root, RelOptInfo *parentrel,
 			allmatchedsubplans = bms_join(matchedsubplans,
 										  allmatchedsubplans);
 		}
-		if (!needs_init_pruning)
-			needs_init_pruning = partrel_needs_init_pruning;
-		if (!needs_exec_pruning)
-			needs_exec_pruning = partrel_needs_exec_pruning;
+
+		needs_init_pruning |= partrel_needs_init_pruning;
+		needs_exec_pruning |= partrel_needs_exec_pruning;
 	}
 
 	pfree(relid_subplan_map);
@@ -570,7 +569,7 @@ make_partitionedrel_pruneinfo(PlannerInfo *root, RelOptInfo *parentrel,
 		 * that would require per-scan pruning.
 		 *
 		 * In the first pass, we note whether the 2nd pass is necessary by
-		 * by noting the presence of EXEC parameters.
+		 * noting the presence of EXEC parameters.
 		 */
 		gen_partprune_steps(subpart, partprunequal, PARTTARGET_INITIAL,
 							&context);
@@ -645,10 +644,11 @@ make_partitionedrel_pruneinfo(PlannerInfo *root, RelOptInfo *parentrel,
 		pinfo->execparamids = execparamids;
 		/* Remaining fields will be filled in the next loop */
 
-		if (!*needs_init_pruning)
-			*needs_init_pruning = (initial_pruning_steps != NIL);
-		if (!*needs_exec_pruning)
-			*needs_exec_pruning = (exec_pruning_steps != NIL);
+		/* record which types of pruning steps we've seen so far */
+		if (initial_pruning_steps != NIL)
+			*needs_init_pruning = true;
+		if (exec_pruning_steps != NIL)
+			*needs_exec_pruning = true;
 
 		pinfolist = lappend(pinfolist, pinfo);
 	}
diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c
index a627448a5a..8cc2e2162d 100644
--- a/src/backend/tcop/pquery.c
+++ b/src/backend/tcop/pquery.c
@@ -1204,7 +1204,6 @@ PortalRunMulti(Portal portal,
 {
 	bool		active_snapshot_set = false;
 	ListCell   *stmtlist_item;
-	int			i;
 
 	/*
 	 * If the destination is DestRemoteExecute, change to DestNone.  The
@@ -1225,15 +1224,9 @@ PortalRunMulti(Portal portal,
 	 * Loop to handle the individual queries generated from a single parsetree
 	 * by analysis and rewrite.
 	 */
-	i = 0;
 	foreach(stmtlist_item, portal->stmts)
 	{
 		PlannedStmt *pstmt = lfirst_node(PlannedStmt, stmtlist_item);
-		PartitionPruneResult *part_prune_result = portal->part_prune_results ?
-									  list_nth(portal->part_prune_results, i) :
-									  NULL;
-
-		i++;
 
 		/*
 		 * If we got a cancel signal in prior command, quit
@@ -1242,6 +1235,8 @@ PortalRunMulti(Portal portal,
 
 		if (pstmt->utilityStmt == NULL)
 		{
+			PartitionPruneResult *part_prune_result = NULL;
+
 			/*
 			 * process a plannable query.
 			 */
@@ -1288,6 +1283,14 @@ PortalRunMulti(Portal portal,
 			else
 				UpdateActiveSnapshotCommandId();
 
+			/*
+			 * Determine if there's a corresponding PartitionPruneResult for
+			 * this PlannedStmt.
+			 */
+			if (portal->part_prune_results != NIL)
+				part_prune_result = list_nth(portal->part_prune_results,
+											 foreach_current_index(stmtlist_item));
+
 			if (pstmt->canSetTag)
 			{
 				/* statement can set tag string */
diff --git a/src/include/commands/explain.h b/src/include/commands/explain.h
index 34975c69ee..bbc8c42d88 100644
--- a/src/include/commands/explain.h
+++ b/src/include/commands/explain.h
@@ -87,7 +87,8 @@ extern void ExplainOneUtility(Node *utilityStmt, IntoClause *into,
 							  ExplainState *es, const char *queryString,
 							  ParamListInfo params, QueryEnvironment *queryEnv);
 
-extern void ExplainOnePlan(PlannedStmt *plannedstmt, PartitionPruneResult *part_prune_resul,
+extern void ExplainOnePlan(PlannedStmt *plannedstmt,
+						   PartitionPruneResult *part_prune_result,
 						   IntoClause *into,
 						   ExplainState *es, const char *queryString,
 						   ParamListInfo params, QueryEnvironment *queryEnv,
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 43bd293433..a8bf908d63 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1000,11 +1000,11 @@ typedef TupleTableSlot *(*ExecProcNodeMtd) (struct PlanState *pstate);
  *
  * This is used by GetCachedPlan() to inform its callers of the pruning
  * decisions made when performing AcquireExecutorLocks() on a given cached
- * PlannedStmt, which the callers then pass that on to the executor.  The
- * executor refers to this node when made available when initializing the plan
- * nodes to which those PartitionPruneInfos apply so that the same set of
- * qualifying subplans are initialized, rather than deriving that set again by
- * redoing initial pruning.
+ * PlannedStmt, which the callers then pass onto the executor.  The executor
+ * refers to this node when made available when initializing the plan nodes to
+ * which those PartitionPruneInfos apply so that the same set of qualifying
+ * subplans are initialized, rather than deriving that set again by redoing
+ * initial pruning.
  */
 typedef struct PartitionPruneResult
 {
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index 550308147d..f8f3971f44 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -274,11 +274,7 @@ typedef struct Append
 	 */
 	int			first_partial_plan;
 
-	/*
-	 * Index of this plan's PartitionPruneInfo in PlannedStmt.partPruneInfos
-	 * to be used for run-time subplan pruning; -1 if run-time pruning is
-	 * not needed.
-	 */
+	/* Index to PlannerInfo.partPruneInfos or -1 if no run-time pruning */
 	int			part_prune_index;
 } Append;
 
@@ -299,11 +295,7 @@ typedef struct MergeAppend
 	Oid		   *collations;		/* OIDs of collations */
 	bool	   *nullsFirst;		/* NULLS FIRST/LAST directions */
 
-	/*
-	 * Index of this plan's PartitionPruneInfo in PlannedStmt.partPruneInfos
-	 * to be used for run-time subplan pruning; -1 if run-time pruning is
-	 * not needed.
-	 */
+	/* Index to PlannerInfo.partPruneInfos or -1 if no run-time pruning */
 	int			part_prune_index;
 } MergeAppend;