From cc1178cfe3302034bfe20e58de0766622487aa77 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 16 May 2023 22:44:11 -0400 Subject: [PATCH v6 4/5] Fix additions of nullingrels to joinrels' output targetlists. Don't mark Vars and PHVs prematurely in build_joinrel_tlist, and consider the transitive closure of previously-pushed-down outer joins to see if additional marking is needed. Richard Guo Discussion: https://postgr.es/m/0b819232-4b50-f245-1c7d-c8c61bf41827@postgrespro.ru --- src/backend/optimizer/util/relnode.c | 60 ++++++++++++++++++++++++---- 1 file changed, 52 insertions(+), 8 deletions(-) diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c index 0d849d9494..e6d2d7a353 100644 --- a/src/backend/optimizer/util/relnode.c +++ b/src/backend/optimizer/util/relnode.c @@ -42,6 +42,7 @@ typedef struct JoinHashEntry static void build_joinrel_tlist(PlannerInfo *root, RelOptInfo *joinrel, RelOptInfo *input_rel, + Relids pushed_down_ojrelids, SpecialJoinInfo *sjinfo, bool can_null); static List *build_joinrel_restrictlist(PlannerInfo *root, @@ -649,6 +650,7 @@ build_join_rel(PlannerInfo *root, { RelOptInfo *joinrel; List *restrictlist; + Relids pushed_down_ojrelids; /* This function should be used only for join between parents. */ Assert(!IS_OTHER_REL(outer_rel) && !IS_OTHER_REL(inner_rel)); @@ -757,9 +759,13 @@ build_join_rel(PlannerInfo *root, * and inner rels we first try to build it from. But the contents should * be the same regardless. */ - build_joinrel_tlist(root, joinrel, outer_rel, sjinfo, + pushed_down_ojrelids = bms_difference(joinrel->relids, + bms_union(outer_rel->relids, + inner_rel->relids)); + pushed_down_ojrelids = bms_del_member(pushed_down_ojrelids, sjinfo->ojrelid); + build_joinrel_tlist(root, joinrel, outer_rel, pushed_down_ojrelids, sjinfo, (sjinfo->jointype == JOIN_FULL)); - build_joinrel_tlist(root, joinrel, inner_rel, sjinfo, + build_joinrel_tlist(root, joinrel, inner_rel, pushed_down_ojrelids, sjinfo, (sjinfo->jointype != JOIN_INNER)); add_placeholders_to_joinrel(root, joinrel, outer_rel, inner_rel, sjinfo); @@ -1046,17 +1052,24 @@ min_join_parameterization(PlannerInfo *root, * identity 3 (see optimizer/README). We must take steps to ensure that * the output Vars have the same nulling bitmaps that they would if the * two joins had been done in syntactic order; else they won't match Vars - * appearing higher in the query tree. We need to do two things: + * appearing higher in the query tree. We need to do three things: * - * First, we add the outer join's relid to the nulling bitmap only if the Var - * or PHV actually comes from within the syntactically nullable side(s) of the - * outer join. This takes care of the possibility that we have transformed + * First, we add the outer join's relid to the nulling bitmap only if the + * outer join has been completely performed and the Var or PHV actually + * comes from within the syntactically nullable side(s) of the outer join. + * This takes care of the possibility that we have transformed * (A leftjoin B on (Pab)) leftjoin C on (Pbc) * to * A leftjoin (B leftjoin C on (Pbc)) on (Pab) - * Here the now-upper A/B join must not mark C columns as nulled by itself. + * Here the pushed-down B/C join cannot mark C columns as nulled yet, + * while the now-upper A/B join must not mark C columns as nulled by itself. * - * Second, any relid in sjinfo->commute_above_r that is already part of + * Second, perform the same operation for each outer-join relid listed in + * pushed_down_ojrelids (which, in this example, would be "C" when we are + * at the now-upper A/B join). This allows the now-upper join to complete + * the marking of "C" Vars that now have fully valid values. + * + * Third, any relid in sjinfo->commute_above_r that is already part of * the joinrel is added to the nulling bitmaps of nullable Vars and PHVs. * This takes care of the reverse case where we implement * A leftjoin (B leftjoin C on (Pbc)) on (Pab) @@ -1069,6 +1082,7 @@ min_join_parameterization(PlannerInfo *root, static void build_joinrel_tlist(PlannerInfo *root, RelOptInfo *joinrel, RelOptInfo *input_rel, + Relids pushed_down_ojrelids, SpecialJoinInfo *sjinfo, bool can_null) { @@ -1100,11 +1114,26 @@ build_joinrel_tlist(PlannerInfo *root, RelOptInfo *joinrel, phv = copyObject(phv); /* See comments above to understand this logic */ if (sjinfo->ojrelid != 0 && + bms_is_member(sjinfo->ojrelid, relids) && (bms_is_subset(phv->phrels, sjinfo->syn_righthand) || (sjinfo->jointype == JOIN_FULL && bms_is_subset(phv->phrels, sjinfo->syn_lefthand)))) phv->phnullingrels = bms_add_member(phv->phnullingrels, sjinfo->ojrelid); + if (pushed_down_ojrelids) + { + ListCell *lc; + + foreach(lc, root->join_info_list) + { + SpecialJoinInfo *othersj = (SpecialJoinInfo *) lfirst(lc); + + if (bms_is_member(othersj->ojrelid, pushed_down_ojrelids) && + bms_is_subset(phv->phrels, othersj->syn_righthand)) + phv->phnullingrels = bms_add_member(phv->phnullingrels, + othersj->ojrelid); + } + } phv->phnullingrels = bms_join(phv->phnullingrels, bms_intersect(sjinfo->commute_above_r, @@ -1165,11 +1194,26 @@ build_joinrel_tlist(PlannerInfo *root, RelOptInfo *joinrel, var = copyObject(var); /* See comments above to understand this logic */ if (sjinfo->ojrelid != 0 && + bms_is_member(sjinfo->ojrelid, relids) && (bms_is_member(var->varno, sjinfo->syn_righthand) || (sjinfo->jointype == JOIN_FULL && bms_is_member(var->varno, sjinfo->syn_lefthand)))) var->varnullingrels = bms_add_member(var->varnullingrels, sjinfo->ojrelid); + if (pushed_down_ojrelids) + { + ListCell *lc; + + foreach(lc, root->join_info_list) + { + SpecialJoinInfo *othersj = (SpecialJoinInfo *) lfirst(lc); + + if (bms_is_member(othersj->ojrelid, pushed_down_ojrelids) && + bms_is_member(var->varno, othersj->syn_righthand)) + var->varnullingrels = bms_add_member(var->varnullingrels, + othersj->ojrelid); + } + } var->varnullingrels = bms_join(var->varnullingrels, bms_intersect(sjinfo->commute_above_r, -- 2.31.1