From 4d823ad9179e98a206de9c2b5a8c8ea30415abb6 Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat Date: Tue, 7 Feb 2017 17:16:46 +0530 Subject: [PATCH 09/11] Adjust join related to code to accept child relations. Existing join related code doesn't expect child relations to be joined. This patch contains various fixes to change that. 1. Uniqe-ifying joining relations. ================================= For semi-joins we unique-ify the joining relations, which tries to estimate nummber of unique values using estimate_num_groups(). This function doesn't expect a Var from a child relation and contained an assertion to that effect. With partition-wise joins, we may compute a join between child relations. This commit changes that assertion to include child relation. The function doesn't need any change other than that to accomodate child relations. 2. OUTER joins require dummy child relations to have targetlist. ================================================================ We need a targetlist defining nullable columns for an outer join, even if the relation on the nullable side is deemed to be empty. Prior to partition-wise join an empty child relation never had a targetlist since it was eliminated from planning. But with partition-wise join an empty child relation may participate in an outer join with another non-empty child relation. Hence set targetlist for a child relation even if it's dummy. 3. prepare_sort_from_pathkeys fixes. ==================================== Before partition-wise join feature were never required to be directly sorted, let's say for merge joins. With partition-wise join feature, the child relations will participate directly in the join and also need to be sorted directly for the purpose of merge join. In order to sort a relation, we use pathkeys. The expression on which to sort a particular relation is provided by the equivalence member corresponding to that relation in the equivalence class referred by the pathkeys. Since the code doesn't expect child relations to bubble up to the sorting, the function prepare_sort_from_pathkeys() skips any child members (those set with em_is_child) unless the caller specifically asks for child relations by passing relids. make_sort_from_pathkeys() calls prepare_sort_from_pathkeys() to create Sort plan for outer and inner plans without passing relids of the relation to be sorted. For partition-wise joins the outer and inner plans produce child relations and thus prepare_sort_from_pathkeys() does not find equivalence members since it skips child members for the want of relids. This particular instance can be fixed by passing outer/inner_path->parent->relids to prepare_sort_from_pathkeys(). All the callers of prepare_sort_from_pathkeys() viz. create_merge_append_plan(), create_merge_append_plan(), create_windowagg_plan() except make_sort_from_pathkeys() pass relids to prepare_sort_from_pathkeys(). make_sort_from_pathkeys() as well passes those with this patch. make_sort_from_pathkeys() itself doesn't know the relids of relation being sorted. It just gets the plan. Hence we need to pass relids to make_sort_from_pathkeys() and thus change each of its callers to pass relids, if required. It has two callers as of now. 1. create_sort_plan(PlannerInfo *root, SortPath *best_path, int flags): does not handle child relations yet, so doesn't need any change. 2. create_mergejoin_plan(PlannerInfo *root, MergePath *best_path): It requires this change and the relids can be obtained from the outer and inner path's parent RelOptInfo. 4. Handling em_is_child cases. ============================== Right now, when comparing relids for child relations, only exact match is considered. This is fine as long as em_relids has only a single member in it and the passed in relids has only a single member in it. But with partition-wise join, relids can have multiple members and em_relids may not exactly match the given relids. But we need to find the member which covers subset of given relids. --- src/backend/optimizer/path/allpaths.c | 41 +++++++++++++++++-------------- src/backend/optimizer/plan/createplan.c | 28 +++++++++++++-------- src/backend/utils/adt/selfuncs.c | 3 ++- 3 files changed, 42 insertions(+), 30 deletions(-) diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index a024f47..7e806c1 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -960,11 +960,27 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel, } /* - * We have to copy the parent's targetlist and quals to the child, - * with appropriate substitution of variables. However, only the - * baserestrictinfo quals are needed before we can check for - * constraint exclusion; so do that first and then check to see if we - * can disregard this child. + * Copy/Modify targetlist. Even if this child is deemed empty, we need + * its targetlist in case it falls on nullable side in a child-join + * because of partition-wise join. + * + * NB: the resulting childrel->reltarget->exprs may contain arbitrary + * expressions, which otherwise would not occur in a rel's targetlist. + * Code that might be looking at an appendrel child must cope with + * such. (Normally, a rel's targetlist would only include Vars and + * PlaceHolderVars.) XXX we do not bother to update the cost or width + * fields of childrel->reltarget; not clear if that would be useful. + */ + childrel->reltarget->exprs = (List *) + adjust_appendrel_attrs(root, + (Node *) rel->reltarget->exprs, + appinfo_list); + + /* + * We have to copy the parent's quals to the child, with appropriate + * substitution of variables. However, only the baserestrictinfo quals + * are needed before we can check for constraint exclusion; so do that + * first and then check to see if we can disregard this child. * * The child rel's targetlist might contain non-Var expressions, which * means that substitution into the quals could produce opportunities @@ -1091,22 +1107,9 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel, continue; } - /* - * CE failed, so finish copying/modifying targetlist and join quals. - * - * NB: the resulting childrel->reltarget->exprs may contain arbitrary - * expressions, which otherwise would not occur in a rel's targetlist. - * Code that might be looking at an appendrel child must cope with - * such. (Normally, a rel's targetlist would only include Vars and - * PlaceHolderVars.) XXX we do not bother to update the cost or width - * fields of childrel->reltarget; not clear if that would be useful. - */ + /* CE failed, so finish copying/modifying targetlist and join quals. */ childrel->joininfo = build_child_clauses(root, rel->joininfo, appinfo_list); - childrel->reltarget->exprs = (List *) - adjust_appendrel_attrs(root, - (Node *) rel->reltarget->exprs, - appinfo_list); /* * We have to make child entries in the EquivalenceClass data diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index fe6b7f8..d0705dc 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -245,7 +245,8 @@ static Plan *prepare_sort_from_pathkeys(Plan *lefttree, List *pathkeys, static EquivalenceMember *find_ec_member_for_tle(EquivalenceClass *ec, TargetEntry *tle, Relids relids); -static Sort *make_sort_from_pathkeys(Plan *lefttree, List *pathkeys); +static Sort *make_sort_from_pathkeys(Plan *lefttree, List *pathkeys, + Relids relids); static Sort *make_sort_from_groupcols(List *groupcls, AttrNumber *grpColIdx, Plan *lefttree); @@ -1555,7 +1556,7 @@ create_sort_plan(PlannerInfo *root, SortPath *best_path, int flags) subplan = create_plan_recurse(root, best_path->subpath, flags | CP_SMALL_TLIST); - plan = make_sort_from_pathkeys(subplan, best_path->path.pathkeys); + plan = make_sort_from_pathkeys(subplan, best_path->path.pathkeys, NULL); copy_generic_path_info(&plan->plan, (Path *) best_path); @@ -3572,6 +3573,8 @@ create_mergejoin_plan(PlannerInfo *root, ListCell *lc; ListCell *lop; ListCell *lip; + Path *outer_path = best_path->jpath.outerjoinpath; + Path *inner_path = best_path->jpath.innerjoinpath; /* * MergeJoin can project, so we don't have to demand exact tlists from the @@ -3635,8 +3638,10 @@ create_mergejoin_plan(PlannerInfo *root, */ if (best_path->outersortkeys) { + Relids outer_relids = outer_path->parent->relids; Sort *sort = make_sort_from_pathkeys(outer_plan, - best_path->outersortkeys); + best_path->outersortkeys, + outer_relids); label_sort_with_costsize(root, sort, -1.0); outer_plan = (Plan *) sort; @@ -3647,8 +3652,10 @@ create_mergejoin_plan(PlannerInfo *root, if (best_path->innersortkeys) { + Relids inner_relids = inner_path->parent->relids; Sort *sort = make_sort_from_pathkeys(inner_plan, - best_path->innersortkeys); + best_path->innersortkeys, + inner_relids); label_sort_with_costsize(root, sort, -1.0); inner_plan = (Plan *) sort; @@ -5630,11 +5637,11 @@ prepare_sort_from_pathkeys(Plan *lefttree, List *pathkeys, continue; /* - * Ignore child members unless they match the rel being + * Ignore child members unless they belong to the rel being * sorted. */ if (em->em_is_child && - !bms_equal(em->em_relids, relids)) + !bms_is_subset(em->em_relids, relids)) continue; sortexpr = em->em_expr; @@ -5745,10 +5752,10 @@ find_ec_member_for_tle(EquivalenceClass *ec, continue; /* - * Ignore child members unless they match the rel being sorted. + * Ignore child members unless they belong to the rel being sorted. */ if (em->em_is_child && - !bms_equal(em->em_relids, relids)) + !bms_is_subset(em->em_relids, relids)) continue; /* Match if same expression (after stripping relabel) */ @@ -5769,9 +5776,10 @@ find_ec_member_for_tle(EquivalenceClass *ec, * * 'lefttree' is the node which yields input tuples * 'pathkeys' is the list of pathkeys by which the result is to be sorted + * 'relids' is the set of relations required by prepare_sort_from_pathkeys() */ static Sort * -make_sort_from_pathkeys(Plan *lefttree, List *pathkeys) +make_sort_from_pathkeys(Plan *lefttree, List *pathkeys, Relids relids) { int numsortkeys; AttrNumber *sortColIdx; @@ -5781,7 +5789,7 @@ make_sort_from_pathkeys(Plan *lefttree, List *pathkeys) /* Compute sort column info, and adjust lefttree as needed */ lefttree = prepare_sort_from_pathkeys(lefttree, pathkeys, - NULL, + relids, NULL, false, &numsortkeys, diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c index fa32e9e..c833846 100644 --- a/src/backend/utils/adt/selfuncs.c +++ b/src/backend/utils/adt/selfuncs.c @@ -3427,7 +3427,8 @@ estimate_num_groups(PlannerInfo *root, List *groupExprs, double input_rows, /* * Sanity check --- don't divide by zero if empty relation. */ - Assert(rel->reloptkind == RELOPT_BASEREL); + Assert(rel->reloptkind == RELOPT_BASEREL || + rel->reloptkind == RELOPT_OTHER_MEMBER_REL); if (rel->tuples > 0) { /* -- 1.7.9.5