diff options
| -rw-r--r-- | src/backend/optimizer/plan/analyzejoins.c | 77 | ||||
| -rw-r--r-- | src/test/regress/expected/join.out | 20 | ||||
| -rw-r--r-- | src/test/regress/sql/join.sql | 8 |
3 files changed, 90 insertions, 15 deletions
diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c index 283539e53c4..7fd11df9afb 100644 --- a/src/backend/optimizer/plan/analyzejoins.c +++ b/src/backend/optimizer/plan/analyzejoins.c @@ -29,6 +29,7 @@ #include "optimizer/pathnode.h" #include "optimizer/paths.h" #include "optimizer/planmain.h" +#include "optimizer/restrictinfo.h" #include "optimizer/tlist.h" #include "utils/lsyscache.h" @@ -36,6 +37,8 @@ static bool join_is_removable(PlannerInfo *root, SpecialJoinInfo *sjinfo); static void remove_rel_from_query(PlannerInfo *root, int relid, int ojrelid, Relids joinrelids); +static void remove_rel_from_restrictinfo(RestrictInfo *rinfo, + int relid, int ojrelid); static List *remove_rel_from_joinlist(List *joinlist, int relid, int *nremoved); static bool rel_supports_distinctness(PlannerInfo *root, RelOptInfo *rel); static bool rel_is_distinct_for(PlannerInfo *root, RelOptInfo *rel, @@ -470,24 +473,12 @@ remove_rel_from_query(PlannerInfo *root, int relid, int ojrelid, { /* * There might be references to relid or ojrelid in the - * clause_relids as a consequence of PHVs having ph_eval_at sets + * RestrictInfo, as a consequence of PHVs having ph_eval_at sets * that include those. We already checked above that any such PHV * is safe, so we can just drop those references. - * - * The clause_relids probably aren't shared with anything else, - * but let's copy them just to be sure. */ - rinfo->clause_relids = bms_copy(rinfo->clause_relids); - rinfo->clause_relids = bms_del_member(rinfo->clause_relids, - relid); - rinfo->clause_relids = bms_del_member(rinfo->clause_relids, - ojrelid); - /* Likewise for required_relids */ - rinfo->required_relids = bms_copy(rinfo->required_relids); - rinfo->required_relids = bms_del_member(rinfo->required_relids, - relid); - rinfo->required_relids = bms_del_member(rinfo->required_relids, - ojrelid); + remove_rel_from_restrictinfo(rinfo, relid, ojrelid); + /* Now throw it back into the joininfo lists */ distribute_restrictinfo_to_rels(root, rinfo); } } @@ -499,6 +490,62 @@ remove_rel_from_query(PlannerInfo *root, int relid, int ojrelid, } /* + * Remove any references to relid or ojrelid from the RestrictInfo. + * + * We only bother to clean out bits in clause_relids and required_relids, + * not nullingrel bits in contained Vars and PHVs. (This might have to be + * improved sometime.) However, if the RestrictInfo contains an OR clause + * we have to also clean up the sub-clauses. + */ +static void +remove_rel_from_restrictinfo(RestrictInfo *rinfo, int relid, int ojrelid) +{ + /* + * The clause_relids probably aren't shared with anything else, but let's + * copy them just to be sure. + */ + rinfo->clause_relids = bms_copy(rinfo->clause_relids); + rinfo->clause_relids = bms_del_member(rinfo->clause_relids, relid); + rinfo->clause_relids = bms_del_member(rinfo->clause_relids, ojrelid); + /* Likewise for required_relids */ + rinfo->required_relids = bms_copy(rinfo->required_relids); + rinfo->required_relids = bms_del_member(rinfo->required_relids, relid); + rinfo->required_relids = bms_del_member(rinfo->required_relids, ojrelid); + + /* If it's an OR, recurse to clean up sub-clauses */ + if (restriction_is_or_clause(rinfo)) + { + ListCell *lc; + + Assert(is_orclause(rinfo->orclause)); + foreach(lc, ((BoolExpr *) rinfo->orclause)->args) + { + Node *orarg = (Node *) lfirst(lc); + + /* OR arguments should be ANDs or sub-RestrictInfos */ + if (is_andclause(orarg)) + { + List *andargs = ((BoolExpr *) orarg)->args; + ListCell *lc2; + + foreach(lc2, andargs) + { + RestrictInfo *rinfo2 = lfirst_node(RestrictInfo, lc2); + + remove_rel_from_restrictinfo(rinfo2, relid, ojrelid); + } + } + else + { + RestrictInfo *rinfo2 = castNode(RestrictInfo, orarg); + + remove_rel_from_restrictinfo(rinfo2, relid, ojrelid); + } + } + } +} + +/* * Remove any occurrences of the target relid from a joinlist structure. * * It's easiest to build a whole new list structure, so we handle it that diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index 98d611412df..2f1f8b8dbe6 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -5344,6 +5344,26 @@ SELECT q2 FROM One-Time Filter: false (9 rows) +-- join removal bug #17786: check that OR conditions are cleaned up +EXPLAIN (COSTS OFF) +SELECT f1, x +FROM int4_tbl + JOIN ((SELECT 42 AS x FROM int8_tbl LEFT JOIN innertab ON q1 = id) AS ss1 + RIGHT JOIN tenk1 ON NULL) + ON tenk1.unique1 = ss1.x OR tenk1.unique2 = ss1.x; + QUERY PLAN +-------------------------------------------------------------------------- + Nested Loop + -> Seq Scan on int4_tbl + -> Materialize + -> Nested Loop Left Join + Join Filter: NULL::boolean + Filter: ((tenk1.unique1 = (42)) OR (tenk1.unique2 = (42))) + -> Seq Scan on tenk1 + -> Result + One-Time Filter: false +(9 rows) + rollback; -- another join removal bug: we must clean up correctly when removing a PHV begin; diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index e50a7696069..400c16958f3 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -1955,6 +1955,14 @@ SELECT q2 FROM RIGHT JOIN int4_tbl ON NULL WHERE x >= x; +-- join removal bug #17786: check that OR conditions are cleaned up +EXPLAIN (COSTS OFF) +SELECT f1, x +FROM int4_tbl + JOIN ((SELECT 42 AS x FROM int8_tbl LEFT JOIN innertab ON q1 = id) AS ss1 + RIGHT JOIN tenk1 ON NULL) + ON tenk1.unique1 = ss1.x OR tenk1.unique2 = ss1.x; + rollback; -- another join removal bug: we must clean up correctly when removing a PHV |