Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 5a6c168

Browse files
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 0c013e0 commit 5a6c168

File tree

10 files changed

+717
-27
lines changed

10 files changed

+717
-27
lines changed

src/backend/optimizer/path/equivclass.c

+20-18
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))
21552157
continue;
21562158

21572159
eq_op = select_equality_operator(cur_ec,

src/backend/optimizer/path/indxpath.c

+2-7
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

+23
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

+58-1
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

+2
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

+3
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)