filter join RTIs out of clumped joins to avoid failures
authorRobert Haas <rhaas@postgresql.org>
Wed, 25 Jun 2025 20:08:20 +0000 (16:08 -0400)
committerRobert Haas <rhaas@postgresql.org>
Wed, 25 Jun 2025 20:08:20 +0000 (16:08 -0400)
contrib/pg_plan_advice/pgpa_join.c
contrib/pg_plan_advice/pgpa_join.h
contrib/pg_plan_advice/pgpa_walker.c

index 66408ab0c1991332a86261a343ee119282f0f346..adfa6e0279126c6aac9e97bee22b6db1c751eec0 100644 (file)
@@ -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
                {
index 969603e63425d7364fbd887ae6030cd9219b02c3..0b03cc1b086532bf7132facd1b4a79fd00b35eec 100644 (file)
@@ -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);
index 56badfb02999da47dfc55237b91447fb534fd01f..fcac8a45dbd834c31589bf781846cae3beac900c 100644 (file)
@@ -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);
                }
        }