From 5acb5066d73b738b0861593bbbcf85014f35f8fa Mon Sep 17 00:00:00 2001 From: Richard Guo Date: Thu, 23 Apr 2026 11:19:25 +0900 Subject: [PATCH v2] Fix bogus calls in remove_self_join_rel() remove_self_join_rel() called adjust_relid_set() on all_result_relids and leaf_result_relids but threw away the return value. Since adjust_relid_set() returns a freshly-built Relids and does not modify the input in place, the calls did nothing. This has been the case since the SJE feature went in (commit fc069a3a6). There has been no observable misbehavior, because the relid being passed is guaranteed not to be a member of either set. At the point remove_self_join_rel() runs, those sets contain only resultRelation; inheritance children have not been added yet, as that happens later in query_planner(), in expand_single_inheritance_child() called from add_other_rels_to_query(). And remove_self_joins_recurse() rejects parse->resultRelation as an SJE candidate to preserve the EvalPlanQual mechanism. Even with the result assigned, the calls would be no-ops in practice. Rather than make the calls do the cleanup they pretend to do, replace them with assertions of the invariant. Any future loosening of the SJE candidate filter -- for instance to allow eliminating a result relation under provable conditions -- will trip the assertion and force whoever does it to revisit this code. Additionally, decorate adjust_relid_set() with pg_nodiscard so that any future accidental discard of its return value is caught at compile time. --- src/backend/optimizer/plan/analyzejoins.c | 12 ++++++++++-- src/include/rewrite/rewriteManip.h | 2 +- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c index 03056bdf3e0..0f82ab9facb 100644 --- a/src/backend/optimizer/plan/analyzejoins.c +++ b/src/backend/optimizer/plan/analyzejoins.c @@ -1994,8 +1994,16 @@ remove_self_join_rel(PlannerInfo *root, PlanRowMark *kmark, PlanRowMark *rmark, ChangeVarNodesExtended((Node *) root->processed_tlist, toRemove->relid, toKeep->relid, 0, replace_relid_callback); - adjust_relid_set(root->all_result_relids, toRemove->relid, toKeep->relid); - adjust_relid_set(root->leaf_result_relids, toRemove->relid, toKeep->relid); + /* + * No need to touch all_result_relids or leaf_result_relids: at this point + * those sets contain only parse->resultRelation; inheritance children + * have not been added yet; that happens later in add_other_rels_to_query. + * And remove_self_joins_recurse rejects parse->resultRelation as an SJE + * candidate to preserve the EPQ mechanism. So toRemove->relid cannot be + * a member. + */ + Assert(!bms_is_member(toRemove->relid, root->all_result_relids)); + Assert(!bms_is_member(toRemove->relid, root->leaf_result_relids)); /* * There may be references to the rel in root->fkey_list, but if so, diff --git a/src/include/rewrite/rewriteManip.h b/src/include/rewrite/rewriteManip.h index a6d4e888e06..9d234cb8741 100644 --- a/src/include/rewrite/rewriteManip.h +++ b/src/include/rewrite/rewriteManip.h @@ -54,7 +54,7 @@ struct ChangeVarNodes_context ChangeVarNodes_callback callback; }; -extern Relids adjust_relid_set(Relids relids, int oldrelid, int newrelid); +pg_nodiscard extern Relids adjust_relid_set(Relids relids, int oldrelid, int newrelid); extern void CombineRangeTables(List **dst_rtable, List **dst_perminfos, List *src_rtable, List *src_perminfos); extern void OffsetVarNodes(Node *node, int offset, int sublevels_up); -- 2.39.5 (Apple Git-154)