From cfcd5711160a42249def8f781bae197829cf44c7 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 20 Apr 2026 14:48:23 -0400 Subject: [PATCH] Clean up all relid fields of RestrictInfos during join removal. MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The original implementation of remove_rel_from_restrictinfo() thought it could skate by with removing no-longer-valid relid bits from only the clause_relids and required_relids fields. This is quite bogus, although somehow we had not run across a counterexample before now. At minimum, the left_relids and right_relids fields need to be fixed because they will be examined later by clause_sides_match_join(). But it seems pretty foolish not to fix all the relid fields, so do that. This needs to be back-patched as far as v16, because the bug report shows a planner failure that does not occur before v16. I'm a little nervous about back-patching, because this could cause unexpected plan changes due to opening up join possibilities that were rejected before. But it's hard to argue that this isn't a regression. Also, the fact that this changes no existing regression test results suggests that the scope of changes may be fairly narrow. I'll refrain from back-patching further though, since no adverse effects have been demonstrated in older branches. Bug: #19460 Reported-by: François Jehl Author: Tom Lane Reviewed-by: Richard Guo Discussion: https://postgr.es/m/19460-5625143cef66012f@postgresql.org Backpatch-through: 16 --- src/backend/optimizer/plan/analyzejoins.c | 18 ++++++++++- src/test/regress/expected/join.out | 39 +++++++++++++++++++++++ src/test/regress/sql/join.sql | 23 +++++++++++++ 3 files changed, 79 insertions(+), 1 deletion(-) diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c index 64874c0d8e3..bfb1af614c2 100644 --- a/src/backend/optimizer/plan/analyzejoins.c +++ b/src/backend/optimizer/plan/analyzejoins.c @@ -673,7 +673,7 @@ remove_rel_from_query(PlannerInfo *root, int relid, /* * Remove any references to relid or ojrelid from the RestrictInfo. * - * We only bother to clean out bits in clause_relids and required_relids, + * We only bother to clean out bits in the RestrictInfo's various relid sets, * 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. @@ -695,6 +695,22 @@ remove_rel_from_restrictinfo(RestrictInfo *rinfo, int relid, int ojrelid) 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); + /* Likewise for incompatible_relids */ + rinfo->incompatible_relids = bms_copy(rinfo->incompatible_relids); + rinfo->incompatible_relids = bms_del_member(rinfo->incompatible_relids, relid); + rinfo->incompatible_relids = bms_del_member(rinfo->incompatible_relids, ojrelid); + /* Likewise for outer_relids */ + rinfo->outer_relids = bms_copy(rinfo->outer_relids); + rinfo->outer_relids = bms_del_member(rinfo->outer_relids, relid); + rinfo->outer_relids = bms_del_member(rinfo->outer_relids, ojrelid); + /* Likewise for left_relids */ + rinfo->left_relids = bms_copy(rinfo->left_relids); + rinfo->left_relids = bms_del_member(rinfo->left_relids, relid); + rinfo->left_relids = bms_del_member(rinfo->left_relids, ojrelid); + /* Likewise for right_relids */ + rinfo->right_relids = bms_copy(rinfo->right_relids); + rinfo->right_relids = bms_del_member(rinfo->right_relids, relid); + rinfo->right_relids = bms_del_member(rinfo->right_relids, ojrelid); /* If it's an OR, recurse to clean up sub-clauses */ if (restriction_is_or_clause(rinfo)) diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index 84872c6f04e..78bf022f7b4 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -6230,6 +6230,45 @@ from int8_tbl t1 -> Seq Scan on onek t4 (13 rows) +-- bug #19460: we need to clean up RestrictInfos more than we had been doing +explain (costs off) +select * from + (select 1::int as id) as lhs +full join + (select dummy_source.id + from (select null::int as id) as dummy_source + left join (select a.id from a where a.id = 42) as sub + on sub.id = dummy_source.id + ) as rhs +on lhs.id = rhs.id; + QUERY PLAN +-------------------------------------- + Hash Full Join + Hash Cond: ((1) = (NULL::integer)) + -> Result + -> Hash + -> Result +(5 rows) + +explain (costs off) +select * from + (select 1::int as id) as lhs +full join + (select dummy_source.id + from (select 2::int as id) as dummy_source + left join (select a.id from a) as sub + on sub.id = dummy_source.id + ) as rhs +on lhs.id = rhs.id; + QUERY PLAN +-------------------------- + Hash Full Join + Hash Cond: ((1) = (2)) + -> Result + -> Hash + -> Result +(5 rows) + -- More tests of correct placement of pseudoconstant quals -- simple constant-false condition explain (costs off) diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index 30b479dda7c..fae19113cef 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -2271,6 +2271,29 @@ from int8_tbl t1 left join onek t4 on t2.q2 < t3.unique2; +-- bug #19460: we need to clean up RestrictInfos more than we had been doing +explain (costs off) +select * from + (select 1::int as id) as lhs +full join + (select dummy_source.id + from (select null::int as id) as dummy_source + left join (select a.id from a where a.id = 42) as sub + on sub.id = dummy_source.id + ) as rhs +on lhs.id = rhs.id; + +explain (costs off) +select * from + (select 1::int as id) as lhs +full join + (select dummy_source.id + from (select 2::int as id) as dummy_source + left join (select a.id from a) as sub + on sub.id = dummy_source.id + ) as rhs +on lhs.id = rhs.id; + -- More tests of correct placement of pseudoconstant quals -- simple constant-false condition -- 2.47.3