From d0f952691ff532aa0c54e9d146fac8d590596646 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 19 May 2023 15:24:07 -0400 Subject: [PATCH] Fix thinko in join removal. In commit 9df8f903e I (tgl) switched join_is_removable() from using the min relid sets of the join under consideration to using its full syntactic relid sets. This was a mistake, as it allowed join removal in cases where a reference to the join output would survive in some syntactically-lower join condition. Revert to the former coding. Richard Guo Discussion: https://postgr.es/m/CAMbWs4-EU9uBGSP7G-iTwLBhRQ=rnZKvFDhD+n+xhajokyPCKg@mail.gmail.com --- src/backend/optimizer/plan/analyzejoins.c | 11 ++++------- src/test/regress/expected/join.out | 16 ++++++++++++++++ src/test/regress/sql/join.sql | 5 +++++ 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c index 7463b3696ac..a61e35f92d0 100644 --- a/src/backend/optimizer/plan/analyzejoins.c +++ b/src/backend/optimizer/plan/analyzejoins.c @@ -88,11 +88,8 @@ restart: */ innerrelid = bms_singleton_member(sjinfo->min_righthand); - /* - * Compute the relid set for the join we are considering. We can - * assume things are done in syntactic order. - */ - joinrelids = bms_union(sjinfo->syn_lefthand, sjinfo->syn_righthand); + /* Compute the relid set for the join we are considering */ + joinrelids = bms_union(sjinfo->min_lefthand, sjinfo->min_righthand); if (sjinfo->ojrelid != 0) joinrelids = bms_add_member(joinrelids, sjinfo->ojrelid); @@ -204,8 +201,8 @@ join_is_removable(PlannerInfo *root, SpecialJoinInfo *sjinfo) if (!rel_supports_distinctness(root, innerrel)) return false; - /* Compute the syntactic relid set for the join we are considering */ - inputrelids = bms_union(sjinfo->syn_lefthand, sjinfo->syn_righthand); + /* Compute the relid set for the join we are considering */ + inputrelids = bms_union(sjinfo->min_lefthand, sjinfo->min_righthand); Assert(sjinfo->ojrelid != 0); joinrelids = bms_copy(inputrelids); joinrelids = bms_add_member(joinrelids, sjinfo->ojrelid); diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index 9bafadde66f..8fa2b376f3a 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -5432,6 +5432,22 @@ select d.* from d left join (select distinct * from b) s -> Seq Scan on d (9 rows) +-- join removal is not possible here +explain (costs off) +select 1 from a t1 + left join (a t2 left join a t3 on t2.id = 1) on t2.id = 1; + QUERY PLAN +-------------------------------------------------------- + Nested Loop Left Join + -> Seq Scan on a t1 + -> Materialize + -> Nested Loop Left Join + Join Filter: (t2.id = 1) + -> Index Only Scan using a_pkey on a t2 + Index Cond: (id = 1) + -> Seq Scan on a t3 +(8 rows) + -- check join removal works when uniqueness of the join condition is enforced -- by a UNION explain (costs off) diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index a44234b0af7..641fd1a21b9 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -1958,6 +1958,11 @@ explain (costs off) select d.* from d left join (select distinct * from b) s on d.a = s.id; +-- join removal is not possible here +explain (costs off) +select 1 from a t1 + left join (a t2 left join a t3 on t2.id = 1) on t2.id = 1; + -- check join removal works when uniqueness of the join condition is enforced -- by a UNION explain (costs off)