Fix up run-time partition pruning's use of relcache's partition data.

The previous coding saved pointers into the partitioned table's relcache
entry, but then closed the relcache entry, causing those pointers to
nominally become dangling.  Actual trouble would be seen in the field
only if a relcache flush occurred mid-query, but that's hardly out of
the question.

While we could fix this by copying all the data in question at query
start, it seems better to just hold the relcache entry open for the
whole query.

While at it, improve the handling of support-function lookups: do that
once per query not once per pruning test.  There's still something to be
desired here, in that we fail to exploit the possibility of caching data
across queries in the fn_extra fields of the relcache's FmgrInfo structs,
which could happen if we just used those structs in-place rather than
copying them.  However, combining that with the possibility of per-query
lookups of cross-type comparison functions seems to require changes in the
APIs of a lot of the pruning support functions, so it's too invasive to
consider as part of this patch.  A win would ensue only for complex
partition key data types (e.g. arrays), so it may not be worth the
trouble.

David Rowley and Tom Lane

Discussion: https://postgr.es/m/17850.1528755844@sss.pgh.pa.us

2 files changed, 55 insertions, 22 deletions

diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c
index 33513ff1d15..4eeee7c5e7f 100644
--- a/src/backend/executor/execPartition.c
+++ b/src/backend/executor/execPartition.c
@@ -1357,11 +1357,14 @@ adjust_partition_tlist(List *tlist, TupleConversionMap *map)
  *
  * Functions:
  *
- * ExecSetupPartitionPruneState:
+ * ExecCreatePartitionPruneState:
  *		Creates the PartitionPruneState required by each of the two pruning
  *		functions.  Details stored include how to map the partition index
  *		returned by the partition pruning code into subplan indexes.
  *
+ * ExecDestroyPartitionPruneState:
+ *		Deletes a PartitionPruneState. Must be called during executor shutdown.
+ *
  * ExecFindInitialMatchingSubPlans:
  *		Returns indexes of matching subplans.  Partition pruning is attempted
  *		without any evaluation of expressions containing PARAM_EXEC Params.
@@ -1382,8 +1385,8 @@ adjust_partition_tlist(List *tlist, TupleConversionMap *map)
  */
 
 /*
- * ExecSetupPartitionPruneState
- *		Set up the data structure required for calling
+ * ExecCreatePartitionPruneState
+ *		Build the data structure required for calling
  *		ExecFindInitialMatchingSubPlans and ExecFindMatchingSubPlans.
  *
  * 'planstate' is the parent plan node's execution state.
@@ -1395,7 +1398,7 @@ adjust_partition_tlist(List *tlist, TupleConversionMap *map)
  * in each PartitionPruneInfo.
  */
 PartitionPruneState *
