From 41a72c0f2f6ae7f53d54ac67a271ccc993ab5ae1 Mon Sep 17 00:00:00 2001 From: Tender Wang Date: Fri, 23 Oct 2026 11:29:58 +0800 Subject: [PATCH v3] Fix wrong result --- src/backend/optimizer/path/equivclass.c | 58 ++++++++++++++++++++++++- src/backend/optimizer/util/relnode.c | 38 +++++++++++++++- src/include/nodes/pathnodes.h | 5 +++ 3 files changed, 99 insertions(+), 2 deletions(-) diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c index 0bacc24b58c..147637d0a90 100644 --- a/src/backend/optimizer/path/equivclass.c +++ b/src/backend/optimizer/path/equivclass.c @@ -75,6 +75,9 @@ static RestrictInfo *create_join_clause(PlannerInfo *root, EquivalenceMember *leftem, EquivalenceMember *rightem, EquivalenceClass *parent_ec); +static Relids find_deferred_const_oj_relids(PlannerInfo *root, + OuterJoinClauseInfo *ojcinfo, + bool outer_on_left); static bool reconsider_outer_join_clause(PlannerInfo *root, OuterJoinClauseInfo *ojcinfo, bool outer_on_left); @@ -2155,7 +2158,11 @@ create_join_clause(PlannerInfo *root, * seen as a clauseless join and avoided during join order searching. * We handle this by generating a constant-TRUE clause that is marked with * the same required_relids etc as the removed outer-join clause, thus - * making it a join clause between the correct relations. + * making it a join clause between the correct relations. But that's not + * safe for a cloned clause (see deconstruct_distribute_oj_quals) that may be + * evaluated above a pushed-down outer join that has already null-extended + * the OUTERVAR, whose relid is not yet included in the join's relids. There + * we need the original strict clause to reject the null-extended rows. */ void reconsider_outer_join_clauses(PlannerInfo *root) @@ -2181,6 +2188,14 @@ reconsider_outer_join_clauses(PlannerInfo *root) /* remove it from the list */ root->left_join_clauses = foreach_delete_current(root->left_join_clauses, cell); + /* keep the original clause if it can still be needed */ + rinfo->const_oj_relids = + find_deferred_const_oj_relids(root, ojcinfo, true); + if (rinfo->const_oj_relids) + { + distribute_restrictinfo_to_rels(root, rinfo); + continue; + } /* throw back a dummy replacement clause (see notes above) */ rinfo = make_restrictinfo(root, (Expr *) makeBoolConst(true, false), @@ -2209,6 +2224,14 @@ reconsider_outer_join_clauses(PlannerInfo *root) /* remove it from the list */ root->right_join_clauses = foreach_delete_current(root->right_join_clauses, cell); + /* keep the original clause if it can still be needed */ + rinfo->const_oj_relids = + find_deferred_const_oj_relids(root, ojcinfo, false); + if (rinfo->const_oj_relids) + { + distribute_restrictinfo_to_rels(root, rinfo); + continue; + } /* throw back a dummy replacement clause (see notes above) */ rinfo = make_restrictinfo(root, (Expr *) makeBoolConst(true, false), @@ -2274,6 +2297,39 @@ reconsider_outer_join_clauses(PlannerInfo *root) } } +/* + * find_deferred_const_oj_relids + * Find outer joins that can make this clause necessary even though + * constant propagation normally makes it redundant. + * + * Such an outer join must be able to null the clause's outer operand, and + * its relid must be one with which this clone is incompatible. A nonempty + * commute_below_l means that add_outer_joins_to_relids() can defer that + * relid after the join has physically null-extended the operand. + */ +static Relids +find_deferred_const_oj_relids(PlannerInfo *root, + OuterJoinClauseInfo *ojcinfo, + bool outer_on_left) +{ + RestrictInfo *rinfo = ojcinfo->rinfo; + Relids outer_relids; + Relids result = NULL; + + if (!rinfo->has_clone && !rinfo->is_clone) + return NULL; + outer_relids = outer_on_left ? rinfo->left_relids : rinfo->right_relids; + foreach_node(SpecialJoinInfo, othersj, root->join_info_list) + { + if (othersj != ojcinfo->sjinfo && othersj->ojrelid != 0 && + bms_is_member(othersj->ojrelid, rinfo->incompatible_relids) && + !bms_is_empty(othersj->commute_below_l) && + bms_overlap(othersj->syn_righthand, outer_relids)) + result = bms_add_member(result, othersj->ojrelid); + } + return result; +} + /* * reconsider_outer_join_clauses for a single LEFT/RIGHT JOIN clause * diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c index f2ccba52f20..16c551c97a7 100644 --- a/src/backend/optimizer/util/relnode.c +++ b/src/backend/optimizer/util/relnode.c @@ -66,6 +66,9 @@ static List *build_joinrel_restrictlist(PlannerInfo *root, #ifdef USE_ASSERT_CHECKING static bool no_duplicate_clause_serials(List *clauses); #endif +static bool deferred_oj_makes_clause_needed(PlannerInfo *root, + RestrictInfo *rinfo, + Relids available_relids); static void build_joinrel_joinlist(RelOptInfo *joinrel, RelOptInfo *outer_rel, RelOptInfo *inner_rel); @@ -1453,6 +1456,30 @@ no_duplicate_clause_serials(List *clauses) } #endif +/* + * Is a constant-propagated outer-join clause needed at this level? + * + * The clause is needed when one of the outer joins that can null its outer + * operand has already had both minimum inputs formed, but its OJ relid is + * still absent because a commuting lower outer join has not been completed. + */ +static bool +deferred_oj_makes_clause_needed(PlannerInfo *root, RestrictInfo *rinfo, + Relids available_relids) +{ + foreach_node(SpecialJoinInfo, sjinfo, root->join_info_list) + { + if (!bms_is_member(sjinfo->ojrelid, rinfo->const_oj_relids)) + continue; + if (!bms_is_member(sjinfo->ojrelid, available_relids) && + bms_is_subset(sjinfo->min_lefthand, available_relids) && + bms_is_subset(sjinfo->min_righthand, available_relids) && + !bms_is_subset(sjinfo->commute_below_l, available_relids)) + return true; + } + return false; +} + /* * build_joinrel_restrictlist * build_joinrel_joinlist @@ -1599,7 +1626,10 @@ subbuild_joinrel_restrictlist(PlannerInfo *root, bms_is_subset(rinfo->required_relids, both_input_relids)); } - + if (rinfo->const_oj_relids && + !deferred_oj_makes_clause_needed(root, rinfo, + both_input_relids)) + continue; /* * OK, so add it to the list, being careful to eliminate * duplicates. (Since RestrictInfo nodes in different joinlists @@ -1828,6 +1858,9 @@ get_baserel_parampathinfo(PlannerInfo *root, RelOptInfo *baserel, if (bms_is_member(rinfo->rinfo_serial, pserials)) continue; } + if (rinfo->const_oj_relids && + !deferred_oj_makes_clause_needed(root, rinfo, joinrelids)) + continue; pclauses = lappend(pclauses, rinfo); pserials = bms_add_member(pserials, rinfo->rinfo_serial); @@ -1974,6 +2007,9 @@ get_joinrel_parampathinfo(PlannerInfo *root, RelOptInfo *joinrel, if (bms_is_member(rinfo->rinfo_serial, pserials)) continue; } + if (rinfo->const_oj_relids && + !deferred_oj_makes_clause_needed(root, rinfo, join_and_req)) + continue; pclauses = lappend(pclauses, rinfo); pserials = bms_add_member(pserials, rinfo->rinfo_serial); diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index 8615db98a55..9400a368086 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -2933,6 +2933,11 @@ typedef struct RestrictInfo /* Relids above which we cannot evaluate the clause (see comment above) */ Relids incompatible_relids; + /* + * For an outer-join clone made redundant by constant propagation, OJ + * relids whose deferred addition can make the original clause necessary. + */ + Relids const_oj_relids; /* If an outer-join clause, the outer-side relations, else NULL: */ Relids outer_relids; -- 2.43.0