Hi, Here's a cleaned up version of the fix, with patches for each backbranch (it's mostly the same as master, except that 18 changed one of the plans in regression tests, and 19 introduced pg_mask field in the struct).
I've considered fixing it per Tender's suggestion, but I didn't like it. It feels more like after-the-fact protection - it constructs the paths and then discards them later. I think we don't want to construct these nonsensical parameterized paths at all. First, it adds overhead (I have not bothered to measure how much, though). Second, I'm afraid it would silently mask a bug/thinko elsewhere. I'm now fairly confident the fix is correct. I'd still welcome someone else doing a quick review, of course. But similarly to [1] I'm planning to probably push this in a couple days, once the minor releases are out the door. regards -- Tomas Vondra
From cfaf4f3ab3784daa73078160851aa688a94d661f Mon Sep 17 00:00:00 2001 From: Tomas Vondra <[email protected]> Date: Sun, 8 Nov 2026 18:55:04 +0100 Subject: [PATCH v3-master] Reject join input paths parameterized by pushed-down outer joins. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit a798660ebe3 taught try_nestloop_path(), try_mergejoin_path() and try_hashjoin_path() to reject an input path with a nonsensical parameterization. But that check has a gap, making it possible to hit the assertion in get_joinrel_parampathinfo() added by 5b7b5518d0ea. We need to reject all paths whose parameterization includes the outer join being formed at the current join, since a value that is supposed to be nulled by that outer join can't be available before the join is done. That check considered only sjinfo->ojrelid, but that's insufficient because of joins pushed down into the RHS per outer-join identity 3. That relid is added only at the join that completes it, with the relid of the outer join performed there (see add_outer_joins_to_relids). This omission allowed building paths parameterized by a pushed-down outer join, which is still nonsensical. In non-assert builds this is mostly harmless - we still reject the path, just a bit later. To fix, determine the full set of outer joins computed at the current join. That includes sjinfo's own OJ plus any pushed-down OJs completed there. We do this once in add_paths_to_joinrel(), and save it in a new JoinPathExtraData field, and check the input paths' parameterization against that whole set. The partial-path variants need no change, since they never accept input paths whose parameterization isn't satisfied by the outer rel. Backpatch to 16, where 2489d76c490 made Vars outer-join-aware and 9df8f903eb6 introduced the delayed addition of pushed-down outer join relids. The checks added by a798660ebe3 did not cover this case. The new JoinPathExtraData field needs to go at the end of the struct, to avoid an ABI break. It's allocated only in add_paths_to_joinrel (on stack), and only passed through a pointer, so this should be safe. Author: Tomas Vondra <[email protected]> Reviewed-by: Tender Wang <[email protected]> Reviewed-by: Álvaro Herrera <[email protected]> Backpatch-through: 16 Discussion: https://postgr.es/m/[email protected] --- src/backend/optimizer/path/joinpath.c | 53 +++++++++++++++++++++------ src/include/nodes/pathnodes.h | 4 ++ src/test/regress/expected/join.out | 35 ++++++++++++++++++ src/test/regress/sql/join.sql | 15 ++++++++ 4 files changed, 95 insertions(+), 12 deletions(-) diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c index 9d9e6fad6a1..beb2856dd21 100644 --- a/src/backend/optimizer/path/joinpath.c +++ b/src/backend/optimizer/path/joinpath.c @@ -154,6 +154,7 @@ add_paths_to_joinrel(PlannerInfo *root, extra.sjinfo = sjinfo; extra.param_source_rels = NULL; extra.pgs_mask = joinrel->pgs_mask; + extra.ojrelids = NULL; /* * Give extensions a chance to take control. In particular, an extension @@ -313,6 +314,28 @@ add_paths_to_joinrel(PlannerInfo *root, extra.param_source_rels = bms_add_members(extra.param_source_rels, joinrel->lateral_relids); + /* + * Identify the outer joins computed at this join, since it'd be + * nonsensical to use an input path that is parameterized by any of them + * (see try_nestloop_path). + * + * Besides sjinfo's own OJ, this includes any pushed-down OJs that are + * completed at this join per outer-join identity 3; those are the members + * of the joinrel's relids that are in neither input rel. (We add + * sjinfo->ojrelid explicitly because it won't be in the joinrel's relids + * if this join has itself been pushed down into the RHS of a lower outer + * join.) + * + * If sjinfo has no ojrelid, then no outer joins can be computed here; see + * add_outer_joins_to_relids. + */ + if (sjinfo->ojrelid != 0) + { + extra.ojrelids = bms_difference(joinrel->relids, outerrel->relids); + extra.ojrelids = bms_del_members(extra.ojrelids, innerrel->relids); + extra.ojrelids = bms_add_member(extra.ojrelids, sjinfo->ojrelid); + } + /* * 1. Consider mergejoin paths where both relations must be explicitly * sorted. Skip this if we can't mergejoin. @@ -1026,11 +1049,13 @@ try_nestloop_path(PlannerInfo *root, * 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.) + * to what is in an input relation not what its parameters are.) The same + * goes for any pushed-down outer joins that are completed at this join, + * so we must check against all of extra->ojrelids, not just + * sjinfo->ojrelid. */ - if (extra->sjinfo->ojrelid != 0 && - (bms_is_member(extra->sjinfo->ojrelid, inner_paramrels) || - bms_is_member(extra->sjinfo->ojrelid, outer_paramrels))) + if (bms_overlap(extra->ojrelids, inner_paramrels) || + bms_overlap(extra->ojrelids, outer_paramrels)) return; /* @@ -1241,11 +1266,13 @@ try_mergejoin_path(PlannerInfo *root, * 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.) + * to what is in an input relation not what its parameters are.) The same + * goes for any pushed-down outer joins that are completed at this join, + * so we must check against all of extra->ojrelids, not just + * sjinfo->ojrelid. */ - if (extra->sjinfo->ojrelid != 0 && - (bms_is_member(extra->sjinfo->ojrelid, PATH_REQ_OUTER(inner_path)) || - bms_is_member(extra->sjinfo->ojrelid, PATH_REQ_OUTER(outer_path)))) + if (bms_overlap(extra->ojrelids, PATH_REQ_OUTER(inner_path)) || + bms_overlap(extra->ojrelids, PATH_REQ_OUTER(outer_path))) return; /* @@ -1415,11 +1442,13 @@ try_hashjoin_path(PlannerInfo *root, * 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.) + * to what is in an input relation not what its parameters are.) The same + * goes for any pushed-down outer joins that are completed at this join, + * so we must check against all of extra->ojrelids, not just + * sjinfo->ojrelid. */ - if (extra->sjinfo->ojrelid != 0 && - (bms_is_member(extra->sjinfo->ojrelid, PATH_REQ_OUTER(inner_path)) || - bms_is_member(extra->sjinfo->ojrelid, PATH_REQ_OUTER(outer_path)))) + if (bms_overlap(extra->ojrelids, PATH_REQ_OUTER(inner_path)) || + bms_overlap(extra->ojrelids, PATH_REQ_OUTER(outer_path))) return; /* diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index ce22529bec0..2ba1dbfb67f 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -3642,6 +3642,9 @@ typedef struct SemiAntiJoinFactors * RIGHT_ANTI/inner_unique joins) * param_source_rels are OK targets for parameterization of result paths * pgs_mask is a bitmask of PGS_* constants to limit the join strategy + * ojrelids are the outer joins computed at this join (sjinfo's own OJ, if + * any, plus any pushed-down OJs completed here); input paths must not + * be parameterized by any of these */ typedef struct JoinPathExtraData { @@ -3652,6 +3655,7 @@ typedef struct JoinPathExtraData SemiAntiJoinFactors semifactors; Relids param_source_rels; uint64 pgs_mask; + Relids ojrelids; } JoinPathExtraData; /* diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index b41f2157490..a4b2c8da0f9 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -7248,6 +7248,41 @@ select 1 from t t1 (3 rows) rollback; +-- +-- check a similar case where the parameterization refers to an outer join +-- that was pushed down per outer-join identity 3 +-- +explain (costs off) +select 1 from tenk1 t1 +where t1.thousand in + (select t2.thousand + from tenk1 t2 + left join tenk1 t3 on t2.thousand = t3.thousand + left join (tenk1 t4 left join tenk1 t5 on t4.thousand = t5.thousand) + on t3.thousand = t4.thousand + where coalesce(t5.tenthous, 0) = t2.tenthous); + QUERY PLAN +-------------------------------------------------------------------------------------- + Hash Right Semi Join + Hash Cond: (t2.thousand = t1.thousand) + -> Hash Left Join + Hash Cond: (t3.thousand = t4.thousand) + Filter: (COALESCE(t5.tenthous, 0) = t2.tenthous) + -> Merge Left Join + Merge Cond: (t2.thousand = t3.thousand) + -> Index Only Scan using tenk1_thous_tenthous on tenk1 t2 + -> Materialize + -> Index Only Scan using tenk1_thous_tenthous on tenk1 t3 + -> Hash + -> Hash Left Join + Hash Cond: (t4.thousand = t5.thousand) + -> Index Only Scan using tenk1_thous_tenthous on tenk1 t4 + -> Hash + -> Index Only Scan using tenk1_thous_tenthous on tenk1 t5 + -> Hash + -> Index Only Scan using tenk1_thous_tenthous on tenk1 t1 +(18 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 eb7135efe8b..72184c326d4 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -2577,6 +2577,21 @@ select 1 from t t1 rollback; +-- +-- check a similar case where the parameterization refers to an outer join +-- that was pushed down per outer-join identity 3 +-- + +explain (costs off) +select 1 from tenk1 t1 +where t1.thousand in + (select t2.thousand + from tenk1 t2 + left join tenk1 t3 on t2.thousand = t3.thousand + left join (tenk1 t4 left join tenk1 t5 on t4.thousand = t5.thousand) + on t3.thousand = t4.thousand + where coalesce(t5.tenthous, 0) = t2.tenthous); + -- -- check a case in which a PlaceHolderVar forces join order -- -- 2.55.0
From b0cf0f63f3b9e44d0ad9cd3d333d9d4acea6cd81 Mon Sep 17 00:00:00 2001 From: Tomas Vondra <[email protected]> Date: Sun, 8 Nov 2026 18:55:04 +0100 Subject: [PATCH v3-19] Reject join input paths parameterized by pushed-down outer joins. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit a798660ebe3 taught try_nestloop_path(), try_mergejoin_path() and try_hashjoin_path() to reject an input path with a nonsensical parameterization. But that check has a gap, making it possible to hit the assertion in get_joinrel_parampathinfo() added by 5b7b5518d0ea. We need to reject all paths whose parameterization includes the outer join being formed at the current join, since a value that is supposed to be nulled by that outer join can't be available before the join is done. That check considered only sjinfo->ojrelid, but that's insufficient because of joins pushed down into the RHS per outer-join identity 3. That relid is added only at the join that completes it, with the relid of the outer join performed there (see add_outer_joins_to_relids). This omission allowed building paths parameterized by a pushed-down outer join, which is still nonsensical. In non-assert builds this is mostly harmless - we still reject the path, just a bit later. To fix, determine the full set of outer joins computed at the current join. That includes sjinfo's own OJ plus any pushed-down OJs completed there. We do this once in add_paths_to_joinrel(), and save it in a new JoinPathExtraData field, and check the input paths' parameterization against that whole set. The partial-path variants need no change, since they never accept input paths whose parameterization isn't satisfied by the outer rel. Backpatch to 16, where 2489d76c490 made Vars outer-join-aware and 9df8f903eb6 introduced the delayed addition of pushed-down outer join relids. The checks added by a798660ebe3 did not cover this case. The new JoinPathExtraData field needs to go at the end of the struct, to avoid an ABI break. It's allocated only in add_paths_to_joinrel (on stack), and only passed through a pointer, so this should be safe. Author: Tomas Vondra <[email protected]> Reviewed-by: Tender Wang <[email protected]> Reviewed-by: Álvaro Herrera <[email protected]> Backpatch-through: 16 Discussion: https://postgr.es/m/[email protected] --- src/backend/optimizer/path/joinpath.c | 53 +++++++++++++++++++++------ src/include/nodes/pathnodes.h | 4 ++ src/test/regress/expected/join.out | 35 ++++++++++++++++++ src/test/regress/sql/join.sql | 15 ++++++++ 4 files changed, 95 insertions(+), 12 deletions(-) diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c index e5e8c55c8f2..4478277581d 100644 --- a/src/backend/optimizer/path/joinpath.c +++ b/src/backend/optimizer/path/joinpath.c @@ -154,6 +154,7 @@ add_paths_to_joinrel(PlannerInfo *root, extra.sjinfo = sjinfo; extra.param_source_rels = NULL; extra.pgs_mask = joinrel->pgs_mask; + extra.ojrelids = NULL; /* * Give extensions a chance to take control. In particular, an extension @@ -310,6 +311,28 @@ add_paths_to_joinrel(PlannerInfo *root, extra.param_source_rels = bms_add_members(extra.param_source_rels, joinrel->lateral_relids); + /* + * Identify the outer joins computed at this join, since it'd be + * nonsensical to use an input path that is parameterized by any of them + * (see try_nestloop_path). + * + * Besides sjinfo's own OJ, this includes any pushed-down OJs that are + * completed at this join per outer-join identity 3; those are the members + * of the joinrel's relids that are in neither input rel. (We add + * sjinfo->ojrelid explicitly because it won't be in the joinrel's relids + * if this join has itself been pushed down into the RHS of a lower outer + * join.) + * + * If sjinfo has no ojrelid, then no outer joins can be computed here; see + * add_outer_joins_to_relids. + */ + if (sjinfo->ojrelid != 0) + { + extra.ojrelids = bms_difference(joinrel->relids, outerrel->relids); + extra.ojrelids = bms_del_members(extra.ojrelids, innerrel->relids); + extra.ojrelids = bms_add_member(extra.ojrelids, sjinfo->ojrelid); + } + /* * 1. Consider mergejoin paths where both relations must be explicitly * sorted. Skip this if we can't mergejoin. @@ -996,11 +1019,13 @@ try_nestloop_path(PlannerInfo *root, * 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.) + * to what is in an input relation not what its parameters are.) The same + * goes for any pushed-down outer joins that are completed at this join, + * so we must check against all of extra->ojrelids, not just + * sjinfo->ojrelid. */ - if (extra->sjinfo->ojrelid != 0 && - (bms_is_member(extra->sjinfo->ojrelid, inner_paramrels) || - bms_is_member(extra->sjinfo->ojrelid, outer_paramrels))) + if (bms_overlap(extra->ojrelids, inner_paramrels) || + bms_overlap(extra->ojrelids, outer_paramrels)) return; /* @@ -1211,11 +1236,13 @@ try_mergejoin_path(PlannerInfo *root, * 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.) + * to what is in an input relation not what its parameters are.) The same + * goes for any pushed-down outer joins that are completed at this join, + * so we must check against all of extra->ojrelids, not just + * sjinfo->ojrelid. */ - if (extra->sjinfo->ojrelid != 0 && - (bms_is_member(extra->sjinfo->ojrelid, PATH_REQ_OUTER(inner_path)) || - bms_is_member(extra->sjinfo->ojrelid, PATH_REQ_OUTER(outer_path)))) + if (bms_overlap(extra->ojrelids, PATH_REQ_OUTER(inner_path)) || + bms_overlap(extra->ojrelids, PATH_REQ_OUTER(outer_path))) return; /* @@ -1385,11 +1412,13 @@ try_hashjoin_path(PlannerInfo *root, * 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.) + * to what is in an input relation not what its parameters are.) The same + * goes for any pushed-down outer joins that are completed at this join, + * so we must check against all of extra->ojrelids, not just + * sjinfo->ojrelid. */ - if (extra->sjinfo->ojrelid != 0 && - (bms_is_member(extra->sjinfo->ojrelid, PATH_REQ_OUTER(inner_path)) || - bms_is_member(extra->sjinfo->ojrelid, PATH_REQ_OUTER(outer_path)))) + if (bms_overlap(extra->ojrelids, PATH_REQ_OUTER(inner_path)) || + bms_overlap(extra->ojrelids, PATH_REQ_OUTER(outer_path))) return; /* diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index 75a736c8a58..b3c531e7b40 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -3637,6 +3637,9 @@ typedef struct SemiAntiJoinFactors * semifactors is as shown above (only valid for SEMI/ANTI/inner_unique joins) * param_source_rels are OK targets for parameterization of result paths * pgs_mask is a bitmask of PGS_* constants to limit the join strategy + * ojrelids are the outer joins computed at this join (sjinfo's own OJ, if + * any, plus any pushed-down OJs completed here); input paths must not + * be parameterized by any of these */ typedef struct JoinPathExtraData { @@ -3647,6 +3650,7 @@ typedef struct JoinPathExtraData SemiAntiJoinFactors semifactors; Relids param_source_rels; uint64 pgs_mask; + Relids ojrelids; } JoinPathExtraData; /* diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index bc0d4ca8a71..f37cb362116 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -6389,6 +6389,41 @@ select 1 from t t1 (3 rows) rollback; +-- +-- check a similar case where the parameterization refers to an outer join +-- that was pushed down per outer-join identity 3 +-- +explain (costs off) +select 1 from tenk1 t1 +where t1.thousand in + (select t2.thousand + from tenk1 t2 + left join tenk1 t3 on t2.thousand = t3.thousand + left join (tenk1 t4 left join tenk1 t5 on t4.thousand = t5.thousand) + on t3.thousand = t4.thousand + where coalesce(t5.tenthous, 0) = t2.tenthous); + QUERY PLAN +-------------------------------------------------------------------------------------- + Hash Right Semi Join + Hash Cond: (t2.thousand = t1.thousand) + -> Hash Left Join + Hash Cond: (t3.thousand = t4.thousand) + Filter: (COALESCE(t5.tenthous, 0) = t2.tenthous) + -> Merge Left Join + Merge Cond: (t2.thousand = t3.thousand) + -> Index Only Scan using tenk1_thous_tenthous on tenk1 t2 + -> Materialize + -> Index Only Scan using tenk1_thous_tenthous on tenk1 t3 + -> Hash + -> Hash Left Join + Hash Cond: (t4.thousand = t5.thousand) + -> Index Only Scan using tenk1_thous_tenthous on tenk1 t4 + -> Hash + -> Index Only Scan using tenk1_thous_tenthous on tenk1 t5 + -> Hash + -> Index Only Scan using tenk1_thous_tenthous on tenk1 t1 +(18 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 1f9895d8e87..d7f617dc44b 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -2251,6 +2251,21 @@ select 1 from t t1 rollback; +-- +-- check a similar case where the parameterization refers to an outer join +-- that was pushed down per outer-join identity 3 +-- + +explain (costs off) +select 1 from tenk1 t1 +where t1.thousand in + (select t2.thousand + from tenk1 t2 + left join tenk1 t3 on t2.thousand = t3.thousand + left join (tenk1 t4 left join tenk1 t5 on t4.thousand = t5.thousand) + on t3.thousand = t4.thousand + where coalesce(t5.tenthous, 0) = t2.tenthous); + -- -- check a case in which a PlaceHolderVar forces join order -- -- 2.55.0
From 1761ad02ecc59a6ba17d2e0a7e3136a53c3e90f6 Mon Sep 17 00:00:00 2001 From: Tomas Vondra <[email protected]> Date: Sun, 8 Nov 2026 18:55:04 +0100 Subject: [PATCH v3-18] Reject join input paths parameterized by pushed-down outer joins. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit a798660ebe3 taught try_nestloop_path(), try_mergejoin_path() and try_hashjoin_path() to reject an input path with a nonsensical parameterization. But that check has a gap, making it possible to hit the assertion in get_joinrel_parampathinfo() added by 5b7b5518d0ea. We need to reject all paths whose parameterization includes the outer join being formed at the current join, since a value that is supposed to be nulled by that outer join can't be available before the join is done. That check considered only sjinfo->ojrelid, but that's insufficient because of joins pushed down into the RHS per outer-join identity 3. That relid is added only at the join that completes it, with the relid of the outer join performed there (see add_outer_joins_to_relids). This omission allowed building paths parameterized by a pushed-down outer join, which is still nonsensical. In non-assert builds this is mostly harmless - we still reject the path, just a bit later. To fix, determine the full set of outer joins computed at the current join. That includes sjinfo's own OJ plus any pushed-down OJs completed there. We do this once in add_paths_to_joinrel(), and save it in a new JoinPathExtraData field, and check the input paths' parameterization against that whole set. The partial-path variants need no change, since they never accept input paths whose parameterization isn't satisfied by the outer rel. Backpatch to 16, where 2489d76c490 made Vars outer-join-aware and 9df8f903eb6 introduced the delayed addition of pushed-down outer join relids. The checks added by a798660ebe3 did not cover this case. The new JoinPathExtraData field needs to go at the end of the struct, to avoid an ABI break. It's allocated only in add_paths_to_joinrel (on stack), and only passed through a pointer, so this should be safe. Author: Tomas Vondra <[email protected]> Reviewed-by: Tender Wang <[email protected]> Reviewed-by: Álvaro Herrera <[email protected]> Backpatch-through: 16 Discussion: https://postgr.es/m/[email protected] --- src/backend/optimizer/path/joinpath.c | 53 +++++++++++++++++++++------ src/include/nodes/pathnodes.h | 4 ++ src/test/regress/expected/join.out | 35 ++++++++++++++++++ src/test/regress/sql/join.sql | 15 ++++++++ 4 files changed, 95 insertions(+), 12 deletions(-) diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c index 1359dd424bb..0d616bdb266 100644 --- a/src/backend/optimizer/path/joinpath.c +++ b/src/backend/optimizer/path/joinpath.c @@ -153,6 +153,7 @@ add_paths_to_joinrel(PlannerInfo *root, extra.mergeclause_list = NIL; extra.sjinfo = sjinfo; extra.param_source_rels = NULL; + extra.ojrelids = NULL; /* * See if the inner relation is provably unique for this outer rel. @@ -278,6 +279,28 @@ add_paths_to_joinrel(PlannerInfo *root, extra.param_source_rels = bms_add_members(extra.param_source_rels, joinrel->lateral_relids); + /* + * Identify the outer joins computed at this join, since it'd be + * nonsensical to use an input path that is parameterized by any of them + * (see try_nestloop_path). + * + * Besides sjinfo's own OJ, this includes any pushed-down OJs that are + * completed at this join per outer-join identity 3; those are the members + * of the joinrel's relids that are in neither input rel. (We add + * sjinfo->ojrelid explicitly because it won't be in the joinrel's relids + * if this join has itself been pushed down into the RHS of a lower outer + * join.) + * + * If sjinfo has no ojrelid, then no outer joins can be computed here; see + * add_outer_joins_to_relids. + */ + if (sjinfo->ojrelid != 0) + { + extra.ojrelids = bms_difference(joinrel->relids, outerrel->relids); + extra.ojrelids = bms_del_members(extra.ojrelids, innerrel->relids); + extra.ojrelids = bms_add_member(extra.ojrelids, sjinfo->ojrelid); + } + /* * 1. Consider mergejoin paths where both relations must be explicitly * sorted. Skip this if we can't mergejoin. @@ -948,11 +971,13 @@ try_nestloop_path(PlannerInfo *root, * 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.) + * to what is in an input relation not what its parameters are.) The same + * goes for any pushed-down outer joins that are completed at this join, + * so we must check against all of extra->ojrelids, not just + * sjinfo->ojrelid. */ - if (extra->sjinfo->ojrelid != 0 && - (bms_is_member(extra->sjinfo->ojrelid, inner_paramrels) || - bms_is_member(extra->sjinfo->ojrelid, outer_paramrels))) + if (bms_overlap(extra->ojrelids, inner_paramrels) || + bms_overlap(extra->ojrelids, outer_paramrels)) return; /* @@ -1160,11 +1185,13 @@ try_mergejoin_path(PlannerInfo *root, * 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.) + * to what is in an input relation not what its parameters are.) The same + * goes for any pushed-down outer joins that are completed at this join, + * so we must check against all of extra->ojrelids, not just + * sjinfo->ojrelid. */ - if (extra->sjinfo->ojrelid != 0 && - (bms_is_member(extra->sjinfo->ojrelid, PATH_REQ_OUTER(inner_path)) || - bms_is_member(extra->sjinfo->ojrelid, PATH_REQ_OUTER(outer_path)))) + if (bms_overlap(extra->ojrelids, PATH_REQ_OUTER(inner_path)) || + bms_overlap(extra->ojrelids, PATH_REQ_OUTER(outer_path))) return; /* @@ -1333,11 +1360,13 @@ try_hashjoin_path(PlannerInfo *root, * 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.) + * to what is in an input relation not what its parameters are.) The same + * goes for any pushed-down outer joins that are completed at this join, + * so we must check against all of extra->ojrelids, not just + * sjinfo->ojrelid. */ - if (extra->sjinfo->ojrelid != 0 && - (bms_is_member(extra->sjinfo->ojrelid, PATH_REQ_OUTER(inner_path)) || - bms_is_member(extra->sjinfo->ojrelid, PATH_REQ_OUTER(outer_path)))) + if (bms_overlap(extra->ojrelids, PATH_REQ_OUTER(inner_path)) || + bms_overlap(extra->ojrelids, PATH_REQ_OUTER(outer_path))) return; /* diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index eef8acf1747..46c45f27ca1 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -3387,6 +3387,9 @@ typedef struct SemiAntiJoinFactors * sjinfo is extra info about special joins for selectivity estimation * semifactors is as shown above (only valid for SEMI/ANTI/inner_unique joins) * param_source_rels are OK targets for parameterization of result paths + * ojrelids are the outer joins computed at this join (sjinfo's own OJ, if + * any, plus any pushed-down OJs completed here); input paths must not + * be parameterized by any of these */ typedef struct JoinPathExtraData { @@ -3396,6 +3399,7 @@ typedef struct JoinPathExtraData SpecialJoinInfo *sjinfo; SemiAntiJoinFactors semifactors; Relids param_source_rels; + Relids ojrelids; } JoinPathExtraData; /* diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index be055a23124..aba11c61247 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -6501,6 +6501,41 @@ select 1 from t t1 (2 rows) rollback; +-- +-- check a similar case where the parameterization refers to an outer join +-- that was pushed down per outer-join identity 3 +-- +explain (costs off) +select 1 from tenk1 t1 +where t1.thousand in + (select t2.thousand + from tenk1 t2 + left join tenk1 t3 on t2.thousand = t3.thousand + left join (tenk1 t4 left join tenk1 t5 on t4.thousand = t5.thousand) + on t3.thousand = t4.thousand + where coalesce(t5.tenthous, 0) = t2.tenthous); + QUERY PLAN +-------------------------------------------------------------------------------------- + Hash Right Semi Join + Hash Cond: (t2.thousand = t1.thousand) + -> Hash Left Join + Hash Cond: (t3.thousand = t4.thousand) + Filter: (COALESCE(t5.tenthous, 0) = t2.tenthous) + -> Merge Left Join + Merge Cond: (t2.thousand = t3.thousand) + -> Index Only Scan using tenk1_thous_tenthous on tenk1 t2 + -> Materialize + -> Index Only Scan using tenk1_thous_tenthous on tenk1 t3 + -> Hash + -> Hash Left Join + Hash Cond: (t4.thousand = t5.thousand) + -> Index Only Scan using tenk1_thous_tenthous on tenk1 t4 + -> Hash + -> Index Only Scan using tenk1_thous_tenthous on tenk1 t5 + -> Hash + -> Index Only Scan using tenk1_thous_tenthous on tenk1 t1 +(18 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 f17c2c5c627..4d2a9302c86 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -2295,6 +2295,21 @@ select 1 from t t1 rollback; +-- +-- check a similar case where the parameterization refers to an outer join +-- that was pushed down per outer-join identity 3 +-- + +explain (costs off) +select 1 from tenk1 t1 +where t1.thousand in + (select t2.thousand + from tenk1 t2 + left join tenk1 t3 on t2.thousand = t3.thousand + left join (tenk1 t4 left join tenk1 t5 on t4.thousand = t5.thousand) + on t3.thousand = t4.thousand + where coalesce(t5.tenthous, 0) = t2.tenthous); + -- -- check a case in which a PlaceHolderVar forces join order -- -- 2.55.0
From 0e1f22c673a7eadb568a643fbcd8a4dc86892082 Mon Sep 17 00:00:00 2001 From: Tomas Vondra <[email protected]> Date: Sun, 8 Nov 2026 18:55:04 +0100 Subject: [PATCH v3-17] Reject join input paths parameterized by pushed-down outer joins. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit a798660ebe3 taught try_nestloop_path(), try_mergejoin_path() and try_hashjoin_path() to reject an input path with a nonsensical parameterization. But that check has a gap, making it possible to hit the assertion in get_joinrel_parampathinfo() added by 5b7b5518d0ea. We need to reject all paths whose parameterization includes the outer join being formed at the current join, since a value that is supposed to be nulled by that outer join can't be available before the join is done. That check considered only sjinfo->ojrelid, but that's insufficient because of joins pushed down into the RHS per outer-join identity 3. That relid is added only at the join that completes it, with the relid of the outer join performed there (see add_outer_joins_to_relids). This omission allowed building paths parameterized by a pushed-down outer join, which is still nonsensical. In non-assert builds this is mostly harmless - we still reject the path, just a bit later. To fix, determine the full set of outer joins computed at the current join. That includes sjinfo's own OJ plus any pushed-down OJs completed there. We do this once in add_paths_to_joinrel(), and save it in a new JoinPathExtraData field, and check the input paths' parameterization against that whole set. The partial-path variants need no change, since they never accept input paths whose parameterization isn't satisfied by the outer rel. Backpatch to 16, where 2489d76c490 made Vars outer-join-aware and 9df8f903eb6 introduced the delayed addition of pushed-down outer join relids. The checks added by a798660ebe3 did not cover this case. The new JoinPathExtraData field needs to go at the end of the struct, to avoid an ABI break. It's allocated only in add_paths_to_joinrel (on stack), and only passed through a pointer, so this should be safe. Author: Tomas Vondra <[email protected]> Reviewed-by: Tender Wang <[email protected]> Reviewed-by: Álvaro Herrera <[email protected]> Backpatch-through: 16 Discussion: https://postgr.es/m/[email protected] --- src/backend/optimizer/path/joinpath.c | 53 +++++++++++++++++++++------ src/include/nodes/pathnodes.h | 4 ++ src/test/regress/expected/join.out | 35 ++++++++++++++++++ src/test/regress/sql/join.sql | 15 ++++++++ 4 files changed, 95 insertions(+), 12 deletions(-) diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c index 8aef90389a4..3855bda6907 100644 --- a/src/backend/optimizer/path/joinpath.c +++ b/src/backend/optimizer/path/joinpath.c @@ -153,6 +153,7 @@ add_paths_to_joinrel(PlannerInfo *root, extra.mergeclause_list = NIL; extra.sjinfo = sjinfo; extra.param_source_rels = NULL; + extra.ojrelids = NULL; /* * See if the inner relation is provably unique for this outer rel. @@ -278,6 +279,28 @@ add_paths_to_joinrel(PlannerInfo *root, extra.param_source_rels = bms_add_members(extra.param_source_rels, joinrel->lateral_relids); + /* + * Identify the outer joins computed at this join, since it'd be + * nonsensical to use an input path that is parameterized by any of them + * (see try_nestloop_path). + * + * Besides sjinfo's own OJ, this includes any pushed-down OJs that are + * completed at this join per outer-join identity 3; those are the members + * of the joinrel's relids that are in neither input rel. (We add + * sjinfo->ojrelid explicitly because it won't be in the joinrel's relids + * if this join has itself been pushed down into the RHS of a lower outer + * join.) + * + * If sjinfo has no ojrelid, then no outer joins can be computed here; see + * add_outer_joins_to_relids. + */ + if (sjinfo->ojrelid != 0) + { + extra.ojrelids = bms_difference(joinrel->relids, outerrel->relids); + extra.ojrelids = bms_del_members(extra.ojrelids, innerrel->relids); + extra.ojrelids = bms_add_member(extra.ojrelids, sjinfo->ojrelid); + } + /* * 1. Consider mergejoin paths where both relations must be explicitly * sorted. Skip this if we can't mergejoin. @@ -838,11 +861,13 @@ try_nestloop_path(PlannerInfo *root, * 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.) + * to what is in an input relation not what its parameters are.) The same + * goes for any pushed-down outer joins that are completed at this join, + * so we must check against all of extra->ojrelids, not just + * sjinfo->ojrelid. */ - if (extra->sjinfo->ojrelid != 0 && - (bms_is_member(extra->sjinfo->ojrelid, inner_paramrels) || - bms_is_member(extra->sjinfo->ojrelid, outer_paramrels))) + if (bms_overlap(extra->ojrelids, inner_paramrels) || + bms_overlap(extra->ojrelids, outer_paramrels)) return; /* @@ -1050,11 +1075,13 @@ try_mergejoin_path(PlannerInfo *root, * 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.) + * to what is in an input relation not what its parameters are.) The same + * goes for any pushed-down outer joins that are completed at this join, + * so we must check against all of extra->ojrelids, not just + * sjinfo->ojrelid. */ - if (extra->sjinfo->ojrelid != 0 && - (bms_is_member(extra->sjinfo->ojrelid, PATH_REQ_OUTER(inner_path)) || - bms_is_member(extra->sjinfo->ojrelid, PATH_REQ_OUTER(outer_path)))) + if (bms_overlap(extra->ojrelids, PATH_REQ_OUTER(inner_path)) || + bms_overlap(extra->ojrelids, PATH_REQ_OUTER(outer_path))) return; /* @@ -1207,11 +1234,13 @@ try_hashjoin_path(PlannerInfo *root, * 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.) + * to what is in an input relation not what its parameters are.) The same + * goes for any pushed-down outer joins that are completed at this join, + * so we must check against all of extra->ojrelids, not just + * sjinfo->ojrelid. */ - if (extra->sjinfo->ojrelid != 0 && - (bms_is_member(extra->sjinfo->ojrelid, PATH_REQ_OUTER(inner_path)) || - bms_is_member(extra->sjinfo->ojrelid, PATH_REQ_OUTER(outer_path)))) + if (bms_overlap(extra->ojrelids, PATH_REQ_OUTER(inner_path)) || + bms_overlap(extra->ojrelids, PATH_REQ_OUTER(outer_path))) return; /* diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index 14a939c69c3..28e48d955dd 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -3248,6 +3248,9 @@ typedef struct SemiAntiJoinFactors * sjinfo is extra info about special joins for selectivity estimation * semifactors is as shown above (only valid for SEMI/ANTI/inner_unique joins) * param_source_rels are OK targets for parameterization of result paths + * ojrelids are the outer joins computed at this join (sjinfo's own OJ, if + * any, plus any pushed-down OJs completed here); input paths must not + * be parameterized by any of these */ typedef struct JoinPathExtraData { @@ -3257,6 +3260,7 @@ typedef struct JoinPathExtraData SpecialJoinInfo *sjinfo; SemiAntiJoinFactors semifactors; Relids param_source_rels; + Relids ojrelids; } JoinPathExtraData; /* diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index b734bac3d49..6d2b71a72db 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -5788,6 +5788,41 @@ select 1 from t t1 (2 rows) rollback; +-- +-- check a similar case where the parameterization refers to an outer join +-- that was pushed down per outer-join identity 3 +-- +explain (costs off) +select 1 from tenk1 t1 +where t1.thousand in + (select t2.thousand + from tenk1 t2 + left join tenk1 t3 on t2.thousand = t3.thousand + left join (tenk1 t4 left join tenk1 t5 on t4.thousand = t5.thousand) + on t3.thousand = t4.thousand + where coalesce(t5.tenthous, 0) = t2.tenthous); + QUERY PLAN +-------------------------------------------------------------------------------------------- + Hash Semi Join + Hash Cond: (t1.thousand = t2.thousand) + -> Index Only Scan using tenk1_thous_tenthous on tenk1 t1 + -> Hash + -> Hash Left Join + Hash Cond: (t3.thousand = t4.thousand) + Filter: (COALESCE(t5.tenthous, 0) = t2.tenthous) + -> Merge Left Join + Merge Cond: (t2.thousand = t3.thousand) + -> Index Only Scan using tenk1_thous_tenthous on tenk1 t2 + -> Materialize + -> Index Only Scan using tenk1_thous_tenthous on tenk1 t3 + -> Hash + -> Hash Left Join + Hash Cond: (t4.thousand = t5.thousand) + -> Index Only Scan using tenk1_thous_tenthous on tenk1 t4 + -> Hash + -> Index Only Scan using tenk1_thous_tenthous on tenk1 t5 +(18 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 26764c6a7bc..88f5ea82ac5 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -2033,6 +2033,21 @@ select 1 from t t1 rollback; +-- +-- check a similar case where the parameterization refers to an outer join +-- that was pushed down per outer-join identity 3 +-- + +explain (costs off) +select 1 from tenk1 t1 +where t1.thousand in + (select t2.thousand + from tenk1 t2 + left join tenk1 t3 on t2.thousand = t3.thousand + left join (tenk1 t4 left join tenk1 t5 on t4.thousand = t5.thousand) + on t3.thousand = t4.thousand + where coalesce(t5.tenthous, 0) = t2.tenthous); + -- -- check a case in which a PlaceHolderVar forces join order -- -- 2.55.0
From 38206fdcd8058e883c464342701ed4d240010a53 Mon Sep 17 00:00:00 2001 From: Tomas Vondra <[email protected]> Date: Sun, 8 Nov 2026 18:55:04 +0100 Subject: [PATCH v3-16] Reject join input paths parameterized by pushed-down outer joins. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit a798660ebe3 taught try_nestloop_path(), try_mergejoin_path() and try_hashjoin_path() to reject an input path with a nonsensical parameterization. But that check has a gap, making it possible to hit the assertion in get_joinrel_parampathinfo() added by 5b7b5518d0ea. We need to reject all paths whose parameterization includes the outer join being formed at the current join, since a value that is supposed to be nulled by that outer join can't be available before the join is done. That check considered only sjinfo->ojrelid, but that's insufficient because of joins pushed down into the RHS per outer-join identity 3. That relid is added only at the join that completes it, with the relid of the outer join performed there (see add_outer_joins_to_relids). This omission allowed building paths parameterized by a pushed-down outer join, which is still nonsensical. In non-assert builds this is mostly harmless - we still reject the path, just a bit later. To fix, determine the full set of outer joins computed at the current join. That includes sjinfo's own OJ plus any pushed-down OJs completed there. We do this once in add_paths_to_joinrel(), and save it in a new JoinPathExtraData field, and check the input paths' parameterization against that whole set. The partial-path variants need no change, since they never accept input paths whose parameterization isn't satisfied by the outer rel. Backpatch to 16, where 2489d76c490 made Vars outer-join-aware and 9df8f903eb6 introduced the delayed addition of pushed-down outer join relids. The checks added by a798660ebe3 did not cover this case. The new JoinPathExtraData field needs to go at the end of the struct, to avoid an ABI break. It's allocated only in add_paths_to_joinrel (on stack), and only passed through a pointer, so this should be safe. Author: Tomas Vondra <[email protected]> Reviewed-by: Tender Wang <[email protected]> Reviewed-by: Álvaro Herrera <[email protected]> Backpatch-through: 16 Discussion: https://postgr.es/m/[email protected] --- src/backend/optimizer/path/joinpath.c | 53 +++++++++++++++++++++------ src/include/nodes/pathnodes.h | 4 ++ src/test/regress/expected/join.out | 35 ++++++++++++++++++ src/test/regress/sql/join.sql | 15 ++++++++ 4 files changed, 95 insertions(+), 12 deletions(-) diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c index ab69f989a3e..b13779d4ed8 100644 --- a/src/backend/optimizer/path/joinpath.c +++ b/src/backend/optimizer/path/joinpath.c @@ -154,6 +154,7 @@ add_paths_to_joinrel(PlannerInfo *root, extra.mergeclause_list = NIL; extra.sjinfo = sjinfo; extra.param_source_rels = NULL; + extra.ojrelids = NULL; /* * See if the inner relation is provably unique for this outer rel. @@ -279,6 +280,28 @@ add_paths_to_joinrel(PlannerInfo *root, extra.param_source_rels = bms_add_members(extra.param_source_rels, joinrel->lateral_relids); + /* + * Identify the outer joins computed at this join, since it'd be + * nonsensical to use an input path that is parameterized by any of them + * (see try_nestloop_path). + * + * Besides sjinfo's own OJ, this includes any pushed-down OJs that are + * completed at this join per outer-join identity 3; those are the members + * of the joinrel's relids that are in neither input rel. (We add + * sjinfo->ojrelid explicitly because it won't be in the joinrel's relids + * if this join has itself been pushed down into the RHS of a lower outer + * join.) + * + * If sjinfo has no ojrelid, then no outer joins can be computed here; see + * add_outer_joins_to_relids. + */ + if (sjinfo->ojrelid != 0) + { + extra.ojrelids = bms_difference(joinrel->relids, outerrel->relids); + extra.ojrelids = bms_del_members(extra.ojrelids, innerrel->relids); + extra.ojrelids = bms_add_member(extra.ojrelids, sjinfo->ojrelid); + } + /* * 1. Consider mergejoin paths where both relations must be explicitly * sorted. Skip this if we can't mergejoin. @@ -838,11 +861,13 @@ try_nestloop_path(PlannerInfo *root, * 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.) + * to what is in an input relation not what its parameters are.) The same + * goes for any pushed-down outer joins that are completed at this join, + * so we must check against all of extra->ojrelids, not just + * sjinfo->ojrelid. */ - if (extra->sjinfo->ojrelid != 0 && - (bms_is_member(extra->sjinfo->ojrelid, inner_paramrels) || - bms_is_member(extra->sjinfo->ojrelid, outer_paramrels))) + if (bms_overlap(extra->ojrelids, inner_paramrels) || + bms_overlap(extra->ojrelids, outer_paramrels)) return; /* @@ -1059,11 +1084,13 @@ try_mergejoin_path(PlannerInfo *root, * 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.) + * to what is in an input relation not what its parameters are.) The same + * goes for any pushed-down outer joins that are completed at this join, + * so we must check against all of extra->ojrelids, not just + * sjinfo->ojrelid. */ - if (extra->sjinfo->ojrelid != 0 && - (bms_is_member(extra->sjinfo->ojrelid, PATH_REQ_OUTER(inner_path)) || - bms_is_member(extra->sjinfo->ojrelid, PATH_REQ_OUTER(outer_path)))) + if (bms_overlap(extra->ojrelids, PATH_REQ_OUTER(inner_path)) || + bms_overlap(extra->ojrelids, PATH_REQ_OUTER(outer_path))) return; /* @@ -1216,11 +1243,13 @@ try_hashjoin_path(PlannerInfo *root, * 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.) + * to what is in an input relation not what its parameters are.) The same + * goes for any pushed-down outer joins that are completed at this join, + * so we must check against all of extra->ojrelids, not just + * sjinfo->ojrelid. */ - if (extra->sjinfo->ojrelid != 0 && - (bms_is_member(extra->sjinfo->ojrelid, PATH_REQ_OUTER(inner_path)) || - bms_is_member(extra->sjinfo->ojrelid, PATH_REQ_OUTER(outer_path)))) + if (bms_overlap(extra->ojrelids, PATH_REQ_OUTER(inner_path)) || + bms_overlap(extra->ojrelids, PATH_REQ_OUTER(outer_path))) return; /* diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index 299fe031b83..759fb423c35 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -3197,6 +3197,9 @@ typedef struct SemiAntiJoinFactors * sjinfo is extra info about special joins for selectivity estimation * semifactors is as shown above (only valid for SEMI/ANTI/inner_unique joins) * param_source_rels are OK targets for parameterization of result paths + * ojrelids are the outer joins computed at this join (sjinfo's own OJ, if + * any, plus any pushed-down OJs completed here); input paths must not + * be parameterized by any of these */ typedef struct JoinPathExtraData { @@ -3206,6 +3209,7 @@ typedef struct JoinPathExtraData SpecialJoinInfo *sjinfo; SemiAntiJoinFactors semifactors; Relids param_source_rels; + Relids ojrelids; } JoinPathExtraData; /* diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index 1082f3dc8a0..92923365ebe 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -5789,6 +5789,41 @@ select 1 from t t1 (2 rows) rollback; +-- +-- check a similar case where the parameterization refers to an outer join +-- that was pushed down per outer-join identity 3 +-- +explain (costs off) +select 1 from tenk1 t1 +where t1.thousand in + (select t2.thousand + from tenk1 t2 + left join tenk1 t3 on t2.thousand = t3.thousand + left join (tenk1 t4 left join tenk1 t5 on t4.thousand = t5.thousand) + on t3.thousand = t4.thousand + where coalesce(t5.tenthous, 0) = t2.tenthous); + QUERY PLAN +-------------------------------------------------------------------------------------------- + Hash Semi Join + Hash Cond: (t1.thousand = t2.thousand) + -> Index Only Scan using tenk1_thous_tenthous on tenk1 t1 + -> Hash + -> Hash Left Join + Hash Cond: (t3.thousand = t4.thousand) + Filter: (COALESCE(t5.tenthous, 0) = t2.tenthous) + -> Merge Left Join + Merge Cond: (t2.thousand = t3.thousand) + -> Index Only Scan using tenk1_thous_tenthous on tenk1 t2 + -> Materialize + -> Index Only Scan using tenk1_thous_tenthous on tenk1 t3 + -> Hash + -> Hash Left Join + Hash Cond: (t4.thousand = t5.thousand) + -> Index Only Scan using tenk1_thous_tenthous on tenk1 t4 + -> Hash + -> Index Only Scan using tenk1_thous_tenthous on tenk1 t5 +(18 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 546d43f3281..05f07e3c412 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -2033,6 +2033,21 @@ select 1 from t t1 rollback; +-- +-- check a similar case where the parameterization refers to an outer join +-- that was pushed down per outer-join identity 3 +-- + +explain (costs off) +select 1 from tenk1 t1 +where t1.thousand in + (select t2.thousand + from tenk1 t2 + left join tenk1 t3 on t2.thousand = t3.thousand + left join (tenk1 t4 left join tenk1 t5 on t4.thousand = t5.thousand) + on t3.thousand = t4.thousand + where coalesce(t5.tenthous, 0) = t2.tenthous); + -- -- check a case in which a PlaceHolderVar forces join order -- -- 2.55.0
