From a971d9edbb83ae068b08812b997464c9bc00b3d9 Mon Sep 17 00:00:00 2001
From: Robert Haas <rhaas@postgresql.org>
Date: Fri, 27 Jun 2025 13:52:17 -0400
Subject: [PATCH] fix duplicate clumped-join creation

---
 contrib/pg_plan_advice/pg_plan_advice.c |  4 +-
 contrib/pg_plan_advice/pgpa_walker.c    | 49 ++++++++++++++-----------
 contrib/pg_plan_advice/pgpa_walker.h    |  1 +
 3 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/contrib/pg_plan_advice/pg_plan_advice.c b/contrib/pg_plan_advice/pg_plan_advice.c
index 4017008d5b..1344bcc169 100644
--- a/contrib/pg_plan_advice/pg_plan_advice.c
+++ b/contrib/pg_plan_advice/pg_plan_advice.c
@@ -59,14 +59,14 @@ pgpa_check_plan(PlannedStmt *pstmt)
 
 	memset(&context, 0, sizeof(pgpa_plan_walker_context));
 	context.pstmt = pstmt;
-	pgpa_plan_walker(&context, pstmt->planTree, NULL, NULL);
+	pgpa_plan_walker(&context, pstmt->planTree, 0, NULL, NULL);
 
 	foreach(lc, pstmt->subplans)
 	{
 		Plan *plan = lfirst(lc);
 
 		if (plan != NULL)
-			pgpa_plan_walker(&context, plan, NULL, NULL);
+			pgpa_plan_walker(&context, plan, 0, NULL, NULL);
 	}
 
 	initStringInfo(&buf);
diff --git a/contrib/pg_plan_advice/pgpa_walker.c b/contrib/pg_plan_advice/pgpa_walker.c
index a4b24e1db0..47c6b37ced 100644
--- a/contrib/pg_plan_advice/pgpa_walker.c
+++ b/contrib/pg_plan_advice/pgpa_walker.c
@@ -7,9 +7,12 @@
 
 /*
  * Iterate over the entire plan tree.
+ *
+ * XXX. More comments.
  */
 void
 pgpa_plan_walker(pgpa_plan_walker_context *context, Plan *plan,
+				 int join_unroll_level,
 				 pgpa_join_unroller *join_unroller,
 				 pgpa_gathered_join *gathered_join)
 {
@@ -123,25 +126,27 @@ pgpa_plan_walker(pgpa_plan_walker_context *context, Plan *plan,
 	class = pgpa_get_join_class(plan);
 
 	/*
-	 * If join_unroller == NULL, then either (a) this join should be unrolled
-	 * but we must create a new join unroller to do so, or (b) this join is
-	 * clumped and we must add it to the toplevel list of clumped joins since
-	 * there's no other place to attach it, or (c) this is not a join.
+	 * If this join needs to unrolled but there's no join unroller already
+	 * available, tcreate one.
 	 */
-	if (join_unroller == NULL)
+	if (class == PGPA_UNROLLED_JOIN && join_unroller == NULL)
 	{
-		if (class == PGPA_UNROLLED_JOIN)
-		{
-			join_unroller = pgpa_create_join_unroller();
-			join_unroller_toplevel = true;
-		}
-		else if (class == PGPA_CLUMPED_JOIN)
-		{
-			pgpa_clumped_join *cjoin;
+		join_unroller = pgpa_create_join_unroller();
+		join_unroller_toplevel = true;
+		++join_unroll_level;
+	}
 
-			cjoin = pgpa_build_clumped_join(context->pstmt, plan, NULL);
-			context->clumped_joins = lappend(context->clumped_joins, cjoin);
-		}
+	/*
+	 * If we're underneath a join unroller, it will create a pgpa_clumped_join
+	 * and attach it to the unrolled join. If we're not, we must create the
+	 * pgpa_clumped_join here.
+	 */
+	if (class == PGPA_CLUMPED_JOIN && join_unroll_level == 0)
+	{
+		pgpa_clumped_join *cjoin;
+
+		cjoin = pgpa_build_clumped_join(context->pstmt, plan, NULL);
+		context->clumped_joins = lappend(context->clumped_joins, cjoin);
 	}
 
 	/*
@@ -162,11 +167,11 @@ pgpa_plan_walker(pgpa_plan_walker_context *context, Plan *plan,
 
 	/* Recurse into the outer and inner subtrees. */
 	if (plan->lefttree != NULL)
-		pgpa_plan_walker(context, plan->lefttree, outer_join_unroller,
-						 gathered_join);
+		pgpa_plan_walker(context, plan->lefttree, join_unroll_level,
+						 outer_join_unroller, gathered_join);
 	if (plan->righttree != NULL)
-		pgpa_plan_walker(context, plan->righttree, inner_join_unroller,
-						 gathered_join);
+		pgpa_plan_walker(context, plan->righttree, join_unroll_level,
+						 inner_join_unroller, gathered_join);
 
 	/*
 	 * If we created a join unroller up above, then it's also our join to use
@@ -202,7 +207,7 @@ pgpa_plan_walker(pgpa_plan_walker_context *context, Plan *plan,
 			break;
 		case T_SubqueryScan:
 			pgpa_plan_walker(context, ((SubqueryScan *) plan)->subplan,
-							 NULL, NULL);
+							 0, NULL, NULL);
 			break;
 		case T_CustomScan:
 			extraplans = ((CustomScan *) plan)->custom_plans;
@@ -216,7 +221,7 @@ pgpa_plan_walker(pgpa_plan_walker_context *context, Plan *plan,
 	{
 		Plan	   *subplan = lfirst(lc);
 
-		pgpa_plan_walker(context, subplan, NULL, NULL);
+		pgpa_plan_walker(context, subplan, 0, NULL, NULL);
 	}
 }
 
diff --git a/contrib/pg_plan_advice/pgpa_walker.h b/contrib/pg_plan_advice/pgpa_walker.h
index 5d16d91ccf..2b8ec48b4b 100644
--- a/contrib/pg_plan_advice/pgpa_walker.h
+++ b/contrib/pg_plan_advice/pgpa_walker.h
@@ -23,6 +23,7 @@ typedef struct pgpa_plan_walker_context
 } pgpa_plan_walker_context;
 
 extern void pgpa_plan_walker(pgpa_plan_walker_context *context, Plan *plan,
+							 int join_unroll_level,
 							 pgpa_join_unroller *join_unroller,
 							 pgpa_gathered_join *gathered_join);
 
-- 
2.39.5