From fc8ed6362404c2abfb87f9f291aa9d19b8323d8c Mon Sep 17 00:00:00 2001 From: Richard Guo Date: Wed, 2 Sep 2026 13:40:14 +0900 Subject: [PATCH] Remove quals made redundant by reducing outer joins to antijoins When reduce_outer_joins reduces an outer join to an antijoin, any IS NULL qual on a Var from the antijoin's nullable side is necessarily true. Previously, such quals were discarded later in distribute_qual_to_rels, mainly to avoid bogus selectivity estimates. But that discard was incomplete: the qual remained in the jointree, while its Vars were not counted in attr_needed. Since commit 2ebf25e7d, join removal edits the jointree and expects it to contain no other references to a removed rel, so it could remove a rel that such a discarded qual still references, and then trip an assertion on the qual's stale Var. To fix, move this processing to an earlier phase: such quals are now removed from the jointree by reduce_outer_joins itself. This way later phases see a consistent query tree, and check_redundant_nullability_qual is no longer needed, so remove it. Back-patch to v16, as with commit 2ebf25e7d. Reported-by: Tender Wang Author: Richard Guo Reviewed-by: Tom Lane Discussion: https://postgr.es/m/CAHewXNk8b0TsSy4dL=CO7FXL2W3WBm0BcdP-zwNJePa-Qj4HzA@mail.gmail.com Backpatch-through: 16 --- src/backend/optimizer/plan/initsplan.c | 61 --------------- src/backend/optimizer/prep/prepjointree.c | 93 +++++++++++++++++++++-- src/backend/optimizer/util/clauses.c | 11 +-- src/test/regress/expected/join.out | 40 ++++++++++ src/test/regress/sql/join.sql | 25 ++++++ 5 files changed, 157 insertions(+), 73 deletions(-) diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c index b31d8921211..37d266a7fde 100644 --- a/src/backend/optimizer/plan/initsplan.c +++ b/src/backend/optimizer/plan/initsplan.c @@ -126,7 +126,6 @@ static void distribute_qual_to_rels(PlannerInfo *root, Node *clause, bool has_clone, bool is_clone, List **postponed_oj_qual_list); -static bool check_redundant_nullability_qual(PlannerInfo *root, Node *clause); static Relids get_join_domain_min_rels(PlannerInfo *root, Relids domain_relids); static void check_mergejoinable(RestrictInfo *restrictinfo); static void check_hashjoinable(RestrictInfo *restrictinfo); @@ -2189,10 +2188,6 @@ distribute_quals_to_rels(PlannerInfo *root, List *clauses, * 'qualscope' identifies what level of JOIN the qual came from syntactically. * 'ojscope' is needed if we decide to force the qual up to the outer-join * level, which will be ojscope not necessarily qualscope. - * - * At the time this is called, root->join_info_list must contain entries for - * at least those special joins that are syntactically below this qual. - * (We now need that only for detection of redundant IS NULL quals.) */ static void distribute_qual_to_rels(PlannerInfo *root, Node *clause, @@ -2396,15 +2391,6 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause, */ is_pushed_down = true; - /* - * It's possible that this is an IS NULL clause that's redundant with - * a lower antijoin; if so we can just discard it. We need not test - * in any of the other cases, because this will only be possible for - * pushed-down clauses. - */ - if (check_redundant_nullability_qual(root, clause)) - return; - /* Feed qual to the equivalence machinery, if allowed by caller */ maybe_equivalence = allow_equivalence; @@ -2571,53 +2557,6 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause, distribute_restrictinfo_to_rels(root, restrictinfo); } -/* - * check_redundant_nullability_qual - * Check to see if the qual is an IS NULL qual that is redundant with - * a lower JOIN_ANTI join. - * - * We want to suppress redundant IS NULL quals, not so much to save cycles - * as to avoid generating bogus selectivity estimates for them. So if - * redundancy is detected here, distribute_qual_to_rels() just throws away - * the qual. - */ -static bool -check_redundant_nullability_qual(PlannerInfo *root, Node *clause) -{ - Var *forced_null_var; - ListCell *lc; - - /* Check for IS NULL, and identify the Var forced to NULL */ - forced_null_var = find_forced_null_var(clause); - if (forced_null_var == NULL) - return false; - - /* - * If the Var comes from the nullable side of a lower antijoin, the IS - * NULL condition is necessarily true. If it's not nulled by anything, - * there is no point in searching the join_info_list. Otherwise, we need - * to find out whether the nulling rel is an antijoin. - */ - if (forced_null_var->varnullingrels == NULL) - return false; - - foreach(lc, root->join_info_list) - { - SpecialJoinInfo *sjinfo = (SpecialJoinInfo *) lfirst(lc); - - /* - * This test will not succeed if sjinfo->ojrelid is zero, which is - * possible for an antijoin that was converted from a semijoin; but in - * such a case the Var couldn't have come from its nullable side. - */ - if (sjinfo->jointype == JOIN_ANTI && sjinfo->ojrelid != 0 && - bms_is_member(sjinfo->ojrelid, forced_null_var->varnullingrels)) - return true; - } - - return false; -} - /* * distribute_restrictinfo_to_rels * Push a completed RestrictInfo into the proper restriction or join diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c index ed8465434c5..4a7f9d905aa 100644 --- a/src/backend/optimizer/prep/prepjointree.c +++ b/src/backend/optimizer/prep/prepjointree.c @@ -79,6 +79,7 @@ typedef struct reduce_outer_joins_pass2_state { Relids inner_reduced; /* OJ relids reduced to plain inner joins */ List *partial_reduced; /* List of partially reduced FULL joins */ + Relids anti_reduced; /* OJ relids reduced to antijoins */ } reduce_outer_joins_pass2_state; typedef struct reduce_outer_joins_partial_state @@ -142,6 +143,9 @@ static void reduce_outer_joins_pass2(Node *jtnode, List *forced_null_vars); static void report_reduced_full_join(reduce_outer_joins_pass2_state *state2, int rtindex, Relids relids); +static void remove_redundant_nullability_quals(Node *jtnode, + Relids antijoins); +static Node *strip_redundant_nullability_quals(Node *quals, Relids antijoins); static Node *remove_useless_results_recurse(PlannerInfo *root, Node *jtnode, Relids baserels, Node **parent_quals, @@ -2779,9 +2783,8 @@ flatten_simple_union_all(PlannerInfo *root) * If the join clause is strict for b.y, then only null-extended rows could * pass the upper WHERE, and we can conclude that what the query is really * specifying is an anti-semijoin. We change the join type from JOIN_LEFT - * to JOIN_ANTI. The IS NULL clause then becomes redundant, and must be - * removed to prevent bogus selectivity calculations, but we leave it to - * distribute_qual_to_rels to get rid of such clauses. + * to JOIN_ANTI. The IS NULL clause then becomes redundant, and is removed + * at the end of this phase; see remove_redundant_nullability_quals. * * Also, we get rid of JOIN_RIGHT cases by flipping them around to become * JOIN_LEFT. This saves some code here and in some later planner routines; @@ -2817,6 +2820,7 @@ reduce_outer_joins(PlannerInfo *root) state2.inner_reduced = NULL; state2.partial_reduced = NIL; + state2.anti_reduced = NULL; reduce_outer_joins_pass2((Node *) root->parse->jointree, state1, &state2, @@ -2859,6 +2863,76 @@ reduce_outer_joins(PlannerInfo *root) full_join_relids, statep->unreduced_side); } + + /* + * Finally, remove any quals made redundant by reducing outer joins to + * antijoins. + */ + if (!bms_is_empty(state2.anti_reduced)) + remove_redundant_nullability_quals((Node *) root->parse->jointree, + state2.anti_reduced); +} + +/* + * remove_redundant_nullability_quals + * Remove quals made redundant by reducing outer joins to antijoins. + * + * An IS NULL qual on a Var from the nullable side of a lower antijoin is + * necessarily true. Keeping such a qual would not be wrong, but it would + * generate bogus selectivity estimates, and it could prevent join removal + * from later removing the rel(s) it references. + */ +static void +remove_redundant_nullability_quals(Node *jtnode, Relids antijoins) +{ + if (jtnode == NULL) + return; + if (IsA(jtnode, RangeTblRef)) + { + /* nothing to do here */ + } + else if (IsA(jtnode, FromExpr)) + { + FromExpr *f = (FromExpr *) jtnode; + ListCell *l; + + foreach(l, f->fromlist) + remove_redundant_nullability_quals(lfirst(l), antijoins); + f->quals = strip_redundant_nullability_quals(f->quals, antijoins); + } + else if (IsA(jtnode, JoinExpr)) + { + JoinExpr *j = (JoinExpr *) jtnode; + + remove_redundant_nullability_quals(j->larg, antijoins); + remove_redundant_nullability_quals(j->rarg, antijoins); + j->quals = strip_redundant_nullability_quals(j->quals, antijoins); + } + else + elog(ERROR, "unrecognized jointree node type: %d", + (int) nodeTag(jtnode)); +} + +/* + * strip_redundant_nullability_quals + * Strip redundant IS NULL quals from one implicit-AND qual list. + */ +static Node * +strip_redundant_nullability_quals(Node *quals, Relids antijoins) +{ + List *newquals = NIL; + ListCell *lc; + + foreach(lc, castNode(List, quals)) + { + Node *clause = (Node *) lfirst(lc); + Var *var = find_forced_null_var(clause); + + if (var && bms_overlap(var->varnullingrels, antijoins)) + continue; + newquals = lappend(newquals, clause); + } + return (Node *) newquals; } /* @@ -2940,9 +3014,10 @@ reduce_outer_joins_pass1(Node *jtnode) * * Returns info in state2 about outer joins that were successfully simplified. * Joins that were fully reduced to inner joins are all added to - * state2->inner_reduced. If a full join is reduced to a left join, - * it needs its own entry in state2->partial_reduced, since that will - * require custom processing to remove only the correct nullingrel markers. + * state2->inner_reduced, and joins that became antijoins are all added to + * state2->anti_reduced. If a full join is reduced to a left join, it also + * needs its own entry in state2->partial_reduced, since that will require + * custom processing to remove only the correct nullingrel markers. */ static void reduce_outer_joins_pass2(Node *jtnode, @@ -3100,7 +3175,8 @@ reduce_outer_joins_pass2(Node *jtnode, /* * Apply the jointype change, if any, to both jointree node and RTE. - * Also, if we changed an RTE to INNER, add its RTI to inner_reduced. + * Also, if we changed an RTE to INNER, add its RTI to inner_reduced; + * if we changed it to ANTI, add its RTI to anti_reduced. */ if (rtindex && jointype != j->jointype) { @@ -3112,6 +3188,9 @@ reduce_outer_joins_pass2(Node *jtnode, if (jointype == JOIN_INNER) state2->inner_reduced = bms_add_member(state2->inner_reduced, rtindex); + else if (jointype == JOIN_ANTI) + state2->anti_reduced = bms_add_member(state2->anti_reduced, + rtindex); } j->jointype = jointype; diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index 82d356789e1..360b6085481 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -1957,11 +1957,12 @@ find_forced_null_vars(Node *node) * *only* nullness of the particular Var, not any other conditions. * * This is just the single-clause case of find_forced_null_vars(), without - * any allowance for AND conditions. It's used by initsplan.c on individual - * qual clauses. The reason for not just applying find_forced_null_vars() - * is that if an AND of an IS NULL clause with something else were to somehow - * survive AND/OR flattening, initsplan.c might get fooled into discarding - * the whole clause when only the IS NULL part of it had been proved redundant. + * any allowance for AND conditions. It's used by prepjointree.c on + * individual qual clauses. The reason for not just applying + * find_forced_null_vars() is that if an AND of an IS NULL clause with + * something else were to somehow survive AND/OR flattening, prepjointree.c + * might get fooled into discarding the whole clause when only the IS NULL + * part of it had been proved redundant. */ Var * find_forced_null_var(Node *node) diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index 1408c333ced..1e15096f405 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -3127,6 +3127,46 @@ where b.unique2 is null; -> Index Only Scan using tenk1_unique2 on tenk1 b (5 rows) +-- +-- Test that quals made redundant by reducing an outer join to an antijoin are +-- removed from the jointree +-- (fallout from the fix for bug #19560) +-- +begin; +create temp table tbl_anti_pk (a int primary key); +-- t3.a IS NULL is redundant, and the t2/t3 join can be removed +explain (costs off) +select 1 from tbl_anti_pk t1 left join + (tbl_anti_pk t2 left join tbl_anti_pk t3 on t3.a = t2.a) on t2.a = t1.a +where t2.a is null and t3.a is null; + QUERY PLAN +---------------------------------------- + Hash Anti Join + Hash Cond: (t1.a = t2.a) + -> Seq Scan on tbl_anti_pk t1 + -> Hash + -> Seq Scan on tbl_anti_pk t2 +(5 rows) + +-- the redundant qual can also be a degenerate ON clause of an upper join +explain (costs off) +select 1 from tbl_anti_pk t1 left join + (tbl_anti_pk t2 left join + (tbl_anti_pk t3 left join tbl_anti_pk t5 on t5.a = t3.a) on t3.a = t2.a) + on t3.a is null and t5.a is null; + QUERY PLAN +---------------------------------------------------- + Nested Loop Left Join + -> Seq Scan on tbl_anti_pk t1 + -> Materialize + -> Hash Anti Join + Hash Cond: (t2.a = t3.a) + -> Seq Scan on tbl_anti_pk t2 + -> Hash + -> Seq Scan on tbl_anti_pk t3 +(8 rows) + +rollback; -- -- regression test for proper handling of outer joins within antijoins -- diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index 998e367c121..c094e5264d9 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -797,6 +797,31 @@ explain (costs off) select a.* from tenk1 a left join tenk1 b on a.unique1 = b.unique2 where b.unique2 is null; +-- +-- Test that quals made redundant by reducing an outer join to an antijoin are +-- removed from the jointree +-- (fallout from the fix for bug #19560) +-- + +begin; + +create temp table tbl_anti_pk (a int primary key); + +-- t3.a IS NULL is redundant, and the t2/t3 join can be removed +explain (costs off) +select 1 from tbl_anti_pk t1 left join + (tbl_anti_pk t2 left join tbl_anti_pk t3 on t3.a = t2.a) on t2.a = t1.a +where t2.a is null and t3.a is null; + +-- the redundant qual can also be a degenerate ON clause of an upper join +explain (costs off) +select 1 from tbl_anti_pk t1 left join + (tbl_anti_pk t2 left join + (tbl_anti_pk t3 left join tbl_anti_pk t5 on t5.a = t3.a) on t3.a = t2.a) + on t3.a is null and t5.a is null; + +rollback; + -- -- regression test for proper handling of outer joins within antijoins --