From 7487400397e60abaf7807cb33b2397f1a81a7ac1 Mon Sep 17 00:00:00 2001 From: "dgrowley@gmail.com" Date: Mon, 18 Jun 2018 20:42:13 +1200 Subject: [PATCH v3] 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 an unused function in core, but it now remains only for backward compatibility for 3rd party code. This fixes a planner performance regression new to v11 reported by Thomas Reiss. --- src/backend/optimizer/plan/planmain.c | 6 ++++ src/backend/optimizer/plan/planner.c | 4 +++ src/backend/optimizer/prep/prepunion.c | 30 +++++++++---------- src/backend/optimizer/util/relnode.c | 54 ++++++++++++++++++++++++++-------- src/include/nodes/relation.h | 7 +++++ src/include/optimizer/pathnode.h | 1 + 6 files changed, 75 insertions(+), 27 deletions(-) diff --git a/src/backend/optimizer/plan/planmain.c b/src/backend/optimizer/plan/planmain.c index 7a34abca04..b05adc70c4 100644 --- a/src/backend/optimizer/plan/planmain.c +++ b/src/backend/optimizer/plan/planmain.c @@ -124,6 +124,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 diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 67a2c7a581..da112ddfd8 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 0ab4014be6..931174206a 100644 --- a/src/backend/optimizer/prep/prepunion.c +++ b/src/backend/optimizer/prep/prepunion.c @@ -166,6 +166,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,23 @@ 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) + Assert(root->append_rel_array); + 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 82b78420e7..ea63d834de 100644 --- a/src/backend/optimizer/util/relnode.c +++ b/src/backend/optimizer/util/relnode.c @@ -88,6 +88,44 @@ 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 will remain unallocated if there are no AppendRelInfos to add to + * the array. + */ +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. @@ -1187,27 +1225,18 @@ 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 (root->append_rel_array[relid]) + return root->append_rel_array[relid]; - 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 */ @@ -1228,10 +1257,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); diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h index 5af484024a..ad06822025 100644 --- a/src/include/nodes/relation.h +++ b/src/include/nodes/relation.h @@ -151,6 +151,7 @@ typedef struct PlannerGlobal #define planner_subplan_get_plan(root, subplan) \ ((Plan *) list_nth((root)->glob->subplans, (subplan)->plan_id - 1)) +struct AppendRelInfo; /*---------- * PlannerInfo @@ -201,6 +202,12 @@ typedef struct PlannerInfo */ RangeTblEntry **simple_rte_array; /* rangetable as an array */ + /* + * append_rel_list contents indexed by child_relid, or NULL if + * append_rel_list is empty. + */ + struct AppendRelInfo **append_rel_array; + /* * all_baserels is a Relids set of all base relids (but not "other" * relids) in the query; that is, the Relids identifier of the final join diff --git a/src/include/optimizer/pathnode.h b/src/include/optimizer/pathnode.h index e99ae36bef..c1c301a648 100644 --- a/src/include/optimizer/pathnode.h +++ b/src/include/optimizer/pathnode.h @@ -261,6 +261,7 @@ extern Path *reparameterize_path_by_child(PlannerInfo *root, Path *path, * prototypes for relnode.c */ extern void setup_simple_rel_arrays(PlannerInfo *root); +extern void setup_append_rel_array(PlannerInfo *root); extern RelOptInfo *build_simple_rel(PlannerInfo *root, int relid, RelOptInfo *parent); extern RelOptInfo *find_base_rel(PlannerInfo *root, int relid); -- 2.16.2.windows.1