From 80db5ca9ece768e45321ca8fb88e2a9e67f8dcb5 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Thu, 8 Oct 2026 14:12:24 -0400 Subject: [PATCH] Add child_append_relid_sets to ElidedNode; use in pg_plan_advice. Commit 0d4391b265f83023d0b7eed71817517410f76e60 taught the planner to store information about the RTIs of SubqueryScan, Append, and MergeAppend node elided from the plan using a new ElidedNode data structure; an hour later, 7358abcc6076f4b2530d10126ab379f8aea612a5 taught to save the RTIs of an Append node into another Append node using a new child_append_relid_sets field. Regrettably, despite the fact that both commits were written by me and the fact that both were part of the same patch set, the latter commit failed to deal with the case where an Append node first acquires child_append_relid_sets != NIL and then gets elided. I believe that I thought that was impossible, but the included test case shows that it isn't. The proper fix involves updating the definition of ElidedNode to carry child_append_relid_sets, so this commit does that, breaking ABI compatibility. That's fine for the master branch, but it's a bit unfortunate to be doing do that so close to the planned ship date for v19. However, shipping v19 without fixing this appears (at least to me) to be even more unfortunate. Backpatch-through: 19 Discussion: http://postgr.es/m/CA+TgmoYYHC4i-Gpzc9-JYHqih6pOK+6LTYFiNR-PioXG1nKkOQ@mail.gmail.com --- .../pg_overexplain/expected/pg_overexplain.out | 9 ++++++--- contrib/pg_overexplain/pg_overexplain.c | 3 +++ .../pg_plan_advice/expected/partitionwise.out | 13 +++++++++++++ contrib/pg_plan_advice/pgpa_scan.c | 3 +++ contrib/pg_plan_advice/sql/partitionwise.sql | 4 ++++ src/backend/optimizer/plan/setrefs.c | 18 +++++++++++++----- src/include/nodes/plannodes.h | 4 ++++ 7 files changed, 46 insertions(+), 8 deletions(-) diff --git a/contrib/pg_overexplain/expected/pg_overexplain.out b/contrib/pg_overexplain/expected/pg_overexplain.out index 0cb2ae29843..445cf405b8b 100644 --- a/contrib/pg_overexplain/expected/pg_overexplain.out +++ b/contrib/pg_overexplain/expected/pg_overexplain.out @@ -592,6 +592,7 @@ SELECT * FROM vegetables WHERE genus = 'daucus'; Scan RTI: 2 Elided Node Type: Append Elided Node RTIs: 1 + Elided Node Child Append RTIs: none RTI 1 (relation, inherited, in-from-clause): Eref: vegetables (id, name, genus) Relation: vegetables @@ -605,7 +606,7 @@ SELECT * FROM vegetables WHERE genus = 'daucus'; Relation Kind: relation Relation Lock Mode: AccessShareLock Unprunable RTIs: 1 2 -(18 rows) +(19 rows) -- Also test a case that involves a write. EXPLAIN (RANGE_TABLE, COSTS OFF) @@ -641,6 +642,7 @@ SELECT * FROM vegetables v, Scan RTI: 6 Elided Node Type: Append Elided Node RTIs: 5 + Elided Node Child Append RTIs: none Elided Node Type: SubqueryScan Elided Node RTIs: 2 -> Append @@ -687,7 +689,7 @@ SELECT * FROM vegetables v, Relation Kind: relation Relation Lock Mode: AccessShareLock Unprunable RTIs: 1 3 4 5 6 -(52 rows) +(53 rows) -- should show "Subplan: unnamed_subquery" EXPLAIN (RANGE_TABLE, COSTS OFF) @@ -701,6 +703,7 @@ SELECT * FROM vegetables v, Scan RTI: 6 Elided Node Type: Append Elided Node RTIs: 5 + Elided Node Child Append RTIs: none Elided Node Type: SubqueryScan Elided Node RTIs: 2 -> Append @@ -746,7 +749,7 @@ SELECT * FROM vegetables v, Relation Kind: relation Relation Lock Mode: AccessShareLock Unprunable RTIs: 1 3 4 5 6 -(51 rows) +(52 rows) -- test display of child append RTIs EXPLAIN (RANGE_TABLE, COSTS OFF) diff --git a/contrib/pg_overexplain/pg_overexplain.c b/contrib/pg_overexplain/pg_overexplain.c index 029fd1d09c5..03d27423aad 100644 --- a/contrib/pg_overexplain/pg_overexplain.c +++ b/contrib/pg_overexplain/pg_overexplain.c @@ -297,6 +297,9 @@ overexplain_per_node_hook(PlanState *planstate, List *ancestors, ExplainOpenGroup("Elided Node", NULL, true, es); ExplainPropertyText("Elided Node Type", elidednodetag, es); overexplain_bitmapset("Elided Node RTIs", n->relids, es); + if (n->elided_type == T_Append || n->elided_type == T_MergeAppend) + overexplain_bitmapset_list("Elided Node Child Append RTIs", + n->child_append_relid_sets, es); ExplainCloseGroup("Elided Node", NULL, true, es); } if (opened_elided_nodes) diff --git a/contrib/pg_plan_advice/expected/partitionwise.out b/contrib/pg_plan_advice/expected/partitionwise.out index e87d35d04b7..cf4fc9077d2 100644 --- a/contrib/pg_plan_advice/expected/partitionwise.out +++ b/contrib/pg_plan_advice/expected/partitionwise.out @@ -476,3 +476,16 @@ SELECT * FROM mllpt WHERE a = 1 UNION ALL SELECT * FROM mllpt; mllpt/public.mllpt_a1_b2@unnamed_subquery mllpt/public.mllpt_a1_b3@unnamed_subquery) (27 rows) +-- Same, but prune down to a single leaf so the Append is elided. +EXPLAIN (PLAN_ADVICE, COSTS OFF) +SELECT * FROM mllpt WHERE a = 1 AND b = 1; + QUERY PLAN +---------------------------------------------- + Seq Scan on mllpt_a1_b1 mllpt + Filter: ((a = 1) AND (b = 1)) + Generated Plan Advice: + SEQ_SCAN(mllpt/public.mllpt_a1_b1) + PARTITIONWISE(mllpt/public.mllpt_a1 mllpt) + NO_GATHER(mllpt/public.mllpt_a1_b1) +(6 rows) + diff --git a/contrib/pg_plan_advice/pgpa_scan.c b/contrib/pg_plan_advice/pgpa_scan.c index ca2128be193..cf247f1de4b 100644 --- a/contrib/pg_plan_advice/pgpa_scan.c +++ b/contrib/pg_plan_advice/pgpa_scan.c @@ -83,6 +83,9 @@ pgpa_build_scan(pgpa_plan_walker_context *walker, Plan *plan, else strategy = PGPA_SCAN_ORDINARY; + /* Be sure to account for pulled-up scans, as for a live Append. */ + child_append_relid_sets = elided_node->child_append_relid_sets; + /* Join RTIs can be present, but advice never refers to them. */ relids = pgpa_filter_out_join_relids(relids, walker->pstmt->rtable); } diff --git a/contrib/pg_plan_advice/sql/partitionwise.sql b/contrib/pg_plan_advice/sql/partitionwise.sql index 940ea49e64d..b295cf27c83 100644 --- a/contrib/pg_plan_advice/sql/partitionwise.sql +++ b/contrib/pg_plan_advice/sql/partitionwise.sql @@ -123,3 +123,7 @@ COMMIT; -- We should get PARTITIONWISE advice for all unpruned partition tables. EXPLAIN (PLAN_ADVICE, COSTS OFF) SELECT * FROM mllpt WHERE a = 1 UNION ALL SELECT * FROM mllpt; + +-- Same, but prune down to a single leaf so the Append is elided. +EXPLAIN (PLAN_ADVICE, COSTS OFF) +SELECT * FROM mllpt WHERE a = 1 AND b = 1; diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c index a37abb7a47d..686b5f94f4f 100644 --- a/src/backend/optimizer/plan/setrefs.c +++ b/src/backend/optimizer/plan/setrefs.c @@ -214,7 +214,8 @@ static List *set_windowagg_runcondition_references(PlannerInfo *root, Plan *plan); static void record_elided_node(PlannerGlobal *glob, int plan_node_id, - NodeTag elided_type, Bitmapset *relids); + NodeTag elided_type, Bitmapset *relids, + List *child_append_relid_sets); /***************************************************************************** @@ -1480,7 +1481,8 @@ set_subqueryscan_references(PlannerInfo *root, /* Remember that we removed a SubqueryScan */ scanrelid = plan->scan.scanrelid + rtoffset; record_elided_node(root->glob, plan->subplan->plan_node_id, - T_SubqueryScan, bms_make_singleton(scanrelid)); + T_SubqueryScan, bms_make_singleton(scanrelid), + NIL); } else { @@ -1915,7 +1917,9 @@ set_append_references(PlannerInfo *root, /* Remember that we removed an Append */ record_elided_node(root->glob, p->plan_node_id, T_Append, - offset_relid_set(aplan->apprelids, rtoffset)); + offset_relid_set(aplan->apprelids, rtoffset), + offset_relid_set_list(aplan->child_append_relid_sets, + rtoffset)); return result; } @@ -1995,7 +1999,9 @@ set_mergeappend_references(PlannerInfo *root, /* Remember that we removed a MergeAppend */ record_elided_node(root->glob, p->plan_node_id, T_MergeAppend, - offset_relid_set(mplan->apprelids, rtoffset)); + offset_relid_set(mplan->apprelids, rtoffset), + offset_relid_set_list(mplan->child_append_relid_sets, + rtoffset)); return result; } @@ -3873,13 +3879,15 @@ extract_query_dependencies_walker(Node *node, PlannerInfo *context) */ static void record_elided_node(PlannerGlobal *glob, int plan_node_id, - NodeTag elided_type, Bitmapset *relids) + NodeTag elided_type, Bitmapset *relids, + List *child_append_relid_sets) { ElidedNode *n = makeNode(ElidedNode); n->plan_node_id = plan_node_id; n->elided_type = elided_type; n->relids = relids; + n->child_append_relid_sets = child_append_relid_sets; glob->elidedNodes = lappend(glob->elidedNodes, n); } diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h index e2c00576d41..3809b44e782 100644 --- a/src/include/nodes/plannodes.h +++ b/src/include/nodes/plannodes.h @@ -1862,6 +1862,9 @@ typedef struct SubPlanRTInfo * * plan_node_id is that of the surviving plan node, the sole child of the * one which was elided. + * + * For an elided Append or MergeAppend, child_append_relid_sets is the + * child_append_relid_sets value from the removed node; otherwise, it is NIL. */ typedef struct ElidedNode { @@ -1869,6 +1872,7 @@ typedef struct ElidedNode int plan_node_id; NodeTag elided_type; Bitmapset *relids; + List *child_append_relid_sets; } ElidedNode; #endif /* PLANNODES_H */