8000 Fix some more problems with nested append relations. · postgrespro/postgres@07afbca · GitHub
[go: up one dir, main page]
Skip to content

    • Commit 07afbca

    Browse files
    tglsfdctglsfdc
    committed
    Fix some more problems with nested append relations.
    As of commit a87c729 (which later got backpatched as far as 9.1), we're explicitly supporting the notion that append relations can be nested; this can occur when UNION ALL constructs are nested, or when a UNION ALL contains a table with inheritance children. Bug #11457 from Nelson Page, as well as an earlier report from Elvis Pranskevichus, showed that there were still nasty bugs associated with such cases: in particular the EquivalenceClass mechanism could try to generate "join" clauses connecting an appendrel child to some grandparent appendrel, which would result in assertion failures or bogus plans. Upon investigation I concluded that all current callers of find_childrel_appendrelinfo() need to be fixed to explicitly consider multiple levels of parent appendrels. The most complex fix was in processing of "broken" EquivalenceClasses, which are ECs for which we have been unable to generate all the derived equality clauses we would like to because of missing cross-type equality operators in the underlying btree operator family. That code path is more or less entirely untested by the regression tests to date, because no standard opfamilies have such holes in them. So I wrote a new regression test script to try to exercise it a bit, which turned out to be quite a worthwhile activity as it exposed existing bugs in all supported branches. The present patch is essentially the same as far back as 9.2, which is where parameterized paths were introduced. In 9.0 and 9.1, we only need to back-patch a small fragment of commit 5b7b551, which fixes failure to propagate out the original WHERE clauses when a broken EC contains constant members. (The regression test case results show that these older branches are noticeably stupider than 9.2+ in terms of the quality of the plans generated; but we don't really care about plan quality in such cases, only that the plan not be outright wrong. A more invasive fix in the older branches would not be a good idea anyway from a plan-stability standpoint.)
    1 parent 6d89b08 commit 07afbca

    File tree

    10 files changed

    +720
    -27
    lines changed

    10 files changed

    +720
    -27
    lines changed

    ‎src/backend/optimizer/path/equivclass.c

    Lines changed: 20 additions & 18 deletions
    Original file line numberDiff line numberDiff line change
    @@ -48,7 +48,7 @@ static List *generate_join_implied_equalities_broken(PlannerInfo *root,
    4848
    Relids nominal_join_relids,
    4949
    Relids outer_relids,
    5050
    Relids nominal_inner_relids,
    51-
    AppendRelInfo *inner_appinfo);
    51+
    RelOptInfo *inner_rel);
    5252
    static Oid select_equality_operator(EquivalenceClass *ec,
    5353
    Oid lefttype, Oid righttype);
    5454
    static RestrictInfo *create_join_clause(PlannerInfo *root,
    @@ -1000,22 +1000,18 @@ generate_join_implied_equalities(PlannerInfo *root,
    10001000
    Relids inner_relids = inner_rel->relids;
    10011001
    Relids nominal_inner_relids;
    10021002
    Relids nominal_join_relids;
    1003-
    AppendRelInfo *inner_appinfo;
    10041003
    ListCell *lc;
    10051004

    10061005
    /* If inner rel is a child, extra setup work is needed */
    10071006
    if (inner_rel->reloptkind == RELOPT_OTHER_MEMBER_REL)
    10081007
    {
    1009-
    /* Lookup parent->child translation data */
    1010-
    inner_appinfo = find_childrel_appendrelinfo(root, inner_rel);
    1011-
    /* Construct relids for the parent rel */
    1012-
    nominal_inner_relids = bms_make_singleton(inner_appinfo->parent_relid);
    1008+
    /* Fetch relid set for the topmost parent rel */
    1009+
    nominal_inner_relids = find_childrel_top_parent(root, inner_rel)->relids;
    10131010
    /* ECs will be marked with the parent's relid, not the child's */
    10141011
    nominal_join_relids = bms_union(outer_relids, nominal_inner_relids);
    10151012
    }
    10161013
    else
    10171014
    {
    1018-
    inner_appinfo = NULL;
    10191015
    nominal_inner_relids = inner_relids;
    10201016
    nominal_join_relids = join_relids;
    10211017
    }
    @@ -1051,7 +1047,7 @@ generate_join_implied_equalities(PlannerInfo *root,
    10511047
    nominal_join_relids,
    10521048
    outer_relids,
    10531049
    nominal_inner_relids,
    1054-
    inner_appinfo);
    1050+
    inner_rel);
    10551051

    10561052
    result = list_concat(result, sublist);
    10571053
    }
    @@ -1244,7 +1240,7 @@ generate_join_implied_equalities_broken(PlannerInfo *root,
    12441240
    Relids nominal_join_relids,
    12451241
    Relids outer_relids,
    12461242
    Relids nominal_inner_relids,
    1247-
    AppendRelInfo *inner_appinfo)
    1243+
    RelOptInfo *inner_rel)
    12481244
    {
    12491245
    List *result = NIL;
    12501246
    ListCell *lc;
    @@ -1266,10 +1262,16 @@ generate_join_implied_equalities_broken(PlannerInfo *root,
    12661262
    * RestrictInfos that are not listed in ec_derives, but there shouldn't be
    12671263
    * any duplication, and it's a sufficiently narrow corner case that we
    12681264
    * shouldn't sweat too much over it anyway.
    1265+
    *
    1266+
    * Since inner_rel might be an indirect descendant of the baserel
    1267+
    * mentioned in the ec_sources clauses, we have to be prepared to apply
    1268+
    * multiple levels of Var translation.
    12691269
    */
    1270-
    if (inner_appinfo)
    1271-
    result = (List *) adjust_appendrel_attrs(root, (Node *) result,
    1272-
    inner_appinfo);
    1270+
    if (inner_rel->reloptkind == RELOPT_OTHER_MEMBER_REL &&
    1271+
    result != NIL)
    1272+
    result = (List *) adjust_appendrel_attrs_multilevel(root,
    1273+
    (Node *) result,
    1274+
    inner_rel);
    12731275

    12741276
    return result;
    12751277
    }
    @@ -2071,14 +2073,14 @@ generate_implied_equalities_for_column(PlannerInfo *root,
    20712073
    {
    20722074
    List *result = NIL;
    20732075
    bool is_child_rel = (rel->reloptkind == RELOPT_OTHER_MEMBER_REL);
    2074-
    Index parent_relid;
    2076+
    Relids parent_relids;
    20752077
    ListCell *lc1;
    20762078

    2077-
    /* If it's a child rel, we'll need to know what its parent is */
    2079+
    /* If it's a child rel, we'll need to know what its parent(s) are */
    20782080
    if (is_child_rel)
    2079-
    parent_relid = find_childrel_appendrelinfo(root, rel)->parent_relid;
    2081+
    parent_relids = find_childrel_parents(root, rel);
    20802082
    else
    2081-
    parent_relid = 0; /* not used, but keep compiler quiet */
    2083+
    parent_relids = NULL; /* not used, but keep compiler quiet */
    20822084

    20832085
    foreach(lc1, root->eq_classes)
    20842086
    {
    @@ -2148,10 +2150,10 @@ generate_implied_equalities_for_column(PlannerInfo *root,
    21482150

    21492151
    /*
    21502152
    * Also, if this is a child rel, avoid generating a useless join
    2151-
    * to its parent rel.
    2153+
    * to its parent rel(s).
    21522154
    */
    21532155
    if (is_child_rel &&
    2154-
    bms_is_member(parent_relid, other_em->em_relids))
    2156+
    bms_overlap(parent_relids, other_em->em_relids))
    2155< F438 /td>2157
    continue;
    21562158

    21572159
    eq_op = select_equality_operator(cur_ec,

    ‎src/backend/optimizer/path/indxpath.c

    Lines changed: 2 additions & 7 deletions
    Original file line numberDiff line numberDiff line change
    @@ -2586,16 +2586,11 @@ check_partial_indexes(PlannerInfo *root, RelOptInfo *rel)
    25862586
    * Add on any equivalence-derivable join clauses. Computing the correct
    25872587
    * relid sets for generate_join_implied_equalities is slightly tricky
    25882588
    * because the rel could be a child rel rather than a true baserel, and in
    2589-
    * that case we must remove its parent's relid from all_baserels.
    2589+
    * that case we must remove its parents' relid(s) from all_baserels.
    25902590
    */
    25912591
    if (rel->reloptkind == RELOPT_OTHER_MEMBER_REL)
    2592-
    {
    2593-
    /* Lookup parent->child translation data */
    2594-
    AppendRelInfo *appinfo = find_childrel_appendrelinfo(root, rel);
    2595-
    25962592
    otherrels = bms_difference(root->all_baserels,
    2597-
    bms_make_singleton(appinfo->parent_relid));
    2598-
    }
    2593+
    find_childrel_parents(root, rel));
    25992594
    else
    26002595
    otherrels = bms_difference(root->all_baserels, rel->relids);
    26012596

    ‎src/backend/optimizer/prep/prepunion.c

    Lines changed: 23 additions & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -1979,3 +1979,26 @@ adjust_inherited_tlist(List *tlist, AppendRelInfo *context)
    19791979

    19801980
    return new_tlist;
    19811981
    }
    1982+
    1983+
    /*
    1984+
    * adjust_appendrel_attrs_multilevel
    1985+
    * Apply Var translations from a toplevel appendrel parent down to a child.
    1986+
    *
    1987+
    * In some cases we need to translate expressions referencing a baserel
    1988+
    * to reference an appendrel child that's multiple levels removed from it.
    1989+
    */
    1990+
    Node *
    1991+
    adjust_appendrel_attrs_multilevel(PlannerInfo *root, Node *node,
    1992+
    RelOptInfo *child_rel)
    1993+
    {
    1994+
    AppendRelInfo *appinfo = find_childrel_appendrelinfo(root, child_rel);
    1995+
    RelOptInfo *parent_rel = find_base_rel(root, appinfo->parent_relid);
    1996+
    1997+
    /* If parent is also a child, first recurse to apply its translations */
    1998+
    if (parent_rel->reloptkind == RELOPT_OTHER_MEMBER_REL)
    1999+
    node = adjust_appendrel_attrs_multilevel(root, node, parent_rel);
    2000+
    else
    2001+
    Assert(parent_rel->reloptkind == RELOPT_BASEREL);
    2002+
    /* Now translate for this child */
    2003+
    return adjust_appendrel_attrs(root, node, appinfo);
    2004+
    }

    ‎src/backend/optimizer/util/relnode.c

    Lines changed: 58 additions & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -713,7 +713,8 @@ build_empty_join_rel(PlannerInfo *root)
    713713
    * Get the AppendRelInfo associated with an appendrel child rel.
    714714
    *
    715715
    * This search could be eliminated by storing a link in child RelOptInfos,
    716-
    * but for now it doesn't seem performance-critical.
    716+
    * but for now it doesn't seem performance-critical. (Also, it might be
    717+
    * difficult to maintain such a link during mutation of the append_rel_list.)
    717718
    */
    718719
    AppendRelInfo *
    719720
    find_childrel_appendrelinfo(PlannerInfo *root, RelOptInfo *rel)
    @@ -737,6 +738,62 @@ find_childrel_appendrelinfo(PlannerInfo *root, RelOptInfo *rel)
    737738
    }
    738739

    739740

    741+
    /*
    742+
    * find_childrel_top_parent
    743+
    * Fetch the topmost appendrel parent rel of an appendrel child rel.
    744+
    *
    745+
    * Since appendrels can be nested, a child could have multiple levels of
    746+
    * appendrel ancestors. This function locates the topmost ancestor,
    747+
    * which will be a regular baserel not an otherrel.
    748+
    */
    749+
    RelOptInfo *
    750+
    find_childrel_top_parent(PlannerInfo *root, RelOptInfo *rel)
    751+
    {
    752+
    do
    753+
    {
    754+
    AppendRelInfo *appinfo = find_childrel_appendrelinfo(root, rel);
    755+
    Index prelid = appinfo->parent_relid;
    756+
    757+
    /* traverse up to the parent rel, loop if it's also a child rel */
    758+
    rel = find_base_rel(root, prelid);
    759+
    } while (rel->reloptkind == RELOPT_OTHER_MEMBER_REL);
    760+
    761+
    Assert(rel->reloptkind == RELOPT_BASEREL);
    762+
    763+
    return rel;
    764+
    }
    765+
    766+
    767+
    /*
    768+
    * find_childrel_parents
    769+
    * Compute the set of parent relids of an appendrel child rel.
    770+
    *
    771+
    * Since appendrels can be nested, a child could have multiple levels of
    772+
    * appendrel ancestors. This function computes a Relids set of all the
    773+
    * parent relation IDs.
    774+
    */
    775+
    Relids
    776+
    find_childrel_parents(PlannerInfo *root, RelOptInfo *rel)
    777+
    {
    778+
    Relids result = NULL;
    779+
    780+
    do
    781+
    {
    782+
    AppendRelInfo *appinfo = find_childrel_appendrelinfo(root, rel);
    783+
    Index prelid = appinfo->parent_relid;
    784+
    785+
    result = bms_add_member(result, prelid);
    786+
    787+
    /* traverse up to the parent rel, loop if it's also a child rel */
    788+
    rel = find_base_rel(root, prelid);
    789+
    } while (rel->reloptkind == RELOPT_OTHER_MEMBER_REL);
    790+
    791+
    Assert(rel->reloptkind == RELOPT_BASEREL);
    792+
    793+
    return result;
    794+
    }
    795+
    796+
    740797
    /*
    741798
    * get_baserel_parampathinfo
    742799
    * Get the ParamPathInfo for a parameterized path for a base relation,

    ‎src/include/optimizer/pathnode.h

    Lines changed: 2 additions & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -145,6 +145,8 @@ extern RelOptInfo *build_join_rel(PlannerInfo *root,
    145145
    extern RelOptInfo *build_empty_join_rel(PlannerInfo *root);
    146146
    extern AppendRelInfo *find_childrel_appendrelinfo(PlannerInfo *root,
    147147
    RelOptInfo *rel);
    148+
    extern RelOptInfo *find_childrel_top_parent(PlannerInfo *root, RelOptInfo *rel);
    149+
    extern Relids find_childrel_parents(PlannerInfo *root, RelOptInfo *rel);
    148150
    extern ParamPathInfo *get_baserel_parampathinfo(PlannerInfo *root,
    149151
    RelOptInfo *baserel,
    150152
    Relids required_outer);

    ‎src/include/optimizer/prep.h

    Lines changed: 3 additions & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -58,4 +58,7 @@ extern void expand_inherited_tables(PlannerInfo *root);
    5858
    extern Node *adjust_appendrel_attrs(PlannerInfo *root, Node *node,
    5959
    AppendRelInfo *appinfo);
    6060

    61+
    extern Node *adjust_appendrel_attrs_multilevel(PlannerInfo *root, Node *node,
    62+
    RelOptInfo *child_rel);
    63+
    6164
    #endif /* PREP_H */

    0 commit comments

    Comments
     (0)
    0