From 7d872c91a3f9d49b56117557cdbb0c3d4c620687 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Tue, 26 Jun 2018 10:35:26 -0400 Subject: [PATCH] Allow direct lookups of AppendRelInfo by child relid MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 --- src/backend/optimizer/plan/planmain.c | 6 +++ src/backend/optimizer/plan/planner.c | 4 ++ src/backend/optimizer/prep/prepunion.c | 29 ++++++----- src/backend/optimizer/util/relnode.c | 70 ++++++++++++++------------ src/include/nodes/relation.h | 10 ++++ src/include/optimizer/pathnode.h | 3 +- 6 files changed, 74 insertions(+), 48 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 1f09fb6e6a..fd45c9767d 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..2d470240d5 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,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 82b78420e7..c69740eda6 100644 --- a/src/backend/optimizer/util/relnode.c +++ b/src/backend/optimizer/util/relnode.c @@ -88,6 +88,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. @@ -1184,36 +1221,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); diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h index 5af484024a..346cf08936 100644 --- a/src/include/nodes/relation.h +++ b/src/include/nodes/relation.h @@ -162,6 +162,8 @@ typedef struct PlannerGlobal * the passed-in Query data structure; someday that should stop. *---------- */ +struct AppendRelInfo; + typedef struct PlannerInfo { NodeTag type; @@ -201,6 +203,14 @@ typedef struct PlannerInfo */ RangeTblEntry **simple_rte_array; /* rangetable as an array */ + /* + * append_rel_list is the same length as the above arrays, and holds + * pointers to the corresponding AppendRelInfo entry indexed by + * child_relid, or NULL if none. The array itself is not allocated 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..4ba358e72d 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); @@ -278,8 +279,6 @@ extern Relids min_join_parameterization(PlannerInfo *root, extern RelOptInfo *build_empty_join_rel(PlannerInfo *root); extern RelOptInfo *fetch_upper_rel(PlannerInfo *root, UpperRelationKind kind, Relids relids); -extern AppendRelInfo *find_childrel_appendrelinfo(PlannerInfo *root, - RelOptInfo *rel); extern Relids find_childrel_parents(PlannerInfo *root, RelOptInfo *rel); extern ParamPathInfo *get_baserel_parampathinfo(PlannerInfo *root, RelOptInfo *baserel, -- 2.40.0