diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c index 9d4a9197ee..e6ef0deb23 100644 --- a/src/backend/optimizer/path/joinpath.c +++ b/src/backend/optimizer/path/joinpath.c @@ -378,9 +378,9 @@ allow_star_schema_join(PlannerInfo *root, * restrictions prevent us from attempting a join that would cause a problem. * (That's unsurprising, because the code worked before we ever added * outer-join relids to expression relids.) It still seems worth checking - * as a backstop, but we don't go to a lot of trouble: just reject if the - * unsatisfied part includes any outer-join relids at all. + * as a backstop, but we only do so in assert-enabled builds. */ +#ifdef USE_ASSERT_CHECKING static inline bool have_unsafe_outer_join_ref(PlannerInfo *root, Relids outerrelids, @@ -388,11 +388,10 @@ have_unsafe_outer_join_ref(PlannerInfo *root, { bool result = false; Relids unsatisfied = bms_difference(inner_paramrels, outerrelids); + Relids satisfied = bms_intersect(inner_paramrels, outerrelids); - if (unlikely(bms_overlap(unsatisfied, root->outer_join_rels))) + if (bms_overlap(unsatisfied, root->outer_join_rels)) { -#ifdef NOT_USED - /* If we ever weaken the join order restrictions, we might need this */ ListCell *lc; foreach(lc, root->join_info_list) @@ -401,25 +400,23 @@ have_unsafe_outer_join_ref(PlannerInfo *root, if (!bms_is_member(sjinfo->ojrelid, unsatisfied)) continue; /* not relevant */ - if (bms_overlap(inner_paramrels, sjinfo->min_righthand) || + if (bms_overlap(satisfied, sjinfo->min_righthand) || (sjinfo->jointype == JOIN_FULL && - bms_overlap(inner_paramrels, sjinfo->min_lefthand))) + bms_overlap(satisfied, sjinfo->min_lefthand))) { result = true; /* doesn't work */ break; } } -#else - /* For now, if we do see an overlap, just assume it's trouble */ - result = true; -#endif } /* Waste no memory when we reject a path here */ bms_free(unsatisfied); + bms_free(satisfied); return result; } +#endif /* USE_ASSERT_CHECKING */ /* * paraminfo_get_equal_hashops @@ -713,16 +710,15 @@ try_nestloop_path(PlannerInfo *root, /* * Check to see if proposed path is still parameterized, and reject if the * parameterization wouldn't be sensible --- unless allow_star_schema_join - * says to allow it anyway. Also, we must reject if either - * have_unsafe_outer_join_ref or have_dangerous_phv don't like the look of - * it, which could only happen if the nestloop is still parameterized. + * says to allow it anyway. Also, we must reject if have_dangerous_phv + * doesn't like the look of it, which could only happen if the nestloop is + * still parameterized. */ required_outer = calc_nestloop_required_outer(outerrelids, outer_paramrels, innerrelids, inner_paramrels); if (required_outer && ((!bms_overlap(required_outer, extra->param_source_rels) && !allow_star_schema_join(root, outerrelids, inner_paramrels)) || - have_unsafe_outer_join_ref(root, outerrelids, inner_paramrels) || have_dangerous_phv(root, outerrelids, inner_paramrels))) { /* Waste no memory when we reject a path here */ @@ -730,6 +726,9 @@ try_nestloop_path(PlannerInfo *root, return; } + /* If we got past that, we shouldn't have any unsafe outer-join refs */ + Assert(!have_unsafe_outer_join_ref(root, outerrelids, inner_paramrels)); + /* * Do a precheck to quickly eliminate obviously-inferior paths. We * calculate a cheap lower bound on the path's cost and then use diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index 2f1f8b8dbe..db9512969f 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -4653,6 +4653,32 @@ where tt1.f1 = ss1.c0; ---------- (0 rows) +explain (verbose, costs off) +select 1 from + int4_tbl as i4 + inner join + ((select 42 as n from int4_tbl x1 left join int8_tbl x2 on f1 = q1) as ss1 + right join (select 1 as z) as ss2 on true) + on false, + lateral (select i4.f1, ss1.n from int8_tbl as i8 limit 1) as ss3; + QUERY PLAN +-------------------------- + Result + Output: 1 + One-Time Filter: false +(3 rows) + +select 1 from + int4_tbl as i4 + inner join + ((select 42 as n from int4_tbl x1 left join int8_tbl x2 on f1 = q1) as ss1 + right join (select 1 as z) as ss2 on true) + on false, + lateral (select i4.f1, ss1.n from int8_tbl as i8 limit 1) as ss3; + ?column? +---------- +(0 rows) + -- -- check a case in which a PlaceHolderVar forces join order -- diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index 400c16958f..2ff68879d2 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -1609,6 +1609,23 @@ select 1 from lateral (select tt4.f1 as c0 from text_tbl as tt5 limit 1) as ss1 where tt1.f1 = ss1.c0; +explain (verbose, costs off) +select 1 from + int4_tbl as i4 + inner join + ((select 42 as n from int4_tbl x1 left join int8_tbl x2 on f1 = q1) as ss1 + right join (select 1 as z) as ss2 on true) + on false, + lateral (select i4.f1, ss1.n from int8_tbl as i8 limit 1) as ss3; + +select 1 from + int4_tbl as i4 + inner join + ((select 42 as n from int4_tbl x1 left join int8_tbl x2 on f1 = q1) as ss1 + right join (select 1 as z) as ss2 on true) + on false, + lateral (select i4.f1, ss1.n from int8_tbl as i8 limit 1) as ss3; + -- -- check a case in which a PlaceHolderVar forces join order --