From 842ab3ecbfa911e0472601c0bb421b384fccca9b Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Mon, 5 Oct 2026 11:40:57 -0400 Subject: [PATCH v7 4/5] pg_plan_advice: Don't suppress NO_GATHER advice for parent rels. When a table is partitioned, the previous code did not emit NO_GATHER advice for the parent table, on the theory that doing this for the child tables was good enough. However, it isn't. I believe what happened here is that, during development, I was planning to enforce NO_GATHER by clearing the consider_parallel flag for the children, which would have been good enough to ensure that parallelism could not be used at any higher level of the plan tree. Eventually, I switched course, but failed to realize that this code needed to be updated as a result. Do that, and add a regression test. Backpatch-through: 19 --- contrib/pg_plan_advice/expected/gather.out | 66 ++++++++++++++++ .../pg_plan_advice/expected/partitionwise.out | 76 ++++++++++--------- contrib/pg_plan_advice/pgpa_scan.c | 7 +- contrib/pg_plan_advice/sql/gather.sql | 26 +++++++ 4 files changed, 134 insertions(+), 41 deletions(-) diff --git a/contrib/pg_plan_advice/expected/gather.out b/contrib/pg_plan_advice/expected/gather.out index 0cc0dedf859..0650381479a 100644 --- a/contrib/pg_plan_advice/expected/gather.out +++ b/contrib/pg_plan_advice/expected/gather.out @@ -15,6 +15,14 @@ CREATE TABLE gt_fact ( INSERT INTO gt_fact SELECT g, (g%3)+1 FROM generate_series(1,100000) g; VACUUM ANALYZE gt_fact; +CREATE TABLE gt_part (id int not null, val int not null) + PARTITION BY RANGE (id); +CREATE TABLE gt_part1 PARTITION OF gt_part FOR VALUES FROM (0) TO (1000) + WITH (autovacuum_enabled = false); +CREATE TABLE gt_part2 PARTITION OF gt_part FOR VALUES FROM (1000) TO (2000) + WITH (autovacuum_enabled = false); +INSERT INTO gt_part SELECT g, g FROM generate_series(0, 1999) g; +VACUUM ANALYZE gt_part; -- By default, we expect Gather Merge with a parallel hash join. EXPLAIN (COSTS OFF, PLAN_ADVICE) SELECT * FROM gt_fact f JOIN gt_dim d ON f.dim_id = d.id ORDER BY d.id; @@ -369,3 +377,61 @@ EXPLAIN (COSTS OFF, PLAN_ADVICE) (17 rows) COMMIT; +-- Test interaction of NO_GATHER with partitioned tables. By default, we +-- expect Gather over a Parallel Append, but no_gather(gt_part) should +-- suppress it, even if we prune down to a single partition such that the +-- Append itself doesn't appear in the plan. Previously, we had a bug where +-- NO_GATHER for the parent was omitted from the generated output even when +-- NO_GATHER was supplied, so this also serves to verify that this is no +-- longer happening. +BEGIN; +EXPLAIN (COSTS OFF, PLAN_ADVICE) + SELECT * FROM gt_part WHERE val = 42; + QUERY PLAN +------------------------------------------------------------- + Gather + Workers Planned: 1 + -> Parallel Append + -> Parallel Seq Scan on gt_part1 gt_part_1 + Filter: (val = 42) + -> Parallel Seq Scan on gt_part2 gt_part_2 + Filter: (val = 42) + Generated Plan Advice: + SEQ_SCAN(gt_part/public.gt_part1 gt_part/public.gt_part2) + PARTITIONWISE(gt_part) + GATHER(gt_part) +(11 rows) + +SET LOCAL pg_plan_advice.advice = 'no_gather(gt_part)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) + SELECT * FROM gt_part WHERE val = 42; + QUERY PLAN +---------------------------------------------------------------------- + Append + -> Seq Scan on gt_part1 gt_part_1 + Filter: (val = 42) + -> Seq Scan on gt_part2 gt_part_2 + Filter: (val = 42) + Supplied Plan Advice: + NO_GATHER(gt_part) /* matched */ + Generated Plan Advice: + SEQ_SCAN(gt_part/public.gt_part1 gt_part/public.gt_part2) + PARTITIONWISE(gt_part) + NO_GATHER(gt_part gt_part/public.gt_part1 gt_part/public.gt_part2) +(11 rows) + +EXPLAIN (COSTS OFF, PLAN_ADVICE) + SELECT * FROM gt_part WHERE id < 500 AND val = 42; + QUERY PLAN +---------------------------------------------- + Seq Scan on gt_part1 gt_part + Filter: ((id < 500) AND (val = 42)) + Supplied Plan Advice: + NO_GATHER(gt_part) /* matched */ + Generated Plan Advice: + SEQ_SCAN(gt_part/public.gt_part1) + PARTITIONWISE(gt_part) + NO_GATHER(gt_part gt_part/public.gt_part1) +(8 rows) + +COMMIT; diff --git a/contrib/pg_plan_advice/expected/partitionwise.out b/contrib/pg_plan_advice/expected/partitionwise.out index f3f7f8ed621..3a3f87007e4 100644 --- a/contrib/pg_plan_advice/expected/partitionwise.out +++ b/contrib/pg_plan_advice/expected/partitionwise.out @@ -70,8 +70,8 @@ CREATE TABLE mllpt_a2_b3 PARTITION OF mllpt_a2 FOR VALUES IN (3) EXPLAIN (PLAN_ADVICE, COSTS OFF) SELECT * FROM pt1, pt2, pt3 WHERE pt1.id = pt2.id AND pt2.id = pt3.id AND val1 = 1 AND val2 = 1 AND val3 = 1; - QUERY PLAN -------------------------------------------------------------------------------------- + QUERY PLAN +-------------------------------------------------------------------------------- Append -> Nested Loop -> Hash Join @@ -117,9 +117,10 @@ SELECT * FROM pt1, pt2, pt3 WHERE pt1.id = pt2.id AND pt2.id = pt3.id INDEX_SCAN(pt1/public.pt1a public.pt1a_pkey pt1/public.pt1b public.pt1b_pkey pt1/public.pt1c public.pt1c_pkey) PARTITIONWISE((pt1 pt2 pt3)) - NO_GATHER(pt1/public.pt1a pt1/public.pt1b pt1/public.pt1c pt2/public.pt2a - pt2/public.pt2b pt2/public.pt2c pt3/public.pt3a pt3/public.pt3b pt3/public.pt3c) -(47 rows) + NO_GATHER(pt1 pt2 pt3 pt1/public.pt1a pt1/public.pt1b pt1/public.pt1c + pt2/public.pt2a pt2/public.pt2b pt2/public.pt2c pt3/public.pt3a + pt3/public.pt3b pt3/public.pt3c) +(48 rows) -- Suppress partitionwise join, or do it just partially. BEGIN; @@ -127,8 +128,8 @@ SET LOCAL pg_plan_advice.advice = 'PARTITIONWISE(pt1 pt2 pt3)'; EXPLAIN (PLAN_ADVICE, COSTS OFF) SELECT * FROM pt1, pt2, pt3 WHERE pt1.id = pt2.id AND pt2.id = pt3.id AND val1 = 1 AND val2 = 1 AND val3 = 1; - QUERY PLAN -------------------------------------------------------------------------------------- + QUERY PLAN +-------------------------------------------------------------------------------- Nested Loop -> Hash Join Hash Cond: (pt2.id = pt3.id) @@ -170,16 +171,17 @@ SELECT * FROM pt1, pt2, pt3 WHERE pt1.id = pt2.id AND pt2.id = pt3.id INDEX_SCAN(pt1/public.pt1a public.pt1a_pkey pt1/public.pt1b public.pt1b_pkey pt1/public.pt1c public.pt1c_pkey) PARTITIONWISE(pt2 pt3 pt1) - NO_GATHER(pt1/public.pt1a pt1/public.pt1b pt1/public.pt1c pt2/public.pt2a - pt2/public.pt2b pt2/public.pt2c pt3/public.pt3a pt3/public.pt3b pt3/public.pt3c) -(43 rows) + NO_GATHER(pt1 pt2 pt3 pt1/public.pt1a pt1/public.pt1b pt1/public.pt1c + pt2/public.pt2a pt2/public.pt2b pt2/public.pt2c pt3/public.pt3a + pt3/public.pt3b pt3/public.pt3c) +(44 rows) SET LOCAL pg_plan_advice.advice = 'PARTITIONWISE((pt1 pt2) pt3)'; EXPLAIN (PLAN_ADVICE, COSTS OFF) SELECT * FROM pt1, pt2, pt3 WHERE pt1.id = pt2.id AND pt2.id = pt3.id AND val1 = 1 AND val2 = 1 AND val3 = 1; - QUERY PLAN -------------------------------------------------------------------------------------- + QUERY PLAN +---------------------------------------------------------------------------- Hash Join Hash Cond: (pt1.id = pt3.id) -> Append @@ -225,9 +227,10 @@ SELECT * FROM pt1, pt2, pt3 WHERE pt1.id = pt2.id AND pt2.id = pt3.id pt1/public.pt1c pt2/public.pt2c pt3/public.pt3a pt3/public.pt3b pt3/public.pt3c) PARTITIONWISE((pt1 pt2) pt3) - NO_GATHER(pt1/public.pt1a pt1/public.pt1b pt1/public.pt1c pt2/public.pt2a - pt2/public.pt2b pt2/public.pt2c pt3/public.pt3a pt3/public.pt3b pt3/public.pt3c) -(47 rows) + NO_GATHER(pt1 pt2 pt3 pt1/public.pt1a pt1/public.pt1b pt1/public.pt1c + pt2/public.pt2a pt2/public.pt2b pt2/public.pt2c pt3/public.pt3a + pt3/public.pt3b pt3/public.pt3c) +(48 rows) COMMIT; -- Test conflicting advice. @@ -236,8 +239,8 @@ SET LOCAL pg_plan_advice.advice = 'PARTITIONWISE((pt1 pt2) (pt1 pt3))'; EXPLAIN (PLAN_ADVICE, COSTS OFF) SELECT * FROM pt1, pt2, pt3 WHERE pt1.id = pt2.id AND pt2.id = pt3.id AND val1 = 1 AND val2 = 1 AND val3 = 1; - QUERY PLAN -------------------------------------------------------------------------------------- + QUERY PLAN +-------------------------------------------------------------------------------- Append Disabled: true -> Nested Loop @@ -287,9 +290,10 @@ SELECT * FROM pt1, pt2, pt3 WHERE pt1.id = pt2.id AND pt2.id = pt3.id INDEX_SCAN(pt1/public.pt1a public.pt1a_pkey pt1/public.pt1b public.pt1b_pkey pt1/public.pt1c public.pt1c_pkey) PARTITIONWISE((pt1 pt2 pt3)) - NO_GATHER(pt1/public.pt1a pt1/public.pt1b pt1/public.pt1c pt2/public.pt2a - pt2/public.pt2b pt2/public.pt2c pt3/public.pt3a pt3/public.pt3b pt3/public.pt3c) -(51 rows) + NO_GATHER(pt1 pt2 pt3 pt1/public.pt1a pt1/public.pt1b pt1/public.pt1c + pt2/public.pt2a pt2/public.pt2b pt2/public.pt2c pt3/public.pt3a + pt3/public.pt3b pt3/public.pt3c) +(52 rows) COMMIT; -- Test use of join order for the partitionwise join case. @@ -298,8 +302,8 @@ SET LOCAL pg_plan_advice.advice = 'PARTITIONWISE((pt1 pt2)) JOIN_ORDER({pt1 pt2} EXPLAIN (PLAN_ADVICE, COSTS OFF) SELECT * FROM pt1, pt2, pt3 WHERE pt1.id = pt2.id AND pt2.id = pt3.id AND val1 = 1 AND val2 = 1 AND val3 = 1; - QUERY PLAN -------------------------------------------------------------------------------------- + QUERY PLAN +---------------------------------------------------------------------------- Hash Join Hash Cond: (pt1.id = pt3.id) -> Append @@ -345,9 +349,10 @@ SELECT * FROM pt1, pt2, pt3 WHERE pt1.id = pt2.id AND pt2.id = pt3.id pt1/public.pt1c pt2/public.pt2c pt3/public.pt3a pt3/public.pt3b pt3/public.pt3c) PARTITIONWISE((pt1 pt2) pt3) - NO_GATHER(pt1/public.pt1a pt1/public.pt1b pt1/public.pt1c pt2/public.pt2a - pt2/public.pt2b pt2/public.pt2c pt3/public.pt3a pt3/public.pt3b pt3/public.pt3c) -(47 rows) + NO_GATHER(pt1 pt2 pt3 pt1/public.pt1a pt1/public.pt1b pt1/public.pt1c + pt2/public.pt2a pt2/public.pt2b pt2/public.pt2c pt3/public.pt3a + pt3/public.pt3b pt3/public.pt3c) +(48 rows) COMMIT; -- Can't force a partitionwise join with a mismatched table. @@ -355,8 +360,8 @@ BEGIN; SET LOCAL pg_plan_advice.advice = 'PARTITIONWISE((pt1 ptmismatch))'; EXPLAIN (PLAN_ADVICE, COSTS OFF) SELECT * FROM pt1, ptmismatch WHERE pt1.id = ptmismatch.id; - QUERY PLAN ---------------------------------------------------------------------------- + QUERY PLAN +---------------------------------------------------------------------------- Nested Loop Disabled: true -> Append @@ -377,7 +382,7 @@ SELECT * FROM pt1, ptmismatch WHERE pt1.id = ptmismatch.id; INDEX_SCAN(ptmismatch/public.ptmismatcha public.ptmismatcha_pkey ptmismatch/public.ptmismatchb public.ptmismatchb_pkey) PARTITIONWISE(pt1 ptmismatch) - NO_GATHER(pt1/public.pt1a pt1/public.pt1b pt1/public.pt1c + NO_GATHER(pt1 ptmismatch pt1/public.pt1a pt1/public.pt1b pt1/public.pt1c ptmismatch/public.ptmismatcha ptmismatch/public.ptmismatchb) (22 rows) @@ -388,8 +393,8 @@ SET LOCAL pg_plan_advice.advice = 'JOIN_ORDER(pt3/public.pt3a pt2/public.pt2a pt EXPLAIN (PLAN_ADVICE, COSTS OFF) SELECT * FROM pt1, pt2, pt3 WHERE pt1.id = pt2.id AND pt2.id = pt3.id AND val1 = 1 AND val2 = 1 AND val3 = 1; - QUERY PLAN -------------------------------------------------------------------------------------- + QUERY PLAN +-------------------------------------------------------------------------------- Append -> Nested Loop -> Hash Join @@ -437,9 +442,10 @@ SELECT * FROM pt1, pt2, pt3 WHERE pt1.id = pt2.id AND pt2.id = pt3.id INDEX_SCAN(pt1/public.pt1a public.pt1a_pkey pt1/public.pt1b public.pt1b_pkey pt1/public.pt1c public.pt1c_pkey) PARTITIONWISE((pt1 pt2 pt3)) - NO_GATHER(pt1/public.pt1a pt1/public.pt1b pt1/public.pt1c pt2/public.pt2a - pt2/public.pt2b pt2/public.pt2c pt3/public.pt3a pt3/public.pt3b pt3/public.pt3c) -(49 rows) + NO_GATHER(pt1 pt2 pt3 pt1/public.pt1a pt1/public.pt1b pt1/public.pt1c + pt2/public.pt2a pt2/public.pt2b pt2/public.pt2c pt3/public.pt3a + pt3/public.pt3b pt3/public.pt3c) +(50 rows) COMMIT; -- We should get PARTITIONWISE advice for all unpruned partition tables. @@ -469,9 +475,9 @@ SELECT * FROM mllpt WHERE a = 1 UNION ALL SELECT * FROM mllpt; mllpt/public.mllpt_a2_b2 mllpt/public.mllpt_a2_b3) PARTITIONWISE(mllpt mllpt/public.mllpt_a1 mllpt/public.mllpt_a2 mllpt/public.mllpt_a1@unnamed_subquery mllpt@unnamed_subquery) - NO_GATHER(unnamed_subquery#3 mllpt/public.mllpt_a1_b1 + NO_GATHER(unnamed_subquery unnamed_subquery#3 mllpt/public.mllpt_a1_b1 mllpt/public.mllpt_a1_b2 mllpt/public.mllpt_a1_b3 mllpt/public.mllpt_a2_b1 - mllpt/public.mllpt_a2_b2 mllpt/public.mllpt_a2_b3 + mllpt/public.mllpt_a2_b2 mllpt/public.mllpt_a2_b3 mllpt@unnamed_subquery mllpt/public.mllpt_a1_b1@unnamed_subquery mllpt/public.mllpt_a1_b2@unnamed_subquery mllpt/public.mllpt_a1_b3@unnamed_subquery) (27 rows) diff --git a/contrib/pg_plan_advice/pgpa_scan.c b/contrib/pg_plan_advice/pgpa_scan.c index 762ba642a17..fccc5c7a763 100644 --- a/contrib/pg_plan_advice/pgpa_scan.c +++ b/contrib/pg_plan_advice/pgpa_scan.c @@ -229,13 +229,8 @@ pgpa_build_scan(pgpa_plan_walker_context *walker, Plan *plan, * * Add nothing if we're beneath a Gather or Gather Merge node, since * NO_GATHER advice is clearly inappropriate in that situation. - * - * Add nothing if this is an Append or MergeAppend node, whether or not - * elided. We'll emit NO_GATHER() for the underlying scan, which is good - * enough. */ - if (!beneath_any_gather && nodetype != T_Append && - nodetype != T_MergeAppend) + if (!beneath_any_gather) walker->no_gather_scans = bms_add_members(walker->no_gather_scans, relids); diff --git a/contrib/pg_plan_advice/sql/gather.sql b/contrib/pg_plan_advice/sql/gather.sql index 776666bf196..616be396c5b 100644 --- a/contrib/pg_plan_advice/sql/gather.sql +++ b/contrib/pg_plan_advice/sql/gather.sql @@ -18,6 +18,15 @@ INSERT INTO gt_fact SELECT g, (g%3)+1 FROM generate_series(1,100000) g; VACUUM ANALYZE gt_fact; +CREATE TABLE gt_part (id int not null, val int not null) + PARTITION BY RANGE (id); +CREATE TABLE gt_part1 PARTITION OF gt_part FOR VALUES FROM (0) TO (1000) + WITH (autovacuum_enabled = false); +CREATE TABLE gt_part2 PARTITION OF gt_part FOR VALUES FROM (1000) TO (2000) + WITH (autovacuum_enabled = false); +INSERT INTO gt_part SELECT g, g FROM generate_series(0, 1999) g; +VACUUM ANALYZE gt_part; + -- By default, we expect Gather Merge with a parallel hash join. EXPLAIN (COSTS OFF, PLAN_ADVICE) SELECT * FROM gt_fact f JOIN gt_dim d ON f.dim_id = d.id ORDER BY d.id; @@ -84,3 +93,20 @@ SET LOCAL pg_plan_advice.advice = 'gather((f d)) no_gather(f)'; EXPLAIN (COSTS OFF, PLAN_ADVICE) SELECT * FROM gt_fact f JOIN gt_dim d ON f.dim_id = d.id ORDER BY d.id; COMMIT; + +-- Test interaction of NO_GATHER with partitioned tables. By default, we +-- expect Gather over a Parallel Append, but no_gather(gt_part) should +-- suppress it, even if we prune down to a single partition such that the +-- Append itself doesn't appear in the plan. Previously, we had a bug where +-- NO_GATHER for the parent was omitted from the generated output even when +-- NO_GATHER was supplied, so this also serves to verify that this is no +-- longer happening. +BEGIN; +EXPLAIN (COSTS OFF, PLAN_ADVICE) + SELECT * FROM gt_part WHERE val = 42; +SET LOCAL pg_plan_advice.advice = 'no_gather(gt_part)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) + SELECT * FROM gt_part WHERE val = 42; +EXPLAIN (COSTS OFF, PLAN_ADVICE) + SELECT * FROM gt_part WHERE id < 500 AND val = 42; +COMMIT; -- 2.53.0