From e139f1953f29db245f60a7acb72fccb1e05d2442 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Tue, 15 Aug 2017 12:30:38 -0400 Subject: [PATCH] Assorted preparatory refactoring for partition-wise join. Instead of duplicating the logic to search for a matching ParamPathInfo in multiple places, factor it out into a separate function. Pass only the relevant bits of the PartitionKey to partition_bounds_equal instead of the whole thing, because partition-wise join will want to call this without having a PartitionKey available. Adjust allow_star_schema_join and calc_nestloop_required_outer to take relevant Relids rather than the entire Path, because partition-wise join will want to call it with the top-parent relids to determine whether a child join is allowable. Ashutosh Bapat. Review and testing of the larger patch set of which this is a part by Amit Langote, Rajkumar Raghuwanshi, Rafia Sabih, Thomas Munro, Dilip Kumar, and me. Discussion: http://postgr.es/m/CA+TgmobQK80vtXjAsPZWWXd7c8u13G86gmuLupN+uUJjA+i4nA@mail.gmail.com --- src/backend/catalog/partition.c | 9 +++--- src/backend/optimizer/path/joinpath.c | 27 ++++++++-------- src/backend/optimizer/util/pathnode.c | 11 ++++--- src/backend/optimizer/util/relnode.c | 45 ++++++++++++++++----------- src/backend/utils/cache/relcache.c | 4 ++- src/include/catalog/partition.h | 5 +-- src/include/optimizer/pathnode.h | 7 ++++- 7 files changed, 62 insertions(+), 46 deletions(-) diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c index 71bc4b3d10..c1a307c8d3 100644 --- a/src/backend/catalog/partition.c +++ b/src/backend/catalog/partition.c @@ -583,7 +583,7 @@ RelationBuildPartitionDesc(Relation rel) * representation of partition bounds. */ bool -partition_bounds_equal(PartitionKey key, +partition_bounds_equal(int partnatts, int16 *parttyplen, bool *parttypbyval, PartitionBoundInfo b1, PartitionBoundInfo b2) { int i; @@ -601,7 +601,7 @@ partition_bounds_equal(PartitionKey key, { int j; - for (j = 0; j < key->partnatts; j++) + for (j = 0; j < partnatts; j++) { /* For range partitions, the bounds might not be finite. */ if (b1->kind != NULL) @@ -627,8 +627,7 @@ partition_bounds_equal(PartitionKey key, * context. datumIsEqual() should be simple enough to be safe. */ if (!datumIsEqual(b1->datums[i][j], b2->datums[i][j], - key->parttypbyval[j], - key->parttyplen[j])) + parttypbyval[j], parttyplen[j])) return false; } @@ -637,7 +636,7 @@ partition_bounds_equal(PartitionKey key, } /* There are ndatums+1 indexes in case of range partitions */ - if (key->strategy == PARTITION_STRATEGY_RANGE && + if (b1->strategy == PARTITION_STRATEGY_RANGE && b1->indexes[i] != b2->indexes[i]) return false; diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c index 511c734980..43833ea9c9 100644 --- a/src/backend/optimizer/path/joinpath.c +++ b/src/backend/optimizer/path/joinpath.c @@ -318,18 +318,15 @@ add_paths_to_joinrel(PlannerInfo *root, */ static inline bool allow_star_schema_join(PlannerInfo *root, - Path *outer_path, - Path *inner_path) + Relids outerrelids, + Relids inner_paramrels) { - Relids innerparams = PATH_REQ_OUTER(inner_path); - Relids outerrelids = outer_path->parent->relids; - /* * It's a star-schema case if the outer rel provides some but not all of * the inner rel's parameterization. */ - return (bms_overlap(innerparams, outerrelids) && - bms_nonempty_difference(innerparams, outerrelids)); + return (bms_overlap(inner_paramrels, outerrelids) && + bms_nonempty_difference(inner_paramrels, outerrelids)); } /* @@ -348,6 +345,12 @@ try_nestloop_path(PlannerInfo *root, { Relids required_outer; JoinCostWorkspace workspace; + RelOptInfo *innerrel = inner_path->parent; + RelOptInfo *outerrel = outer_path->parent; + Relids innerrelids = innerrel->relids; + Relids outerrelids = outerrel->relids; + Relids inner_paramrels = PATH_REQ_OUTER(inner_path); + Relids outer_paramrels = PATH_REQ_OUTER(outer_path); /* * Check to see if proposed path is still parameterized, and reject if the @@ -356,14 +359,12 @@ try_nestloop_path(PlannerInfo *root, * doesn't like the look of it, which could only happen if the nestloop is * still parameterized. */ - required_outer = calc_nestloop_required_outer(outer_path, - inner_path); + required_outer = calc_nestloop_required_outer(outerrelids, outer_paramrels, + innerrelids, inner_paramrels); if (required_outer && ((!bms_overlap(required_outer, extra->param_source_rels) && - !allow_star_schema_join(root, outer_path, inner_path)) || - have_dangerous_phv(root, - outer_path->parent->relids, - PATH_REQ_OUTER(inner_path)))) + !allow_star_schema_join(root, outerrelids, inner_paramrels)) || + have_dangerous_phv(root, outerrelids, inner_paramrels))) { /* Waste no memory when we reject a path here */ bms_free(required_outer); diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c index f2d6385f18..26567cb7f6 100644 --- a/src/backend/optimizer/util/pathnode.c +++ b/src/backend/optimizer/util/pathnode.c @@ -1993,14 +1993,15 @@ create_foreignscan_path(PlannerInfo *root, RelOptInfo *rel, * Note: result must not share storage with either input */ Relids -calc_nestloop_required_outer(Path *outer_path, Path *inner_path) +calc_nestloop_required_outer(Relids outerrelids, + Relids outer_paramrels, + Relids innerrelids, + Relids inner_paramrels) { - Relids outer_paramrels = PATH_REQ_OUTER(outer_path); - Relids inner_paramrels = PATH_REQ_OUTER(inner_path); Relids required_outer; /* inner_path can require rels from outer path, but not vice versa */ - Assert(!bms_overlap(outer_paramrels, inner_path->parent->relids)); + Assert(!bms_overlap(outer_paramrels, innerrelids)); /* easy case if inner path is not parameterized */ if (!inner_paramrels) return bms_copy(outer_paramrels); @@ -2008,7 +2009,7 @@ calc_nestloop_required_outer(Path *outer_path, Path *inner_path) required_outer = bms_union(outer_paramrels, inner_paramrels); /* ... and remove any mention of now-satisfied outer rels */ required_outer = bms_del_members(required_outer, - outer_path->parent->relids); + outerrelids); /* maintain invariant that required_outer is exactly NULL if empty */ if (bms_is_empty(required_outer)) { diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c index c8e2b67da4..8ad0b4a669 100644 --- a/src/backend/optimizer/util/relnode.c +++ b/src/backend/optimizer/util/relnode.c @@ -1048,12 +1048,8 @@ get_baserel_parampathinfo(PlannerInfo *root, RelOptInfo *baserel, Assert(!bms_overlap(baserel->relids, required_outer)); /* If we already have a PPI for this parameterization, just return it */ - foreach(lc, baserel->ppilist) - { - ppi = (ParamPathInfo *) lfirst(lc); - if (bms_equal(ppi->ppi_req_outer, required_outer)) - return ppi; - } + if ((ppi = find_param_path_info(baserel, required_outer))) + return ppi; /* * Identify all joinclauses that are movable to this base rel given this @@ -1290,12 +1286,8 @@ get_joinrel_parampathinfo(PlannerInfo *root, RelOptInfo *joinrel, *restrict_clauses = list_concat(pclauses, *restrict_clauses); /* If we already have a PPI for this parameterization, just return it */ - foreach(lc, joinrel->ppilist) - { - ppi = (ParamPathInfo *) lfirst(lc); - if (bms_equal(ppi->ppi_req_outer, required_outer)) - return ppi; - } + if ((ppi = find_param_path_info(joinrel, required_outer))) + return ppi; /* Estimate the number of rows returned by the parameterized join */ rows = get_parameterized_joinrel_size(root, joinrel, @@ -1334,7 +1326,6 @@ ParamPathInfo * get_appendrel_parampathinfo(RelOptInfo *appendrel, Relids required_outer) { ParamPathInfo *ppi; - ListCell *lc; /* Unparameterized paths have no ParamPathInfo */ if (bms_is_empty(required_outer)) @@ -1343,12 +1334,8 @@ get_appendrel_parampathinfo(RelOptInfo *appendrel, Relids required_outer) Assert(!bms_overlap(appendrel->relids, required_outer)); /* If we already have a PPI for this parameterization, just return it */ - foreach(lc, appendrel->ppilist) - { - ppi = (ParamPathInfo *) lfirst(lc); - if (bms_equal(ppi->ppi_req_outer, required_outer)) - return ppi; - } + if ((ppi = find_param_path_info(appendrel, required_outer))) + return ppi; /* Else build the ParamPathInfo */ ppi = makeNode(ParamPathInfo); @@ -1359,3 +1346,23 @@ get_appendrel_parampathinfo(RelOptInfo *appendrel, Relids required_outer) return ppi; } + +/* + * Returns a ParamPathInfo for the parameterization given by required_outer, if + * already available in the given rel. Returns NULL otherwise. + */ +ParamPathInfo * +find_param_path_info(RelOptInfo *rel, Relids required_outer) +{ + ListCell *lc; + + foreach(lc, rel->ppilist) + { + ParamPathInfo *ppi = (ParamPathInfo *) lfirst(lc); + + if (bms_equal(ppi->ppi_req_outer, required_outer)) + return ppi; + } + + return NULL; +} diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 6bd93b03d0..2150fe9a39 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -1208,7 +1208,9 @@ equalPartitionDescs(PartitionKey key, PartitionDesc partdesc1, if (partdesc2->boundinfo == NULL) return false; - if (!partition_bounds_equal(key, partdesc1->boundinfo, + if (!partition_bounds_equal(key->partnatts, key->parttyplen, + key->parttypbyval, + partdesc1->boundinfo, partdesc2->boundinfo)) return false; } diff --git a/src/include/catalog/partition.h b/src/include/catalog/partition.h index 434ded37d7..bef7a0f5fb 100644 --- a/src/include/catalog/partition.h +++ b/src/include/catalog/partition.h @@ -71,8 +71,9 @@ typedef struct PartitionDispatchData typedef struct PartitionDispatchData *PartitionDispatch; extern void RelationBuildPartitionDesc(Relation relation); -extern bool partition_bounds_equal(PartitionKey key, - PartitionBoundInfo p1, PartitionBoundInfo p2); +extern bool partition_bounds_equal(int partnatts, int16 *parttyplen, + bool *parttypbyval, PartitionBoundInfo b1, + PartitionBoundInfo b2); extern void check_new_partition_bound(char *relname, Relation parent, PartitionBoundSpec *spec); diff --git a/src/include/optimizer/pathnode.h b/src/include/optimizer/pathnode.h index 0c0549db7f..e372f8862b 100644 --- a/src/include/optimizer/pathnode.h +++ b/src/include/optimizer/pathnode.h @@ -112,7 +112,10 @@ extern ForeignPath *create_foreignscan_path(PlannerInfo *root, RelOptInfo *rel, Path *fdw_outerpath, List *fdw_private); -extern Relids calc_nestloop_required_outer(Path *outer_path, Path *inner_path); +extern Relids calc_nestloop_required_outer(Relids outerrelids, + Relids outer_paramrels, + Relids innerrelids, + Relids inner_paramrels); extern Relids calc_non_nestloop_required_outer(Path *outer_path, Path *inner_path); extern NestPath *create_nestloop_path(PlannerInfo *root, @@ -285,5 +288,7 @@ extern ParamPathInfo *get_joinrel_parampathinfo(PlannerInfo *root, List **restrict_clauses); extern ParamPathInfo *get_appendrel_parampathinfo(RelOptInfo *appendrel, Relids required_outer); +extern ParamPathInfo *find_param_path_info(RelOptInfo *rel, + Relids required_outer); #endif /* PATHNODE_H */ -- 2.40.0