On Thu, Dec 18, 2025 at 8:36 AM Robert Haas <[email protected]> wrote:
> What must be happening here is that either pgpa_join.c (maybe with
> complicity from pgpa_walker.c) is not populating the
> pgpa_plan_walker_context's join_strategies[JSTRAT_NESTED_LOOP_PLAIN]
> member correctly, or else pgpa_output.c is not serializing it to text
> correctly. I suspect the former is a more likely but I'm not sure
> exactly what's happening.

I think I see the problem: pgpa_process_unrolled_join() returns a set
called "all_relids" but it only returns the union of the inner relid
sets, not including the outer relid set. In your example, we want to
get:

NESTED_LOOP_PLAIN((part partsupp) (supplier part partsupp))

But the join order is:

JOIN_ORDER(nation (supplier (part partsupp)))

So every table is the outer table of some unrolled join, except for
the innermost table, which is partsupp. So all the others get omitted
from the output, and we get the output you saw:

NESTED_LOOP_PLAIN(partsupp partsupp)

Proposed fix attached.

-- 
Robert Haas
EDB: http://www.enterprisedb.com
commit aa085a812c2db077b86caff366d31624b9a06ecf
Author: Robert Haas <[email protected]>
Date:   Thu Jan 8 15:57:34 2026 -0500

    fix pgpa_process_unrolled_join return value

diff --git a/contrib/pg_plan_advice/expected/join_order.out 
b/contrib/pg_plan_advice/expected/join_order.out
index eaaa38f000c..f3dd7810484 100644
--- a/contrib/pg_plan_advice/expected/join_order.out
+++ b/contrib/pg_plan_advice/expected/join_order.out
@@ -159,7 +159,7 @@ SELECT * FROM jo_fact f
    JOIN_ORDER(f (d1 d2)) /* matched */
  Generated Plan Advice:
    JOIN_ORDER(f (d1 d2))
-   MERGE_JOIN_PLAIN(d2)
+   MERGE_JOIN_PLAIN((d1 d2))
    NESTED_LOOP_MATERIALIZE(d2)
    SEQ_SCAN(f d1 d2)
    NO_GATHER(f d1 d2)
diff --git a/contrib/pg_plan_advice/pgpa_walker.c 
b/contrib/pg_plan_advice/pgpa_walker.c
index 4dffc60114a..fdc003ec509 100644
--- a/contrib/pg_plan_advice/pgpa_walker.c
+++ b/contrib/pg_plan_advice/pgpa_walker.c
@@ -438,7 +438,10 @@ static Bitmapset *
 pgpa_process_unrolled_join(pgpa_plan_walker_context *walker,
                                                   pgpa_unrolled_join *ujoin)
 {
-       Bitmapset  *all_relids = NULL;
+       Bitmapset  *all_relids = bms_copy(ujoin->outer.scan->relids);
+
+       /* If this fails, we didn't unroll properly. */
+       Assert(ujoin->outer.unrolled_join == NULL);
 
        for (int k = 0; k < ujoin->ninner; ++k)
        {

Reply via email to