Richard Guo <[email protected]> writes:
> On Wed, Jun 28, 2023 at 6:28 AM Tom Lane <[email protected]> wrote:
>> For a real fix, I'm inclined to extend the loop that calculates
>> param_source_rels (in add_paths_to_joinrel) so that it also tracks
>> a set of incompatible relids that *must not* be present in the
>> parameterization of a proposed path. This would basically include
>> OJ relids of OJs that partially overlap the target joinrel; maybe
>> we should also include the min RHS of such OJs. Then we could
>> check that in try_nestloop_path. I've not tried to code this yet.
> I went ahead and drafted a patch based on this idea.
Hmm. This patch is the opposite of what I'd been imagining, because
I was thinking we needed to add OJs to param_incompatible_relids if
they were *not* already in the join, rather than if they were.
However, I tried it like that and while it did stop the assertion
failure, it also broke a bunch of other test cases that no longer
found the parameterized-nestloop plans they were supposed to find.
So clearly I just didn't have my head screwed on in the correct
direction yesterday.
However, given that what we need is to exclude parameterization
that depends on the currently-formed OJ, it seems to me we can do
it more simply and without any new JoinPathExtraData field,
as attached. What do you think?
> * I think we need to check the incompatible relids also in
> try_hashjoin_path and try_mergejoin_path besides try_nestloop_path.
I think this isn't necessary, at least in my formulation.
Those cases will go through calc_non_nestloop_required_outer
which has
/* neither path can require rels from the other */
Assert(!bms_overlap(outer_paramrels, inner_path->parent->relids));
Assert(!bms_overlap(inner_paramrels, outer_path->parent->relids));
In order to have a dependency on an OJ, a path would have to have
a dependency on at least one of the OJ's base relations too, so
I think these assertions show that the case won't arise. (Of
course, if someone can trip one of these assertions, I'm wrong.)
regards, tom lane
diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c
index c2f211a60d..4b6ed6e312 100644
--- a/src/backend/optimizer/path/joinpath.c
+++ b/src/backend/optimizer/path/joinpath.c
@@ -698,6 +698,17 @@ try_nestloop_path(PlannerInfo *root,
Relids inner_paramrels = PATH_REQ_OUTER(inner_path);
Relids outer_paramrels = PATH_REQ_OUTER(outer_path);
+ /*
+ * If we are forming an outer join at this join, it's nonsensical to use
+ * an input path that uses the outer join as part of its parameterization.
+ * (This can happen despite our join order restrictions, since those apply
+ * to what is in an input relation not what its parameters are.)
+ */
+ if (extra->sjinfo && extra->sjinfo->ojrelid != 0 &&
+ (bms_is_member(extra->sjinfo->ojrelid, outer_paramrels) ||
+ bms_is_member(extra->sjinfo->ojrelid, inner_paramrels)))
+ return;
+
/*
* Paths are parameterized by top-level parents, so run parameterization
* tests on the parent relids.
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 6917faec14..12b828fae3 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -5063,6 +5063,37 @@ select 1 from
----------
(0 rows)
+explain (costs off)
+select 1 from tenk1 t1
+ join lateral
+ (select t1.unique1 from (select 1) foo offset 0) s1 on true
+ join
+ (select 1 from tenk1 t2
+ inner join tenk1 t3
+ left join tenk1 t4 left join tenk1 t5 on t4.unique1 = 1
+ on t4.unique1 = 1 on false
+ where t3.unique1 = coalesce(t5.unique1,1)) as s2
+ on true;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+select 1 from tenk1 t1
+ join lateral
+ (select t1.unique1 from (select 1) foo offset 0) s1 on true
+ join
+ (select 1 from tenk1 t2
+ inner join tenk1 t3
+ left join tenk1 t4 left join tenk1 t5 on t4.unique1 = 1
+ on t4.unique1 = 1 on false
+ where t3.unique1 = coalesce(t5.unique1,1)) as s2
+ on true;
+ ?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 55080bec9a..38899ed3b9 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -1751,6 +1751,29 @@ select 1 from
on false,
lateral (select i4.f1, ss1.n from int8_tbl as i8 limit 1) as ss3;
+explain (costs off)
+select 1 from tenk1 t1
+ join lateral
+ (select t1.unique1 from (select 1) foo offset 0) s1 on true
+ join
+ (select 1 from tenk1 t2
+ inner join tenk1 t3
+ left join tenk1 t4 left join tenk1 t5 on t4.unique1 = 1
+ on t4.unique1 = 1 on false
+ where t3.unique1 = coalesce(t5.unique1,1)) as s2
+ on true;
+
+select 1 from tenk1 t1
+ join lateral
+ (select t1.unique1 from (select 1) foo offset 0) s1 on true
+ join
+ (select 1 from tenk1 t2
+ inner join tenk1 t3
+ left join tenk1 t4 left join tenk1 t5 on t4.unique1 = 1
+ on t4.unique1 = 1 on false
+ where t3.unique1 = coalesce(t5.unique1,1)) as s2
+ on true;
+
--
-- check a case in which a PlaceHolderVar forces join order
--
