From b90b7bb4cf5066742e0849922cad2473ffc673d0 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Mon, 5 Oct 2026 15:10:34 -0400 Subject: [PATCH v7 3/5] 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 --- .../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 d3ce932be4c..c4857ed80be 100644 --- a/contrib/pg_overexplain/expected/pg_overexplain.out +++ b/contrib/pg_overexplain/expected/pg_overexplain.out @@ -628,6 +628,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 @@ -641,7 +642,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) @@ -677,6 +678,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 @@ -723,7 +725,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) @@ -737,6 +739,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 @@ -782,7 +785,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 4d74316a18e..d71a59d0ac2 100644 --- a/contrib/pg_overexplain/pg_overexplain.c +++ b/contrib/pg_overexplain/pg_overexplain.c @@ -311,6 +311,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..f3f7f8ed621 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 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 61b8f5eb9b7..762ba642a17 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 8aba20af25d..af78892fafc 100644 --- a/src/backend/optimizer/plan/setrefs.c +++ b/src/backend/optimizer/plan/setrefs.c @@ -211,7 +211,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); /***************************************************************************** @@ -1472,7 +1473,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 { @@ -1900,7 +1902,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; } @@ -1980,7 +1984,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; } @@ -3838,13 +3844,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 09a1ec73180..9e538f3c44a 100644 --- a/src/include/nodes/plannodes.h +++ b/src/include/nodes/plannodes.h @@ -1866,6 +1866,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 { @@ -1873,6 +1876,7 @@ typedef struct ElidedNode int plan_node_id; NodeTag elided_type; Bitmapset *relids; + List *child_append_relid_sets; } ElidedNode; #endif /* PLANNODES_H */ -- 2.53.0