Allow direct lookups of AppendRelInfo by child relid

find_appinfos_by_relids had quite a large overhead when the number of
items in the append_rel_list was high, as it had to trawl through the
append_rel_list looking for AppendRelInfos belonging to the given
childrelids.  Since there can only be a single AppendRelInfo for each
child rel, it seems much better to store an array in PlannerInfo which
indexes these by child relid, making the function O(1) rather than O(N).
This function was only called once inside the planner, so just replace
that call with a lookup to the new array.  find_childrel_appendrelinfo
is now unused and thus removed.

This fixes a planner performance regression new to v11 reported by
Thomas Reiss.

Author: David Rowley
Reported-by: Thomas Reiss
Reviewed-by: Ashutosh Bapat
Reviewed-by: Álvaro Herrera
Discussion: https://postgr.es/m/94dd7a4b-5e50-0712-911d-2278e055c622@dalibo.com

4 files changed, 63 insertions, 46 deletions

diff --git a/src/backend/optimizer/plan/planmain.c b/src/backend/optimizer/plan/planmain.c
index 7a34abca048..b05adc70c4a 100644
--- a/src/backend/optimizer/plan/planmain.c
+++ b/src/backend/optimizer/plan/planmain.c
@@ -125,6 +125,12 @@ query_planner(PlannerInfo *root, List *tlist,
 	setup_simple_rel_arrays(root);
 
 	/*
+	 * Populate append_rel_array with each AppendRelInfo to allow direct
+	 * lookups by child relid.
+	 */
+	setup_append_rel_array(root);
+
+	/*
 	 * Construct RelOptInfo nodes for all base relations in query, and
 	 * indirectly for all appendrel member relations ("other rels").  This
 	 * will give us a RelOptInfo for every "simple" (non-join) rel involved in
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 1f09fb6e6ad..fd45c9767df 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -1163,6 +1163,7 @@ inheritance_planner(PlannerInfo *root)
 	List	   *final_rtable = NIL;
 	int			save_rel_array_size = 0;
 	RelOptInfo **save_rel_array = NULL;
+	AppendRelInfo **save_append_rel_array = NULL;
 	List	   *subpaths = NIL;
 	List	   *subroots = NIL;
 	List	   *resultRelations = NIL;
@@ -1529,6 +1530,7 @@ inheritance_planner(PlannerInfo *root)
 		}
 		save_rel_array_size = subroot->simple_rel_array_size;
 		save_rel_array = subroot->simple_rel_array;
+		save_append_rel_array = subroot->append_rel_array;
 
 		/* Make sure any initplans from this rel get into the outer list */
 		root->init_plans = subroot->init_plans;
@@ -1579,6 +1581,8 @@ inheritance_planner(PlannerInfo *root)
 	parse->rtable = final_rtable;
 	root->simple_rel_array_size = save_rel_array_size;
 	root->simple_rel_array = save_rel_array;
+	root->append_rel_array = save_append_rel_array;
+
 	/* Must reconstruct master's simple_rte_array, too */
 	root->simple_rte_array = (RangeTblEntry **)
 		palloc0((list_length(final_rtable) + 1) * sizeof(RangeTblEntry *));
diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c
index 0ab4014be6d..2d470240d5d 100644
--- a/src/backend/optimizer/prep/prepunion.c
+++ b/src/backend/optimizer/prep/prepunion.c
@@ -167,6 +167,12 @@ plan_set_operations(PlannerInfo *root)
 	setup_simple_rel_arrays(root);
 
 	/*
+	 * Populate append_rel_array with each AppendRelInfo to allow direct
+	 * lookups by child relid.
+	 */
+	setup_append_rel_array(root);
+
+	/*
 	 * Find the leftmost component Query.  We need to use its column names for
 	 * all generated tlists (else SELECT INTO won't work right).
 	 */
@@ -2617,29 +2623,22 @@ build_child_join_sjinfo(PlannerInfo *root, SpecialJoinInfo *parent_sjinfo,
 AppendRelInfo **
 find_appinfos_by_relids(PlannerInfo *root, Relids relids, int *nappinfos)
 {
-	ListCell   *lc;
 	AppendRelInfo **appinfos;
 	int			cnt = 0;
+	int			i;
 
 	*nappinfos = bms_num_members(relids);
 	appinfos = (AppendRelInfo **) palloc(sizeof(AppendRelInfo *) * *nappinfos);
 
-	foreach(lc, root->append_rel_list)
+	i = -1;
+	while ((i = bms_next_member(relids, i)) >= 0)
 	{
-		AppendRelInfo *appinfo = lfirst(lc);
+		AppendRelInfo *appinfo = root->append_rel_array[i];
 
-		if (bms_is_member(appinfo->child_relid, relids))
-		{
-			appinfos[cnt] = appinfo;
-			cnt++;
+		if (!appinfo)
+			elog(ERROR, "child rel %d not found in append_rel_array", i);
 
-			/* Stop when we have gathered all the AppendRelInfos. */
-			if (cnt == *nappinfos)
-				return appinfos;
-		}
+		appinfos[cnt++] = appinfo;
 	}
-
-	/* Should have found the entries ... */
-	elog(ERROR, "did not find all requested child rels in append_rel_list");
-	return NULL;				/* not reached */
+	return appinfos;
 }
diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c
index 82b78420e79..c69740eda6c 100644
--- a/src/backend/optimizer/util/relnode.c
+++ b/src/backend/optimizer/util/relnode.c
@@ -89,6 +89,43 @@ setup_simple_rel_arrays(PlannerInfo *root)
 }
 
 /*
+ * setup_append_rel_array
+ *		Populate the append_rel_array to allow direct lookups of
+ *		AppendRelInfos by child relid.
+ *
+ * The array remains unallocated if there are no AppendRelInfos.
+ */
+void
+setup_append_rel_array(PlannerInfo *root)
+{
+	ListCell   *lc;
+	int			size = list_length(root->parse->rtable) + 1;
+
+	if (root->append_rel_list == NIL)
+	{
+		root->append_rel_array = NULL;
+		return;
+	}
+
+	root->append_rel_array = (AppendRelInfo **)
+		palloc0(size * sizeof(AppendRelInfo *));
+
+	foreach(lc, root->append_rel_list)
+	{
+		AppendRelInfo *appinfo = lfirst_node(AppendRelInfo, lc);
+		int			child_relid = appinfo->child_relid;
+
+		/* Sanity check */
+		Assert(child_relid < size);
+
+		if (root->append_rel_array[child_relid])
+			elog(ERROR, "child relation already exists");
+
+		root->append_rel_array[child_relid] = appinfo;
+	}
+}
+
+/*
  * build_simple_rel
  *	  Construct a new RelOptInfo for a base relation or 'other' relation.
  */
@@ -1185,36 +1222,6 @@ fetch_upper_rel(PlannerInfo *root, UpperRelationKind kind, Relids relids)
 
 
 /*
- * find_childrel_appendrelinfo
- *		Get the AppendRelInfo associated with an appendrel child rel.
- *
- * This search could be eliminated by storing a link in child RelOptInfos,
- * but for now it doesn't seem performance-critical.  (Also, it might be
- * difficult to maintain such a link during mutation of the append_rel_list.)
- */
-AppendRelInfo *
-find_childrel_appendrelinfo(PlannerInfo *root, RelOptInfo *rel)
-{
-	Index		relid = rel->relid;
-	ListCell   *lc;
-
-	/* Should only be called on child rels */
-	Assert(rel->reloptkind == RELOPT_OTHER_MEMBER_REL);
-
-	foreach(lc, root->append_rel_list)
-	{
-		AppendRelInfo *appinfo = (AppendRelInfo *) lfirst(lc);
-
-		if (appinfo->child_relid == relid)
-			return appinfo;
-	}
-	/* should have found the entry ... */
-	elog(ERROR, "child rel %d not found in append_rel_list", relid);
-	return NULL;				/* not reached */
-}
-
-
-/*
  * find_childrel_parents
  *		Compute the set of parent relids of an appendrel child rel.
  *
@@ -1228,10 +1235,11 @@ find_childrel_parents(PlannerInfo *root, RelOptInfo *rel)
 	Relids		result = NULL;
 
 	Assert(rel->reloptkind == RELOPT_OTHER_MEMBER_REL);
+	Assert(rel->relid > 0 && rel->relid < root->simple_rel_array_size);
 
 	do
 	{
-		AppendRelInfo *appinfo = find_childrel_appendrelinfo(root, rel);
+		AppendRelInfo *appinfo = root->append_rel_array[rel->relid];
 		Index		prelid = appinfo->parent_relid;
 
 		result = bms_add_member(result, prelid);