-ExecSetupPartitionPruneState(PlanState *planstate, List *partitionpruneinfo)
+ExecCreatePartitionPruneState(PlanState *planstate, List *partitionpruneinfo)
 {
 	PartitionPruneState *prunestate;
 	PartitionPruningData *prunedata;
@@ -1435,11 +1438,10 @@ ExecSetupPartitionPruneState(PlanState *planstate, List *partitionpruneinfo)
 		PartitionPruningData *pprune = &prunedata[i];
 		PartitionPruneContext *context = &pprune->context;
 		PartitionDesc partdesc;
-		Relation	rel;
 		PartitionKey partkey;
-		ListCell   *lc2;
 		int			partnatts;
 		int			n_steps;
+		ListCell   *lc2;
 
 		/*
 		 * We must copy the subplan_map rather than pointing directly to the
@@ -1456,26 +1458,33 @@ ExecSetupPartitionPruneState(PlanState *planstate, List *partitionpruneinfo)
 		pprune->present_parts = bms_copy(pinfo->present_parts);
 
 		/*
-		 * Grab some info from the table's relcache; lock was already obtained
-		 * by ExecLockNonLeafAppendTables.
+		 * We need to hold a pin on the partitioned table's relcache entry so
+		 * that we can rely on its copies of the table's partition key and
+		 * partition descriptor.  We need not get a lock though; one should
+		 * have been acquired already by InitPlan or
+		 * ExecLockNonLeafAppendTables.
 		 */
-		rel = relation_open(pinfo->reloid, NoLock);
+		context->partrel = relation_open(pinfo->reloid, NoLock);
 
-		partkey = RelationGetPartitionKey(rel);
-		partdesc = RelationGetPartitionDesc(rel);
+		partkey = RelationGetPartitionKey(context->partrel);
+		partdesc = RelationGetPartitionDesc(context->partrel);
+		n_steps = list_length(pinfo->pruning_steps);
 
 		context->strategy = partkey->strategy;
 		context->partnatts = partnatts = partkey->partnatts;
-		context->partopfamily = partkey->partopfamily;
-		context->partopcintype = partkey->partopcintype;
+		context->nparts = pinfo->nparts;
+		context->boundinfo = partdesc->boundinfo;
 		context->partcollation = partkey->partcollation;
 		context->partsupfunc = partkey->partsupfunc;
-		context->nparts = pinfo->nparts;
-		context->boundinfo = partition_bounds_copy(partdesc->boundinfo, partkey);
+
+		/* We'll look up type-specific support functions as needed */
+		context->stepcmpfuncs = (FmgrInfo *)
+			palloc0(sizeof(FmgrInfo) * n_steps * partnatts);
+
+		context->ppccontext = CurrentMemoryContext;
 		context->planstate = planstate;
 
 		/* Initialize expression state for each expression we need */
-		n_steps = list_length(pinfo->pruning_steps);
 		context->exprstates = (ExprState **)
 			palloc0(sizeof(ExprState *) * n_steps * partnatts);
 		foreach(lc2, pinfo->pruning_steps)
@@ -1527,8 +1536,6 @@ ExecSetupPartitionPruneState(PlanState *planstate, List *partitionpruneinfo)
 		prunestate->execparamids = bms_add_members(prunestate->execparamids,
 												   pinfo->execparamids);
 
-		relation_close(rel, NoLock);
-
 		i++;
 	}
 
@@ -1536,6 +1543,26 @@ ExecSetupPartitionPruneState(PlanState *planstate, List *partitionpruneinfo)
 }
 
 /*
+ * ExecDestroyPartitionPruneState
+ *		Release resources at plan shutdown.
+ *
+ * We don't bother to free any memory here, since the whole executor context
+ * will be going away shortly.  We do need to release our relcache pins.
+ */
+void
+ExecDestroyPartitionPruneState(PartitionPruneState *prunestate)
+{
+	int			i;
+
+	for (i = 0; i < prunestate->num_partprunedata; i++)
+	{
+		PartitionPruningData *pprune = &prunestate->partprunedata[i];
+
+		relation_close(pprune->context.partrel, NoLock);
+	}
+}
+
+/*
  * ExecFindInitialMatchingSubPlans
  *		Identify the set of subplans that cannot be eliminated by initial
  *		pruning (disregarding any pruning constraints involving PARAM_EXEC
diff --git a/src/backend/executor/nodeAppend.c b/src/backend/executor/nodeAppend.c
index 6dd53e90ba8..5ce4fb43e1a 100644
--- a/src/backend/executor/nodeAppend.c
+++ b/src/backend/executor/nodeAppend.c
@@ -136,8 +136,10 @@ ExecInitAppend(Append *node, EState *estate, int eflags)
 		/* We may need an expression context to evaluate partition exprs */
 		ExecAssignExprContext(estate, &appendstate->ps);
 
-		prunestate = ExecSetupPartitionPruneState(&appendstate->ps,
-												  node->part_prune_infos);
+		/* Create the working data structure for pruning. */
+		prunestate = ExecCreatePartitionPruneState(&appendstate->ps,
+												   node->part_prune_infos);
+		appendstate->as_prune_state = prunestate;
 
 		/* Perform an initial partition prune, if required. */
 		if (prunestate->do_initial_prune)
@@ -178,8 +180,6 @@ ExecInitAppend(Append *node, EState *estate, int eflags)
 		 */
 		if (!prunestate->do_exec_prune)
 			appendstate->as_valid_subplans = bms_add_range(NULL, 0, nplans - 1);
-
-		appendstate->as_prune_state = prunestate;
 	}
 	else
 	{
@@ -330,6 +330,12 @@ ExecEndAppend(AppendState *node)
 	 */
 	for (i = 0; i < nplans; i++)
 		ExecEndNode(appendplans[i]);
+
+	/*
+	 * release any resources associated with run-time pruning
+	 */
+	if (node->as_prune_state)
+		ExecDestroyPartitionPruneState(node->as_prune_state);
 }
 
 void