From 28df8bae223f7614fd4bb41623bcca4a4d2c731a Mon Sep 17 00:00:00 2001 From: Richard Guo Date: Wed, 26 Aug 2026 11:42:50 +0900 Subject: [PATCH v1] Some updates to v5 --- src/backend/optimizer/plan/analyzejoins.c | 50 +++++++++++++---------- src/backend/optimizer/prep/prepjointree.c | 7 ++-- src/include/optimizer/prep.h | 1 + src/test/regress/expected/join.out | 11 +++++ src/test/regress/sql/join.sql | 7 ++++ 5 files changed, 51 insertions(+), 25 deletions(-) diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c index 7c77118c76a..a8cb8aaca37 100644 --- a/src/backend/optimizer/plan/analyzejoins.c +++ b/src/backend/optimizer/plan/analyzejoins.c @@ -62,8 +62,7 @@ bool enable_self_join_elimination; static bool join_is_removable(PlannerInfo *root, SpecialJoinInfo *sjinfo); static Node *remove_join_from_jointree(Node *jtnode, int ojrelid, int *nremoved); -static void remove_rels_from_query_tree(PlannerInfo *root, - Relids removed_relids); +static void remove_rels_from_phvs(PlannerInfo *root, Relids removed_relids); static bool reduce_semijoin_in_jointree(Node *jtnode, Relids syn_righthand); static bool rel_supports_distinctness(PlannerInfo *root, RelOptInfo *rel); static bool rel_is_distinct_for(PlannerInfo *root, RelOptInfo *rel, @@ -155,6 +154,10 @@ remove_useless_outer_joins(PlannerInfo *root) removed_relids = bms_add_member(removed_relids, innerrelid); removed_relids = bms_add_member(removed_relids, sjinfo->ojrelid); + /* As in pull_up_simple_subquery, discard a no-longer-needed subquery */ + if (root->simple_rte_array[innerrelid]->rtekind == RTE_SUBQUERY) + root->simple_rte_array[innerrelid]->subquery = NULL; + /* * It's okay to keep scanning join_info_list for more removable joins, * even though the data that join_is_removable consults is now @@ -173,8 +176,8 @@ remove_useless_outer_joins(PlannerInfo *root) if (bms_is_empty(removed_relids)) return false; - /* Clean up the traces that the removed rels have left elsewhere */ - remove_rels_from_query_tree(root, removed_relids); + /* PlaceHolderVars may still mention the removed relids in their phrels */ + remove_rels_from_phvs(root, removed_relids); return true; } @@ -408,37 +411,42 @@ remove_join_from_jointree(Node *jtnode, int ojrelid, int *nremoved) } /* - * remove_rels_from_query_tree - * Delete all remaining references to the given relids from the query. + * remove_rels_from_phvs + * Strip the given relids out of all PlaceHolderVars in the query. * - * Having removed some relations and outer joins from the jointree, we must - * get rid of any references to them that are left behind elsewhere. There - * should be no ordinary Vars of a removed relation left, but the relids can - * still appear in the nullingrels sets of surviving Vars and PlaceHolderVars, - * and in the phrels sets of PlaceHolderVars. ChangeVarNodes knows how to - * strip a relid out of all of those. + * join_is_removable ensures that no Var of a removed rel survives, nor any + * Var or PlaceHolderVar nulled by a removed outer join. What can survive is + * a PlaceHolderVar whose phrels includes the removed relids even though its + * expression doesn't reference them, so just strip them from phrels. */ static void -remove_rels_from_query_tree(PlannerInfo *root, Relids removed_relids) +remove_rels_from_phvs(PlannerInfo *root, Relids removed_relids) { int relid = -1; + /* If there are no PHVs anywhere, we needn't work hard */ + if (root->glob->lastPHId == 0) + return; + while ((relid = bms_next_member(removed_relids, relid)) >= 0) { - /* Pass -1 for new_index to get the removal behavior */ - ChangeVarNodes((Node *) root->parse, relid, -1, 0); + /* Pass NULL for subrelids to get the removal behavior */ + substitute_phv_relids((Node *) root->parse, relid, NULL); /* * processed_tlist shares some but not all of its nodes with - * parse->targetList, so it has to be processed separately. (That's - * harmless: ChangeVarNodes works in-place, and removing a relid that - * isn't there is idempotent.) + * parse->targetList, so it has to be processed separately. */ - ChangeVarNodes((Node *) root->processed_tlist, relid, -1, 0); + substitute_phv_relids((Node *) root->processed_tlist, relid, NULL); /* There could be references in the append_rel_list, too */ - if (root->append_rel_list != NIL) - ChangeVarNodes((Node *) root->append_rel_list, relid, -1, 0); + foreach_node(AppendRelInfo, appinfo, root->append_rel_list) + { + Assert(appinfo->parent_relid != relid); + Assert(appinfo->child_relid != relid); + substitute_phv_relids((Node *) appinfo->translated_vars, + relid, NULL); + } } } diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c index 5931501e470..05f0d8644ce 100644 --- a/src/backend/optimizer/prep/prepjointree.c +++ b/src/backend/optimizer/prep/prepjointree.c @@ -181,8 +181,6 @@ static bool find_dependent_phvs(PlannerInfo *root, int varno, Relids baserels); static bool find_dependent_phvs_in_jointree(PlannerInfo *root, Node *node, int varno, Relids baserels); -static void substitute_phv_relids(Node *node, - int varno, Relids subrelids); static void fix_append_rel_relids(PlannerInfo *root, int varno, Relids subrelids); static Node *find_jointree_node_for_rel(Node *jtnode, int relid); @@ -4580,10 +4578,11 @@ find_dependent_phvs_in_jointree(PlannerInfo *root, Node *node, int varno, /* * substitute_phv_relids - adjust PlaceHolderVar relid sets after pulling up - * a subquery or removing an RTE_RESULT jointree item + * a subquery or removing a jointree item * * Find any PlaceHolderVar nodes in the given tree that reference the * pulled-up relid, and change them to reference the replacement relid(s). + * If subrelids is empty, the relid is simply deleted. * * NOTE: although this has the form of a walker, we cheat and modify the * nodes in-place. This should be OK since the tree was copied by @@ -4641,7 +4640,7 @@ substitute_phv_relids_walker(Node *node, return expression_tree_walker(node, substitute_phv_relids_walker, context); } -static void +void substitute_phv_relids(Node *node, int varno, Relids subrelids) { substitute_phv_relids_context context; diff --git a/src/include/optimizer/prep.h b/src/include/optimizer/prep.h index 00bc567da76..113c2be3243 100644 --- a/src/include/optimizer/prep.h +++ b/src/include/optimizer/prep.h @@ -30,6 +30,7 @@ extern void pull_up_subqueries(PlannerInfo *root); extern void flatten_simple_union_all(PlannerInfo *root); extern void reduce_outer_joins(PlannerInfo *root); extern void remove_useless_result_rtes(PlannerInfo *root); +extern void substitute_phv_relids(Node *node, int varno, Relids subrelids); extern Relids get_relids_in_jointree(Node *jtnode, bool include_outer_joins, bool include_inner_joins); extern Relids get_relids_for_join(Query *query, int joinrelid); diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index db4fcc5a5a0..9f9b28f3334 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -7361,6 +7361,17 @@ SELECT q.val FROM b LEFT JOIN ( -> Seq Scan on a t1 (7 rows) +-- removing a LATERAL subquery can make the rel it references removable too +explain (costs off) +select d.* from d + left join a on d.a = a.id + left join lateral (select count(*) as cnt from c where c.id = a.b_id) s + on true; + QUERY PLAN +--------------- + Seq Scan on d +(1 row) + CREATE TEMP TABLE parted_b (id int PRIMARY KEY) partition by range(id); CREATE TEMP TABLE parted_b1 partition of parted_b for values from (0) to (10); -- test join removals on a partitioned table diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index 9533af8656e..a202e66dc52 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -2712,6 +2712,13 @@ SELECT q.val FROM b LEFT JOIN ( ON true ) AS q ON true; +-- removing a LATERAL subquery can make the rel it references removable too +explain (costs off) +select d.* from d + left join a on d.a = a.id + left join lateral (select count(*) as cnt from c where c.id = a.b_id) s + on true; + CREATE TEMP TABLE parted_b (id int PRIMARY KEY) partition by range(id); CREATE TEMP TABLE parted_b1 partition of parted_b for values from (0) to (10); -- 2.37.1 (Apple Git-137.1)