From 0dcfda62dcf59fe70e2448d2b44d2b253a848bae Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 15 May 2023 19:24:57 -0400 Subject: [PATCH v5 1/2] Split SpecialJoinInfo.commute_below into LHS and RHS parts. Storing just a single relid set seems to have been a poor decision, as there are more places where that adds effort than removes effort compared to having separate LHS and RHS sets. Up to now, none of the involved places were performance-critical, but the next patch in this series will add a usage that's hit multiple times per outer join joinrel. So let's split the field into two parts, making it more like commute_above_[lr]. --- src/backend/optimizer/path/costsize.c | 6 ++- src/backend/optimizer/path/joinrels.c | 3 +- src/backend/optimizer/plan/analyzejoins.c | 3 +- src/backend/optimizer/plan/initsplan.c | 60 ++++++++++------------- src/backend/optimizer/util/orclauses.c | 3 +- src/include/nodes/pathnodes.h | 17 ++++--- 6 files changed, 47 insertions(+), 45 deletions(-) diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c index 0a2562c149..3039186530 100644 --- a/src/backend/optimizer/path/costsize.c +++ b/src/backend/optimizer/path/costsize.c @@ -4788,7 +4788,8 @@ compute_semi_anti_join_factors(PlannerInfo *root, norm_sjinfo.ojrelid = 0; norm_sjinfo.commute_above_l = NULL; norm_sjinfo.commute_above_r = NULL; - norm_sjinfo.commute_below = NULL; + norm_sjinfo.commute_below_l = NULL; + norm_sjinfo.commute_below_r = NULL; /* we don't bother trying to make the remaining fields valid */ norm_sjinfo.lhs_strict = false; norm_sjinfo.semi_can_btree = false; @@ -4956,7 +4957,8 @@ approx_tuple_count(PlannerInfo *root, JoinPath *path, List *quals) sjinfo.ojrelid = 0; sjinfo.commute_above_l = NULL; sjinfo.commute_above_r = NULL; - sjinfo.commute_below = NULL; + sjinfo.commute_below_l = NULL; + sjinfo.commute_below_r = NULL; /* we don't bother trying to make the remaining fields valid */ sjinfo.lhs_strict = false; sjinfo.semi_can_btree = false; diff --git a/src/backend/optimizer/path/joinrels.c b/src/backend/optimizer/path/joinrels.c index 4c6ea3a2f0..c0d9c50270 100644 --- a/src/backend/optimizer/path/joinrels.c +++ b/src/backend/optimizer/path/joinrels.c @@ -740,7 +740,8 @@ make_join_rel(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2) sjinfo->ojrelid = 0; sjinfo->commute_above_l = NULL; sjinfo->commute_above_r = NULL; - sjinfo->commute_below = NULL; + sjinfo->commute_below_l = NULL; + sjinfo->commute_below_r = NULL; /* we don't bother trying to make the remaining fields valid */ sjinfo->lhs_strict = false; sjinfo->semi_can_btree = false; diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c index 657b6e5907..1116ad978e 100644 --- a/src/backend/optimizer/plan/analyzejoins.c +++ b/src/backend/optimizer/plan/analyzejoins.c @@ -399,7 +399,8 @@ remove_rel_from_query(PlannerInfo *root, int relid, int ojrelid, /* relid cannot appear in these fields, but ojrelid can: */ sjinfo->commute_above_l = bms_del_member(sjinfo->commute_above_l, ojrelid); sjinfo->commute_above_r = bms_del_member(sjinfo->commute_above_r, ojrelid); - sjinfo->commute_below = bms_del_member(sjinfo->commute_below, ojrelid); + sjinfo->commute_below_l = bms_del_member(sjinfo->commute_below_l, ojrelid); + sjinfo->commute_below_r = bms_del_member(sjinfo->commute_below_r, ojrelid); } /* diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c index 06f90882c4..5cbb7b9a86 100644 --- a/src/backend/optimizer/plan/initsplan.c +++ b/src/backend/optimizer/plan/initsplan.c @@ -1213,8 +1213,8 @@ deconstruct_distribute(PlannerInfo *root, JoinTreeItem *jtitem) * positioning decisions will be made later, when we revisit the * postponed clauses. */ - if (sjinfo->commute_below) - ojscope = bms_add_members(ojscope, sjinfo->commute_below); + ojscope = bms_add_members(ojscope, sjinfo->commute_below_l); + ojscope = bms_add_members(ojscope, sjinfo->commute_below_r); } else postponed_oj_qual_list = NULL; @@ -1400,7 +1400,8 @@ make_outerjoininfo(PlannerInfo *root, /* these fields may get added to later: */ sjinfo->commute_above_l = NULL; sjinfo->commute_above_r = NULL; - sjinfo->commute_below = NULL; + sjinfo->commute_below_l = NULL; + sjinfo->commute_below_r = NULL; compute_semijoin_info(root, sjinfo, clause); @@ -1643,37 +1644,30 @@ make_outerjoininfo(PlannerInfo *root, * Now that we've identified the correct min_lefthand and min_righthand, * any commute_below_l or commute_below_r relids that have not gotten * added back into those sets (due to intervening outer joins) are indeed - * commutable with this one. Update the derived data in the - * SpecialJoinInfos. + * commutable with this one. + * + * First, delete any subsequently-added-back relids (this is easier than + * maintaining commute_below_l/r precisely through all the above). */ + commute_below_l = bms_del_members(commute_below_l, min_lefthand); + commute_below_r = bms_del_members(commute_below_r, min_righthand); + + /* Anything left? */ if (commute_below_l || commute_below_r) { - Relids commute_below; - - /* - * Delete any subsequently-added-back relids (this is easier than - * maintaining commute_below_l/r precisely through all the above). - */ - commute_below_l = bms_del_members(commute_below_l, min_lefthand); - commute_below_r = bms_del_members(commute_below_r, min_righthand); - - /* Anything left? */ - commute_below = bms_union(commute_below_l, commute_below_r); - if (!bms_is_empty(commute_below)) + /* Yup, so we must update the derived data in the SpecialJoinInfos */ + sjinfo->commute_below_l = commute_below_l; + sjinfo->commute_below_r = commute_below_r; + foreach(l, root->join_info_list) { - /* Yup, so we must update the data structures */ - sjinfo->commute_below = commute_below; - foreach(l, root->join_info_list) - { - SpecialJoinInfo *otherinfo = (SpecialJoinInfo *) lfirst(l); - - if (bms_is_member(otherinfo->ojrelid, commute_below_l)) - otherinfo->commute_above_l = - bms_add_member(otherinfo->commute_above_l, ojrelid); - else if (bms_is_member(otherinfo->ojrelid, commute_below_r)) - otherinfo->commute_above_r = - bms_add_member(otherinfo->commute_above_r, ojrelid); - } + SpecialJoinInfo *otherinfo = (SpecialJoinInfo *) lfirst(l); + + if (bms_is_member(otherinfo->ojrelid, commute_below_l)) + otherinfo->commute_above_l = + bms_add_member(otherinfo->commute_above_l, ojrelid); + else if (bms_is_member(otherinfo->ojrelid, commute_below_r)) + otherinfo->commute_above_r = + bms_add_member(otherinfo->commute_above_r, ojrelid); } } @@ -1889,8 +1883,7 @@ deconstruct_distribute_oj_quals(PlannerInfo *root, * as-is. */ Assert(sjinfo->lhs_strict); /* else we shouldn't be here */ - if (sjinfo->commute_above_r || - bms_overlap(sjinfo->commute_below, sjinfo->syn_lefthand)) + if (sjinfo->commute_above_r || sjinfo->commute_below_l) { Relids joins_above; Relids joins_below; @@ -1901,8 +1894,7 @@ deconstruct_distribute_oj_quals(PlannerInfo *root, /* Identify the outer joins this one commutes with */ joins_above = sjinfo->commute_above_r; - joins_below = bms_intersect(sjinfo->commute_below, - sjinfo->syn_lefthand); + joins_below = sjinfo->commute_below_l; /* * Generate qual variants with different sets of nullingrels bits. diff --git a/src/backend/optimizer/util/orclauses.c b/src/backend/optimizer/util/orclauses.c index 85ecdfc14f..ca8b5ff92f 100644 --- a/src/backend/optimizer/util/orclauses.c +++ b/src/backend/optimizer/util/orclauses.c @@ -334,7 +334,8 @@ consider_new_or_clause(PlannerInfo *root, RelOptInfo *rel, sjinfo.ojrelid = 0; sjinfo.commute_above_l = NULL; sjinfo.commute_above_r = NULL; - sjinfo.commute_below = NULL; + sjinfo.commute_below_l = NULL; + sjinfo.commute_below_r = NULL; /* we don't bother trying to make the remaining fields valid */ sjinfo.lhs_strict = false; sjinfo.semi_can_btree = false; diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index 7d4f24d250..23dd671bf4 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -2783,11 +2783,15 @@ typedef struct PlaceHolderVar * 3, when this join is in the RHS of the upper join (so, this is the lower * join in the second form of the identity). * - * commute_below is filled with the relids of syntactically-lower outer joins - * that have been found to commute with this one per outer join identity 3. - * (We need not record which side they are on, since that can be determined - * by seeing whether the lower join's relid appears in syn_lefthand or - * syn_righthand.) + * commute_below_l is filled with the relids of syntactically-lower outer + * joins that have been found to commute with this one per outer join identity + * 3 and are in the LHS of this join (so, this is the upper join in the first + * form of the identity). + * + * commute_below_r is filled with the relids of syntactically-lower outer + * joins that have been found to commute with this one per outer join identity + * 3 and are in the RHS of this join (so, this is the upper join in the second + * form of the identity). * * lhs_strict is true if the special join's condition cannot succeed when the * LHS variables are all NULL (this means that an outer join can commute with @@ -2829,7 +2833,8 @@ struct SpecialJoinInfo Index ojrelid; /* outer join's RT index; 0 if none */ Relids commute_above_l; /* commuting OJs above this one, if LHS */ Relids commute_above_r; /* commuting OJs above this one, if RHS */ - Relids commute_below; /* commuting OJs below this one */ + Relids commute_below_l; /* commuting OJs in this one's LHS */ + Relids commute_below_r; /* commuting OJs in this one's RHS */ bool lhs_strict; /* joinclause is strict for some LHS rel */ /* Remaining fields are set only for JOIN_SEMI jointype: */ bool semi_can_btree; /* true if semi_operators are all btree */ -- 2.31.1