diff options
| -rw-r--r-- | src/backend/nodes/copyfuncs.c | 4 | ||||
| -rw-r--r-- | src/backend/nodes/equalfuncs.c | 4 | ||||
| -rw-r--r-- | src/backend/nodes/outfuncs.c | 4 | ||||
| -rw-r--r-- | src/backend/optimizer/plan/initsplan.c | 149 | ||||
| -rw-r--r-- | src/include/nodes/relation.h | 9 | ||||
| -rw-r--r-- | src/test/regress/expected/join.out | 24 | ||||
| -rw-r--r-- | src/test/regress/expected/join_1.out | 24 | ||||
| -rw-r--r-- | src/test/regress/sql/join.sql | 22 |
8 files changed, 189 insertions, 51 deletions
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 3bc6afe1df8..932da9f4d23 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -15,7 +15,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.380 2007/07/17 05:02:01 neilc Exp $ + * $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.381 2007/08/31 01:44:05 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1453,6 +1453,8 @@ _copyOuterJoinInfo(OuterJoinInfo *from) COPY_BITMAPSET_FIELD(min_lefthand); COPY_BITMAPSET_FIELD(min_righthand); + COPY_BITMAPSET_FIELD(syn_lefthand); + COPY_BITMAPSET_FIELD(syn_righthand); COPY_SCALAR_FIELD(is_full_join); COPY_SCALAR_FIELD(lhs_strict); COPY_SCALAR_FIELD(delay_upper_joins); diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 317a5a29959..68a50354760 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -18,7 +18,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.311 2007/07/17 05:02:01 neilc Exp $ + * $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.312 2007/08/31 01:44:05 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -706,6 +706,8 @@ _equalOuterJoinInfo(OuterJoinInfo *a, OuterJoinInfo *b) { COMPARE_BITMAPSET_FIELD(min_lefthand); COMPARE_BITMAPSET_FIELD(min_righthand); + COMPARE_BITMAPSET_FIELD(syn_lefthand); + COMPARE_BITMAPSET_FIELD(syn_righthand); COMPARE_SCALAR_FIELD(is_full_join); COMPARE_SCALAR_FIELD(lhs_strict); COMPARE_SCALAR_FIELD(delay_upper_joins); diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index 2d2b229c9e8..db0bf7c050d 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.313 2007/07/17 05:02:01 neilc Exp $ + * $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.314 2007/08/31 01:44:05 tgl Exp $ * * NOTES * Every node type that can appear in stored rules' parsetrees *must* @@ -1471,6 +1471,8 @@ _outOuterJoinInfo(StringInfo str, OuterJoinInfo *node) WRITE_BITMAPSET_FIELD(min_lefthand); WRITE_BITMAPSET_FIELD(min_righthand); + WRITE_BITMAPSET_FIELD(syn_lefthand); + WRITE_BITMAPSET_FIELD(syn_righthand); WRITE_BOOL_FIELD(is_full_join); WRITE_BOOL_FIELD(lhs_strict); WRITE_BOOL_FIELD(delay_upper_joins); diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c index fe08dfcc9c6..b71cf6dae2e 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.132 2007/05/22 23:23:56 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/plan/initsplan.c,v 1.133 2007/08/31 01:44:05 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -38,9 +38,11 @@ int join_collapse_limit; static List *deconstruct_recurse(PlannerInfo *root, Node *jtnode, - bool below_outer_join, Relids *qualscope); + bool below_outer_join, + Relids *qualscope, Relids *inner_join_rels); static OuterJoinInfo *make_outerjoininfo(PlannerInfo *root, Relids left_rels, Relids right_rels, + Relids inner_join_rels, bool is_full_join, Node *clause); static void distribute_qual_to_rels(PlannerInfo *root, Node *clause, bool is_deduced, @@ -204,13 +206,14 @@ List * deconstruct_jointree(PlannerInfo *root) { Relids qualscope; + Relids inner_join_rels; /* Start recursion at top of jointree */ Assert(root->parse->jointree != NULL && IsA(root->parse->jointree, FromExpr)); return deconstruct_recurse(root, (Node *) root->parse->jointree, false, - &qualscope); + &qualscope, &inner_join_rels); } /* @@ -224,20 +227,24 @@ deconstruct_jointree(PlannerInfo *root) * Outputs: * *qualscope gets the set of base Relids syntactically included in this * jointree node (do not modify or free this, as it may also be pointed - * to by RestrictInfo nodes) + * to by RestrictInfo and OuterJoinInfo nodes) + * *inner_join_rels gets the set of base Relids syntactically included in + * inner joins appearing at or below this jointree node (do not modify + * or free this, either) * Return value is the appropriate joinlist for this jointree node * * In addition, entries will be added to root->oj_info_list for outer joins. */ static List * deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join, - Relids *qualscope) + Relids *qualscope, Relids *inner_join_rels) { List *joinlist; if (jtnode == NULL) { *qualscope = NULL; + *inner_join_rels = NULL; return NIL; } if (IsA(jtnode, RangeTblRef)) @@ -246,6 +253,8 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join, /* No quals to deal with, just return correct result */ *qualscope = bms_make_singleton(varno); + /* A single baserel does not create an inner join */ + *inner_join_rels = NULL; joinlist = list_make1(jtnode); } else if (IsA(jtnode, FromExpr)) @@ -261,6 +270,7 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join, * subproblems, since that won't lengthen the joinlist anyway. */ *qualscope = NULL; + *inner_join_rels = NULL; joinlist = NIL; remaining = list_length(f->fromlist); foreach(l, f->fromlist) @@ -271,7 +281,8 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join, sub_joinlist = deconstruct_recurse(root, lfirst(l), below_outer_join, - &sub_qualscope); + &sub_qualscope, + inner_join_rels); *qualscope = bms_add_members(*qualscope, sub_qualscope); sub_members = list_length(sub_joinlist); remaining--; @@ -283,6 +294,16 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join, } /* + * A FROM with more than one list element is an inner join subsuming + * all below it, so we should report inner_join_rels = qualscope. + * If there was exactly one element, we should (and already did) report + * whatever its inner_join_rels were. If there were no elements + * (is that possible?) the initialization before the loop fixed it. + */ + if (list_length(f->fromlist) > 1) + *inner_join_rels = *qualscope; + + /* * Now process the top-level quals. */ foreach(l, (List *) f->quals) @@ -295,6 +316,8 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join, JoinExpr *j = (JoinExpr *) jtnode; Relids leftids, rightids, + left_inners, + right_inners, nonnullable_rels, ojscope; List *leftjoinlist, @@ -319,32 +342,35 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join, case JOIN_INNER: leftjoinlist = deconstruct_recurse(root, j->larg, below_outer_join, - &leftids); + &leftids, &left_inners); rightjoinlist = deconstruct_recurse(root, j->rarg, below_outer_join, - &rightids); + &rightids, &right_inners); *qualscope = bms_union(leftids, rightids); + *inner_join_rels = *qualscope; /* Inner join adds no restrictions for quals */ nonnullable_rels = NULL; break; case JOIN_LEFT: leftjoinlist = deconstruct_recurse(root, j->larg, below_outer_join, - &leftids); + &leftids, &left_inners); rightjoinlist = deconstruct_recurse(root, j->rarg, true, - &rightids); + &rightids, &right_inners); *qualscope = bms_union(leftids, rightids); + *inner_join_rels = bms_union(left_inners, right_inners); nonnullable_rels = leftids; break; case JOIN_FULL: leftjoinlist = deconstruct_recurse(root, j->larg, true, - &leftids); + &leftids, &left_inners); rightjoinlist = deconstruct_recurse(root, j->rarg, true, - &rightids); + &rightids, &right_inners); *qualscope = bms_union(leftids, rightids); + *inner_join_rels = bms_union(left_inners, right_inners); /* each side is both outer and inner */ nonnullable_rels = *qualscope; break; @@ -352,11 +378,12 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join, /* notice we switch leftids and rightids */ leftjoinlist = deconstruct_recurse(root, j->larg, true, - &rightids); + &rightids, &right_inners); rightjoinlist = deconstruct_recurse(root, j->rarg, below_outer_join, - &leftids); + &leftids, &left_inners); *qualscope = bms_union(leftids, rightids); + *inner_join_rels = bms_union(left_inners, right_inners); nonnullable_rels = leftids; break; default: @@ -375,8 +402,11 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join, */ if (j->jointype != JOIN_INNER) { - ojinfo = make_outerjoininfo(root, leftids, rightids, - (j->jointype == JOIN_FULL), j->quals); + ojinfo = make_outerjoininfo(root, + leftids, rightids, + *inner_join_rels, + (j->jointype == JOIN_FULL), + j->quals); ojscope = bms_union(ojinfo->min_lefthand, ojinfo->min_righthand); } else @@ -445,6 +475,7 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join, * Inputs: * left_rels: the base Relids syntactically on outer side of join * right_rels: the base Relids syntactically on inner side of join + * inner_join_rels: base Relids participating in inner joins below this one * is_full_join: what it says * clause: the outer join's join condition * @@ -457,17 +488,19 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join, * * 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. + * syntactically below this one. */ static OuterJoinInfo * make_outerjoininfo(PlannerInfo *root, Relids left_rels, Relids right_rels, + Relids inner_join_rels, bool is_full_join, Node *clause) { OuterJoinInfo *ojinfo = makeNode(OuterJoinInfo); Relids clause_relids; Relids strict_relids; + Relids min_lefthand; + Relids min_righthand; ListCell *l; /* @@ -497,6 +530,8 @@ make_outerjoininfo(PlannerInfo *root, ojinfo->delay_upper_joins = false; /* If it's a full join, no need to be very smart */ + ojinfo->syn_lefthand = left_rels; + ojinfo->syn_righthand = right_rels; ojinfo->is_full_join = is_full_join; if (is_full_join) { @@ -521,21 +556,17 @@ make_outerjoininfo(PlannerInfo *root, ojinfo->lhs_strict = bms_overlap(strict_relids, left_rels); /* - * Required LHS is basically the LHS rels mentioned in the clause... but - * if there aren't any, punt and make it the full LHS, to avoid having an - * empty min_lefthand which will confuse later processing. (We don't try - * to be smart about such cases, just correct.) We may have to add more - * rels based on lower outer joins; see below. + * Required LHS always includes the LHS rels mentioned in the clause. + * We may have to add more rels based on lower outer joins; see below. */ - ojinfo->min_lefthand = bms_intersect(clause_relids, left_rels); - if (bms_is_empty(ojinfo->min_lefthand)) - ojinfo->min_lefthand = bms_copy(left_rels); + min_lefthand = bms_intersect(clause_relids, left_rels); /* - * Required RHS is normally the full set of RHS rels. Sometimes we can - * exclude some, see below. + * Similarly for required RHS. But here, we must also include any lower + * inner joins, to ensure we don't try to commute with any of them. */ - ojinfo->min_righthand = bms_copy(right_rels); + min_righthand = bms_int_members(bms_union(clause_relids, inner_join_rels), + right_rels); foreach(l, root->oj_info_list) { @@ -548,23 +579,29 @@ make_outerjoininfo(PlannerInfo *root, /* * For a lower OJ in our LHS, if our join condition uses the lower * join's RHS and is not strict for that rel, we must preserve the - * ordering of the two OJs, so add lower OJ's full required relset to - * min_lefthand. + * ordering of the two OJs, so add lower OJ's full syntactic relset to + * min_lefthand. (We must use its full syntactic relset, not just + * its min_lefthand + min_righthand. This is because there might + * be other OJs below this one that this one can commute with, + * but we cannot commute with them if we don't with this one.) + * + * Note: I believe we have to insist on being strict for at least one + * rel in the lower OJ's min_righthand, not its whole syn_righthand. */ - if (bms_overlap(ojinfo->min_lefthand, otherinfo->min_righthand) && + if (bms_overlap(left_rels, otherinfo->syn_righthand) && !bms_overlap(strict_relids, otherinfo->min_righthand)) { - ojinfo->min_lefthand = bms_add_members(ojinfo->min_lefthand, - otherinfo->min_lefthand); - ojinfo->min_lefthand = bms_add_members(ojinfo->min_lefthand, - otherinfo->min_righthand); + min_lefthand = bms_add_members(min_lefthand, + otherinfo->syn_lefthand); + min_lefthand = bms_add_members(min_lefthand, + otherinfo->syn_righthand); } /* * For a lower OJ in our RHS, if our join condition does not use the * lower join's RHS and the lower OJ's join condition is strict, we - * can interchange the ordering of the two OJs, so exclude the lower - * RHS from our min_righthand. + * can interchange the ordering of the two OJs; otherwise we must + * add lower OJ's full syntactic relset to min_righthand. * * Here, we have to consider that "our join condition" includes * any clauses that syntactically appeared above the lower OJ and @@ -577,20 +614,38 @@ make_outerjoininfo(PlannerInfo *root, * effect is therefore sufficiently represented by the * delay_upper_joins flag saved for us by check_outerjoin_delay. */ - if (bms_overlap(ojinfo->min_righthand, otherinfo->min_righthand) && - !bms_overlap(clause_relids, otherinfo->min_righthand) && - otherinfo->lhs_strict && !otherinfo->delay_upper_joins) + if (bms_overlap(right_rels, otherinfo->syn_righthand)) { - ojinfo->min_righthand = bms_del_members(ojinfo->min_righthand, - otherinfo->min_righthand); + if (bms_overlap(clause_relids, otherinfo->syn_righthand) || + !otherinfo->lhs_strict || otherinfo->delay_upper_joins) + { + min_righthand = bms_add_members(min_righthand, + otherinfo->syn_lefthand); + min_righthand = bms_add_members(min_righthand, + otherinfo->syn_righthand); + } } } - /* Neither set should be empty, else we might get confused later */ - Assert(!bms_is_empty(ojinfo->min_lefthand)); - Assert(!bms_is_empty(ojinfo->min_righthand)); + /* + * If we found nothing to put in min_lefthand, punt and make it the full + * LHS, to avoid having an empty min_lefthand which will confuse later + * processing. (We don't try to be smart about such cases, just correct.) + * Likewise for min_righthand. + */ + if (bms_is_empty(min_lefthand)) + min_lefthand = bms_copy(left_rels); + if (bms_is_empty(min_righthand)) + min_righthand = bms_copy(right_rels); + + /* Now they'd better be nonempty */ + Assert(!bms_is_empty(min_lefthand)); + Assert(!bms_is_empty(min_righthand)); /* Shouldn't overlap either */ - Assert(!bms_overlap(ojinfo->min_lefthand, ojinfo->min_righthand)); + Assert(!bms_overlap(min_lefthand, min_righthand)); + + ojinfo->min_lefthand = min_lefthand; + ojinfo->min_righthand = min_righthand; return ojinfo; } diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h index 4a2f2649691..ab26491a629 100644 --- a/src/include/nodes/relation.h +++ b/src/include/nodes/relation.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/nodes/relation.h,v 1.144 2007/05/31 16:57:34 tgl Exp $ + * $PostgreSQL: pgsql/src/include/nodes/relation.h,v 1.145 2007/08/31 01:44:06 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1044,6 +1044,11 @@ typedef struct InnerIndexscanInfo * It is not valid for either min_lefthand or min_righthand to be empty sets; * if they were, this would break the logic that enforces join order. * + * syn_lefthand and syn_righthand are the sets of base relids that are + * syntactically below this outer join. (These are needed to help compute + * min_lefthand and min_righthand for higher joins, but are not used + * thereafter.) + * * delay_upper_joins is set TRUE if we detect a pushed-down clause that has * to be evaluated after this join is formed (because it references the RHS). * Any outer joins that have such a clause and this join in their RHS cannot @@ -1061,6 +1066,8 @@ typedef struct OuterJoinInfo NodeTag type; Relids min_lefthand; /* base relids in minimum LHS for join */ Relids min_righthand; /* base relids in minimum RHS for join */ + Relids syn_lefthand; /* base relids syntactically within LHS */ + Relids syn_righthand; /* base relids syntactically within RHS */ bool is_full_join; /* it's a FULL OUTER JOIN */ bool lhs_strict; /* joinclause is strict for some LHS rel */ bool delay_upper_joins; /* can't commute with upper RHS */ diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index 3b7bdd9cbb3..3dd4f5ed7d0 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -2266,3 +2266,27 @@ select * from tt5,tt6 where tt5.f1 = tt6.f1 and tt5.f1 = tt5.f2 - tt6.f2; 1 | 10 | 1 | 9 (1 row) +-- +-- regression test for problems of the sort depicted in bug #3588 +-- +create temp table xx (pkxx int); +create temp table yy (pkyy int, pkxx int); +insert into xx values (1); +insert into xx values (2); +insert into xx values (3); +insert into yy values (101, 1); +insert into yy values (201, 2); +insert into yy values (301, NULL); +select yy.pkyy as yy_pkyy, yy.pkxx as yy_pkxx, yya.pkyy as yya_pkyy, + xxa.pkxx as xxa_pkxx, xxb.pkxx as xxb_pkxx +from yy + left join (SELECT * FROM yy where pkyy = 101) as yya ON yy.pkyy = yya.pkyy + left join xx xxa on yya.pkxx = xxa.pkxx + left join xx xxb on coalesce (xxa.pkxx, 1) = xxb.pkxx; + yy_pkyy | yy_pkxx | yya_pkyy | xxa_pkxx | xxb_pkxx +---------+---------+----------+----------+---------- + 101 | 1 | 101 | 1 | 1 + 201 | 2 | | | 1 + 301 | | | | 1 +(3 rows) + diff --git a/src/test/regress/expected/join_1.out b/src/test/regress/expected/join_1.out index f1bf4b9af26..9e10c73acb7 100644 --- a/src/test/regress/expected/join_1.out +++ b/src/test/regress/expected/join_1.out @@ -2266,3 +2266,27 @@ select * from tt5,tt6 where tt5.f1 = tt6.f1 and tt5.f1 = tt5.f2 - tt6.f2; 1 | 10 | 1 | 9 (1 row) +-- +-- regression test for problems of the sort depicted in bug #3588 +-- +create temp table xx (pkxx int); +create temp table yy (pkyy int, pkxx int); +insert into xx values (1); +insert into xx values (2); +insert into xx values (3); +insert into yy values (101, 1); +insert into yy values (201, 2); +insert into yy values (301, NULL); +select yy.pkyy as yy_pkyy, yy.pkxx as yy_pkxx, yya.pkyy as yya_pkyy, + xxa.pkxx as xxa_pkxx, xxb.pkxx as xxb_pkxx +from yy + left join (SELECT * FROM yy where pkyy = 101) as yya ON yy.pkyy = yya.pkyy + left join xx xxa on yya.pkxx = xxa.pkxx + left join xx xxb on coalesce (xxa.pkxx, 1) = xxb.pkxx; + yy_pkyy | yy_pkxx | yya_pkyy | xxa_pkxx | xxb_pkxx +---------+---------+----------+----------+---------- + 101 | 1 | 101 | 1 | 1 + 201 | 2 | | | 1 + 301 | | | | 1 +(3 rows) + diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index ad8957cd6e3..4efb3c72836 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -440,3 +440,25 @@ insert into tt6 values(1, 2); insert into tt6 values(2, 9); select * from tt5,tt6 where tt5.f1 = tt6.f1 and tt5.f1 = tt5.f2 - tt6.f2; + +-- +-- regression test for problems of the sort depicted in bug #3588 +-- + +create temp table xx (pkxx int); +create temp table yy (pkyy int, pkxx int); + +insert into xx values (1); +insert into xx values (2); +insert into xx values (3); + +insert into yy values (101, 1); +insert into yy values (201, 2); +insert into yy values (301, NULL); + +select yy.pkyy as yy_pkyy, yy.pkxx as yy_pkxx, yya.pkyy as yya_pkyy, + xxa.pkxx as xxa_pkxx, xxb.pkxx as xxb_pkxx +from yy + left join (SELECT * FROM yy where pkyy = 101) as yya ON yy.pkyy = yya.pkyy + left join xx xxa on yya.pkxx = xxa.pkxx + left join xx xxb on coalesce (xxa.pkxx, 1) = xxb.pkxx; |