From 30c0836daf68b649a64d277f507913e2559dc29c Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Wed, 25 Jun 2025 16:08:20 -0400 Subject: [PATCH] filter join RTIs out of clumped joins to avoid failures --- contrib/pg_plan_advice/pgpa_join.c | 45 ++++++++++++++++++++++------ contrib/pg_plan_advice/pgpa_join.h | 3 +- contrib/pg_plan_advice/pgpa_walker.c | 4 +-- 3 files changed, 40 insertions(+), 12 deletions(-) diff --git a/contrib/pg_plan_advice/pgpa_join.c b/contrib/pg_plan_advice/pgpa_join.c index 66408ab0c1..adfa6e0279 100644 --- a/contrib/pg_plan_advice/pgpa_join.c +++ b/contrib/pg_plan_advice/pgpa_join.c @@ -15,7 +15,9 @@ #include "pgpa_join.h" #include "pgpa_walker.h" +#include "nodes/pathnodes.h" #include "nodes/print.h" +#include "parser/parsetree.h" /* * Temporary object used when unrolling a join tree. @@ -38,7 +40,8 @@ static pgpa_join_strategy pgpa_decompose_join(PlannedStmt *pstmt, Plan **realinner, ElidedNode **elidedrealouter, ElidedNode **elidedrealinner); -static void pgpa_fix_scan_or_clump_member(pgpa_join_member *member); +static void pgpa_fix_scan_or_clump_member(PlannedStmt *pstmt, + pgpa_join_member *member); static ElidedNode *pgpa_descend_node(PlannedStmt *pstmt, Plan **plan); static ElidedNode *pgpa_descend_any_gather(PlannedStmt *pstmt, Plan **plan); static ElidedNode *pgpa_descend_any_unique(PlannedStmt *pstmt, Plan **plan); @@ -83,9 +86,12 @@ pgpa_get_join_class(Plan *plan) * one as elided_node, else pass NULL. */ pgpa_clumped_join * -pgpa_build_clumped_join(Plan *plan, ElidedNode *elided_node) +pgpa_build_clumped_join(PlannedStmt *pstmt, Plan *plan, + ElidedNode *elided_node) { pgpa_clumped_join *clump = palloc(sizeof(pgpa_clumped_join)); + Bitmapset *relids = NULL; + int rti = -1; clump->plan = plan; @@ -101,7 +107,7 @@ pgpa_build_clumped_join(Plan *plan, ElidedNode *elided_node) * case it's not a join. */ Assert(bms_membership(elided_node->relids) == BMS_MULTIPLE); - clump->relids = elided_node->relids; + relids = elided_node->relids; if (elided_type == T_Append || elided_type == T_MergeAppend) clump->strategy = JSTRAT_CLUMP_PARTITIONWISE; else @@ -111,7 +117,7 @@ pgpa_build_clumped_join(Plan *plan, ElidedNode *elided_node) { Assert(pgpa_get_join_class(plan) == PGPA_CLUMPED_JOIN); - clump->relids = pgpa_relids(plan); + relids = pgpa_relids(plan); if (IsA(plan, Result)) clump->strategy = JSTRAT_CLUMP_DEGENERATE; @@ -125,6 +131,27 @@ pgpa_build_clumped_join(Plan *plan, ElidedNode *elided_node) elog(ERROR, "unexpected plan type"); } + /* + * Filter out any RTIs of type RTE_JOIN, since we have no use for them, + * and don't want them creating confusion later. (We always refer to + * groups of relations in terms of the relation RTIs, not the join RTIs.) + */ + clump->relids = NULL; + while ((rti = bms_next_member(relids, rti)) >= 0) + { + RangeTblEntry *rte = rt_fetch(rti, pstmt->rtable); + + if (rte->rtekind != RTE_JOIN) + clump->relids = bms_add_member(clump->relids, rti); + } + + /* + * We concluded that this was a join based on the fact that there were + * multiple RTIs -- and even after removing the join RTIs, that should + * still be the case. + */ + Assert(bms_membership(clump->relids) == BMS_MULTIPLE); + return clump; } @@ -294,7 +321,7 @@ pgpa_build_unrolled_join(PlannedStmt *pstmt, /* Handle the outermost join. */ ujoin->outer.plan = join_unroller->outer_subplan; ujoin->outer.elided_node = join_unroller->outer_elided_node; - pgpa_fix_scan_or_clump_member(&ujoin->outer); + pgpa_fix_scan_or_clump_member(pstmt, &ujoin->outer); /* * We want the joins from the deepest part of the plan tree to appear @@ -320,7 +347,7 @@ pgpa_build_unrolled_join(PlannedStmt *pstmt, pgpa_build_unrolled_join(pstmt, join_unroller->inner_unrollers[k]); else - pgpa_fix_scan_or_clump_member(&ujoin->inner[i]); + pgpa_fix_scan_or_clump_member(pstmt, &ujoin->inner[i]); } return ujoin; @@ -502,7 +529,7 @@ pgpa_decompose_join(PlannedStmt *pstmt, Plan *plan, * pgpa_join_member. */ static void -pgpa_fix_scan_or_clump_member(pgpa_join_member *member) +pgpa_fix_scan_or_clump_member(PlannedStmt *pstmt, pgpa_join_member *member) { if (member->elided_node != NULL) { @@ -513,7 +540,7 @@ pgpa_fix_scan_or_clump_member(pgpa_join_member *member) */ if (bms_membership(member->elided_node->relids) == BMS_MULTIPLE) member->clump_join = - pgpa_build_clumped_join(member->plan, + pgpa_build_clumped_join(pstmt, member->plan, member->elided_node); else member->rti = bms_singleton_member(member->elided_node->relids); @@ -526,7 +553,7 @@ pgpa_fix_scan_or_clump_member(pgpa_join_member *member) */ if (pgpa_get_join_class(member->plan) == PGPA_CLUMPED_JOIN) member->clump_join = - pgpa_build_clumped_join(member->plan, + pgpa_build_clumped_join(pstmt, member->plan, member->elided_node); else { diff --git a/contrib/pg_plan_advice/pgpa_join.h b/contrib/pg_plan_advice/pgpa_join.h index 969603e634..0b03cc1b08 100644 --- a/contrib/pg_plan_advice/pgpa_join.h +++ b/contrib/pg_plan_advice/pgpa_join.h @@ -140,7 +140,8 @@ typedef struct pgpa_gathered_join } pgpa_gathered_join; extern pgpa_join_class pgpa_get_join_class(Plan *plan); -extern pgpa_clumped_join *pgpa_build_clumped_join(Plan *plan, +extern pgpa_clumped_join *pgpa_build_clumped_join(PlannedStmt *pstmt, + Plan *plan, ElidedNode *elided_node); extern pgpa_join_unroller *pgpa_create_join_unroller(void); diff --git a/contrib/pg_plan_advice/pgpa_walker.c b/contrib/pg_plan_advice/pgpa_walker.c index 56badfb029..fcac8a45db 100644 --- a/contrib/pg_plan_advice/pgpa_walker.c +++ b/contrib/pg_plan_advice/pgpa_walker.c @@ -78,7 +78,7 @@ pgpa_plan_walker(pgpa_plan_walker_context *context, Plan *plan, { pgpa_clumped_join *cjoin; - cjoin = pgpa_build_clumped_join(plan, n); + cjoin = pgpa_build_clumped_join(context->pstmt, plan, n); context->clumped_joins = lappend(context->clumped_joins, cjoin); } } @@ -138,7 +138,7 @@ pgpa_plan_walker(pgpa_plan_walker_context *context, Plan *plan, { pgpa_clumped_join *cjoin; - cjoin = pgpa_build_clumped_join(plan, NULL); + cjoin = pgpa_build_clumped_join(context->pstmt, plan, NULL); context->clumped_joins = lappend(context->clumped_joins, cjoin); } } -- 2.39.5