From 64f44fdda9e670223253c9dad58e048e0e44c1ae Mon Sep 17 00:00:00 2001 From: Richard Guo Date: Wed, 28 Jun 2023 14:19:30 +0800 Subject: [PATCH v1] Draft: Avoid invalid parameterized path for joinrel --- src/backend/optimizer/path/joinpath.c | 43 ++++++++++++++++++++------- src/include/nodes/pathnodes.h | 3 ++ src/test/regress/expected/join.out | 31 +++++++++++++++++++ src/test/regress/sql/join.sql | 23 ++++++++++++++ 4 files changed, 89 insertions(+), 11 deletions(-) diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c index c2f211a60d..b54b12ee47 100644 --- a/src/backend/optimizer/path/joinpath.c +++ b/src/backend/optimizer/path/joinpath.c @@ -149,6 +149,7 @@ add_paths_to_joinrel(PlannerInfo *root, extra.mergeclause_list = NIL; extra.sjinfo = sjinfo; extra.param_source_rels = NULL; + extra.param_incompatible_relids = NULL; /* * See if the inner relation is provably unique for this outer rel. @@ -226,14 +227,16 @@ add_paths_to_joinrel(PlannerInfo *root, /* * Decide whether it's sensible to generate parameterized paths for this - * joinrel, and if so, which relations such paths should require. There - * is usually no need to create a parameterized result path unless there - * is a join order restriction that prevents joining one of our input rels - * directly to the parameter source rel instead of joining to the other - * input rel. (But see allow_star_schema_join().) This restriction - * reduces the number of parameterized paths we have to deal with at - * higher join levels, without compromising the quality of the resulting - * plan. We express the restriction as a Relids set that must overlap the + * joinrel, and if so, which relations such paths should require and which + * relations such paths should not require. There is usually no need to + * create a parameterized result path unless there is a join order + * restriction that prevents joining one of our input rels directly to the + * parameter source rel instead of joining to the other input rel. (But + * see allow_star_schema_join().) This restriction reduces the number of + * parameterized paths we have to deal with at higher join levels, without + * compromising the quality of the resulting plan. We express the + * restriction as a Relids set that must overlap the parameterization of + * any proposed join path and a Relids set that must not overlap the * parameterization of any proposed join path. Note: param_source_rels * should contain only baserels, not OJ relids, so starting from * all_baserels not all_query_rels is correct. @@ -262,6 +265,21 @@ add_paths_to_joinrel(PlannerInfo *root, extra.param_source_rels = bms_join(extra.param_source_rels, bms_difference(root->all_baserels, sjinfo2->min_lefthand)); + + /* + * For an OJ that is part of this joinrel, the parameterization of any + * proposed join path must not overlap its relid, as well as anything + * in its LHS and RHS. + */ + if (bms_is_member(sjinfo2->ojrelid, joinrelids)) + { + extra.param_incompatible_relids = + bms_add_member(extra.param_incompatible_relids, sjinfo2->ojrelid); + extra.param_incompatible_relids = + bms_add_members(extra.param_incompatible_relids, sjinfo2->min_lefthand); + extra.param_incompatible_relids = + bms_add_members(extra.param_incompatible_relids, sjinfo2->min_righthand); + } } /* @@ -722,7 +740,8 @@ try_nestloop_path(PlannerInfo *root, required_outer = calc_nestloop_required_outer(outerrelids, outer_paramrels, innerrelids, inner_paramrels); if (required_outer && - ((!bms_overlap(required_outer, extra->param_source_rels) && + (((!bms_overlap(required_outer, extra->param_source_rels) || + bms_overlap(required_outer, extra->param_incompatible_relids)) && !allow_star_schema_join(root, outerrelids, inner_paramrels)) || have_dangerous_phv(root, outerrelids, inner_paramrels))) { @@ -915,7 +934,8 @@ try_mergejoin_path(PlannerInfo *root, required_outer = calc_non_nestloop_required_outer(outer_path, inner_path); if (required_outer && - !bms_overlap(required_outer, extra->param_source_rels)) + (!bms_overlap(required_outer, extra->param_source_rels) || + bms_overlap(required_outer, extra->param_incompatible_relids))) { /* Waste no memory when we reject a path here */ bms_free(required_outer); @@ -1061,7 +1081,8 @@ try_hashjoin_path(PlannerInfo *root, required_outer = calc_non_nestloop_required_outer(outer_path, inner_path); if (required_outer && - !bms_overlap(required_outer, extra->param_source_rels)) + (!bms_overlap(required_outer, extra->param_source_rels) || + bms_overlap(required_outer, extra->param_incompatible_relids))) { /* Waste no memory when we reject a path here */ bms_free(required_outer); diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index c17b53f7ad..52973dc91f 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -3170,6 +3170,8 @@ 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 + * param_incompatible_relids are incompatible targets for parameterization of + * result paths */ typedef struct JoinPathExtraData { @@ -3179,6 +3181,7 @@ typedef struct JoinPathExtraData SpecialJoinInfo *sjinfo; SemiAntiJoinFactors semifactors; Relids param_source_rels; + Relids param_incompatible_relids; } JoinPathExtraData; /* 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 -- -- 2.31.0