Clean up all relid fields of RestrictInfos during join removal.

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 <francois.jehl@pigment.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Richard Guo <guofenglinux@gmail.com>
Discussion: https://postgr.es/m/19460-5625143cef66012f@postgresql.org
Backpatch-through: 16

diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c
index 64874c0d8e359faddf263352007b7b0b058d12a9..bfb1af614c2bfe76781a39fde79c146cd026a1c5 100644 (file)
--- 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 84872c6f04ee8cf92255f372077267ff5ab7e7f4..78bf022f7b4e3475ccabb6a4392b5b65f3670098 100644 (file)
--- 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 30b479dda7c040dc39589012ffb6bb2f8f1b93d1..fae19113cefba28a444f33c17f57cac11e4f4804 100644 (file)
--- 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