diff --git a/src/backend/optimizer/README b/src/backend/optimizer/README index 2e736b6dbe..bd302bc215 100644 --- a/src/backend/optimizer/README +++ b/src/backend/optimizer/README @@ -223,8 +223,8 @@ rels to the OJ's syntactic rels may be legal. Per identities 1 and 2, non-FULL joins can be freely associated into the lefthand side of an OJ, but in general they can't be associated into the righthand side. So the restriction enforced by make_join_rel is that a proposed join -can't join across a RHS boundary (ie, join anything inside the RHS -to anything else) unless the join validly implements some outer join. +can't join a rel within or partly within an RHS boundary to one outside +the boundary, unless the join validly implements some outer join. (To support use of identity 3, we have to allow cases where an apparent violation of a lower OJ's RHS is committed while forming an upper OJ. If this wouldn't in fact be legal, the upper OJ's minimum LHS or RHS diff --git a/src/backend/optimizer/geqo/geqo_eval.c b/src/backend/optimizer/geqo/geqo_eval.c index 67cbefdaa6..847afd465d 100644 --- a/src/backend/optimizer/geqo/geqo_eval.c +++ b/src/backend/optimizer/geqo/geqo_eval.c @@ -6,7 +6,7 @@ * Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/backend/optimizer/geqo/geqo_eval.c,v 1.83 2007/01/05 22:19:30 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/geqo/geqo_eval.c,v 1.84 2007/02/13 02:31:02 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -182,7 +182,7 @@ gimme_tree(Gene *tour, int num_gene, GeqoEvalData *evaldata) * tour other than the one given. To the extent that the heuristics are * helpful, however, this will be a better plan than the raw tour. * - * Also, when a join attempt fails (because of IN-clause constraints), we + * Also, when a join attempt fails (because of OJ or IN constraints), we * may be able to recover and produce a workable plan, where the old code * just had to give up. This case acts the same as a false result from * desirable_join(). @@ -262,9 +262,9 @@ desirable_join(PlannerInfo *root, return true; /* - * Join if the rels are members of the same outer-join side. This is - * needed to ensure that we can find a valid solution in a case where - * an OJ contains a clauseless join. + * Join if the rels overlap the same outer-join side and don't already + * implement the outer join. This is needed to ensure that we can find a + * valid solution in a case where an OJ contains a clauseless join. */ foreach(l, root->oj_info_list) { @@ -273,11 +273,15 @@ desirable_join(PlannerInfo *root, /* ignore full joins --- other mechanisms preserve their ordering */ if (ojinfo->is_full_join) continue; - if (bms_is_subset(outer_rel->relids, ojinfo->min_righthand) && - bms_is_subset(inner_rel->relids, ojinfo->min_righthand)) + if (bms_overlap(outer_rel->relids, ojinfo->min_righthand) && + bms_overlap(inner_rel->relids, ojinfo->min_righthand) && + !bms_overlap(outer_rel->relids, ojinfo->min_lefthand) && + !bms_overlap(inner_rel->relids, ojinfo->min_lefthand)) return true; - if (bms_is_subset(outer_rel->relids, ojinfo->min_lefthand) && - bms_is_subset(inner_rel->relids, ojinfo->min_lefthand)) + if (bms_overlap(outer_rel->relids, ojinfo->min_lefthand) && + bms_overlap(inner_rel->relids, ojinfo->min_lefthand) && + !bms_overlap(outer_rel->relids, ojinfo->min_righthand) && + !bms_overlap(inner_rel->relids, ojinfo->min_righthand)) return true; } diff --git a/src/backend/optimizer/path/joinrels.c b/src/backend/optimizer/path/joinrels.c index 70e16eeb8e..e138f9b0aa 100644 --- a/src/backend/optimizer/path/joinrels.c +++ b/src/backend/optimizer/path/joinrels.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/optimizer/path/joinrels.c,v 1.84 2007/01/20 20:45:39 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/path/joinrels.c,v 1.85 2007/02/13 02:31:03 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -350,8 +350,9 @@ has_join_restriction(PlannerInfo *root, RelOptInfo *rel) /* ignore full joins --- other mechanisms preserve their ordering */ if (ojinfo->is_full_join) continue; - /* anything inside the RHS is definitely restricted */ - if (bms_is_subset(rel->relids, ojinfo->min_righthand)) + /* if it overlaps RHS and isn't yet joined to LHS, it's restricted */ + if (bms_overlap(rel->relids, ojinfo->min_righthand) && + !bms_overlap(rel->relids, ojinfo->min_lefthand)) return true; /* if it's a proper subset of the LHS, it's also restricted */ if (bms_is_subset(rel->relids, ojinfo->min_lefthand) && @@ -468,16 +469,36 @@ make_join_rel(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2) /*---------- * Otherwise, the proposed join overlaps the RHS but isn't * a valid implementation of this OJ. It might still be - * a valid implementation of some other OJ, however. We have - * to allow this to support the associative identity - * (a LJ b on Pab) LJ c ON Pbc = a LJ (b LJ c ON Pbc) on Pab + * a legal join, however. If both inputs overlap the RHS, + * assume that it's OK. Since the inputs presumably got past + * this function's checks previously, they can't overlap the + * LHS and their violations of the RHS boundary must represent + * OJs that have been determined to commute with this one. + * We have to allow this to work correctly in cases like + * (a LEFT JOIN (b JOIN (c LEFT JOIN d))) + * when the c/d join has been determined to commute with the join + * to a, and hence d is not part of min_righthand for the upper + * join. It should be legal to join b to c/d but this will appear + * as a violation of the upper join's RHS. + * Furthermore, if one input overlaps the RHS and the other does + * not, we should still allow the join if it is a valid + * implementation of some other OJ. We have to allow this to + * support the associative identity + * (a LJ b on Pab) LJ c ON Pbc = a LJ (b LJ c ON Pbc) on Pab * since joining B directly to C violates the lower OJ's RHS. * We assume that make_outerjoininfo() set things up correctly - * so that we'll only match to the upper OJ if the transformation - * is valid. Set flag here to check at bottom of loop. + * so that we'll only match to some OJ if the join is valid. + * Set flag here to check at bottom of loop. *---------- */ - is_valid_inner = false; + if (bms_overlap(rel1->relids, ojinfo->min_righthand) && + bms_overlap(rel2->relids, ojinfo->min_righthand)) + { + /* seems OK */ + Assert(!bms_overlap(joinrelids, ojinfo->min_lefthand)); + } + else + is_valid_inner = false; } } diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c index 63b03bd28a..d05705507d 100644 --- a/src/backend/optimizer/plan/initsplan.c +++ b/src/backend/optimizer/plan/initsplan.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/optimizer/plan/initsplan.c,v 1.129 2007/02/01 19:10:26 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/plan/initsplan.c,v 1.130 2007/02/13 02:31:03 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -370,7 +370,9 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join, /* * For an OJ, form the OuterJoinInfo now, because we need the OJ's - * semantic scope (ojscope) to pass to distribute_qual_to_rels. + * semantic scope (ojscope) to pass to distribute_qual_to_rels. But + * we mustn't add it to oj_info_list just yet, because we don't want + * distribute_qual_to_rels to think it is an outer join below us. */ if (j->jointype != JOIN_INNER) { @@ -451,8 +453,13 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join, * the caller, so that left_rels is always the nonnullable side. Hence * we need only distinguish the LEFT and FULL cases. * - * The node should eventually be put into root->oj_info_list, but we + * The node should eventually be appended to root->oj_info_list, but we * do not do that here. + * + * Note: we assume that this function is invoked bottom-up, so that + * root->oj_info_list already contains entries for all outer joins that are + * syntactically below this one; and indeed that oj_info_list is ordered + * with syntactically lower joins listed first. */ static OuterJoinInfo * make_outerjoininfo(PlannerInfo *root,