diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2015-08-01 20:57:41 -0400 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2015-08-01 20:57:41 -0400 |
commit | f69b4b9495269cc4957bac0f10aaada4d4cfa61e (patch) | |
tree | 862af561088a33757b1376af1eee1c96c4b82cae /src/backend/optimizer/path/joinrels.c | |
parent | dea1491ffb448d20764a5f2cec8ae33b64dd39f8 (diff) | |
download | postgresql-f69b4b9495269cc4957bac0f10aaada4d4cfa61e.tar.gz postgresql-f69b4b9495269cc4957bac0f10aaada4d4cfa61e.zip |
Fix some planner issues with degenerate outer join clauses.
An outer join clause that didn't actually reference the RHS (perhaps only
after constant-folding) could confuse the join order enforcement logic,
leading to wrong query results. Also, nested occurrences of such things
could trigger an Assertion that on reflection seems incorrect.
Per fuzz testing by Andreas Seltenreich. The practical use of such cases
seems thin enough that it's not too surprising we've not heard field
reports about it.
This has been broken for a long time, so back-patch to all active branches.
Diffstat (limited to 'src/backend/optimizer/path/joinrels.c')
-rw-r--r-- | src/backend/optimizer/path/joinrels.c | 26 |
1 files changed, 17 insertions, 9 deletions
diff --git a/src/backend/optimizer/path/joinrels.c b/src/backend/optimizer/path/joinrels.c index b6c9494fed6..02c7c5ea864 100644 --- a/src/backend/optimizer/path/joinrels.c +++ b/src/backend/optimizer/path/joinrels.c @@ -467,20 +467,26 @@ join_is_legal(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2, } else { + /* + * Otherwise, the proposed join overlaps the RHS but isn't a valid + * implementation of this SJ. It might still be a legal join, + * however, if it does not overlap the LHS. + */ + if (bms_overlap(joinrelids, sjinfo->min_lefthand)) + return false; + /*---------- - * Otherwise, the proposed join overlaps the RHS but isn't - * a valid implementation of this SJ. It might still be - * 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 - * SJs that have been determined to commute with this one. + * If both inputs overlap the RHS, assume that it's OK. Since the + * inputs presumably got past this function's checks previously, + * their violations of the RHS boundary must represent SJs 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 SJ. We have to allow this to @@ -496,11 +502,13 @@ join_is_legal(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2, bms_overlap(rel1->relids, sjinfo->min_righthand) && bms_overlap(rel2->relids, sjinfo->min_righthand)) { - /* seems OK */ - Assert(!bms_overlap(joinrelids, sjinfo->min_lefthand)); + /* both overlap; assume OK */ } else + { + /* one overlaps, the other doesn't (or it's a semijoin) */ is_valid_inner = false; + } } } |