From dc06dcf0333e9f9682bd4b36cba4a1412457390c Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Fri, 16 Oct 2026 16:06:10 -0400 Subject: [PATCH v8 2/3] pg_plan_advice: Suppress advice from unflattened set operations. Previously, we could generate GATHER, GATHER_MERGE, or NO_GATHER advice for portions of the plan tree generated by set operation planning. There are no upper-planner hooks we can use to enforce such advice, so we shouldn't generate it, either. Make it so, and also add a bunch of test cases to verify the behavior. Reported-by: Nikolay Samokhvalov --- contrib/pg_plan_advice/expected/gather.out | 165 +++++++++++++++++++-- contrib/pg_plan_advice/pgpa_join.c | 4 +- contrib/pg_plan_advice/pgpa_scan.c | 92 ++++++++++-- contrib/pg_plan_advice/pgpa_scan.h | 4 +- contrib/pg_plan_advice/pgpa_walker.c | 135 ++++++++++------- contrib/pg_plan_advice/pgpa_walker.h | 4 +- contrib/pg_plan_advice/sql/gather.sql | 54 +++++++ 7 files changed, 374 insertions(+), 84 deletions(-) diff --git a/contrib/pg_plan_advice/expected/gather.out b/contrib/pg_plan_advice/expected/gather.out index 85e13a2dcc5..60f2b32e24f 100644 --- a/contrib/pg_plan_advice/expected/gather.out +++ b/contrib/pg_plan_advice/expected/gather.out @@ -452,14 +452,13 @@ EXPLAIN (COSTS OFF, PLAN_ADVICE) -> Parallel Seq Scan on gt_fact gt_fact_1 Generated Plan Advice: SEQ_SCAN(gt_fact@setop_1 gt_fact@setop_2) - GATHER((unnamed_subquery unnamed_subquery#2)) -(10 rows) +(9 rows) SET LOCAL pg_plan_advice.advice = 'NO_GATHER(gt_fact@setop_1 gt_fact@setop_2)'; EXPLAIN (COSTS OFF, PLAN_ADVICE) SELECT dim_id FROM gt_fact UNION SELECT dim_id FROM gt_fact; - QUERY PLAN ----------------------------------------------------------------------------------- + QUERY PLAN +---------------------------------------------- HashAggregate Group Key: gt_fact.dim_id -> Append @@ -470,7 +469,7 @@ EXPLAIN (COSTS OFF, PLAN_ADVICE) NO_GATHER(gt_fact@setop_2) /* matched */ Generated Plan Advice: SEQ_SCAN(gt_fact@setop_1 gt_fact@setop_2) - NO_GATHER(unnamed_subquery unnamed_subquery#2 gt_fact@setop_1 gt_fact@setop_2) + NO_GATHER(gt_fact@setop_1 gt_fact@setop_2) (11 rows) COMMIT; @@ -479,8 +478,8 @@ COMMIT; BEGIN; EXPLAIN (COSTS OFF, PLAN_ADVICE) SELECT dim_id FROM gt_fact UNION SELECT dim_id FROM gt_fact WHERE false; - QUERY PLAN --------------------------------------------------- + QUERY PLAN +------------------------------------------ HashAggregate Group Key: gt_fact.dim_id -> Gather @@ -489,14 +488,13 @@ EXPLAIN (COSTS OFF, PLAN_ADVICE) Generated Plan Advice: SEQ_SCAN(gt_fact@setop_1) GATHER(gt_fact@setop_1) - NO_GATHER(unnamed_subquery unnamed_subquery#2) -(9 rows) +(8 rows) SET LOCAL pg_plan_advice.advice = 'NO_GATHER(gt_fact@setop_1)'; EXPLAIN (COSTS OFF, PLAN_ADVICE) SELECT dim_id FROM gt_fact UNION SELECT dim_id FROM gt_fact WHERE false; - QUERY PLAN ------------------------------------------------------------------- + QUERY PLAN +-------------------------------------------- HashAggregate Group Key: gt_fact.dim_id -> Seq Scan on gt_fact @@ -504,9 +502,58 @@ EXPLAIN (COSTS OFF, PLAN_ADVICE) NO_GATHER(gt_fact@setop_1) /* matched */ Generated Plan Advice: SEQ_SCAN(gt_fact@setop_1) - NO_GATHER(unnamed_subquery unnamed_subquery#2 gt_fact@setop_1) + NO_GATHER(gt_fact@setop_1) (8 rows) +COMMIT; +-- Test a UNION that has no surviving arms at all. No advice should be +-- generated. +EXPLAIN (COSTS OFF, PLAN_ADVICE) + SELECT dim_id FROM gt_fact WHERE false UNION + SELECT dim_id FROM gt_fact WHERE false; + QUERY PLAN +--------------------------------------------------------------- + Result + Replaces: Aggregate on unnamed_subquery, unnamed_subquery_1 + One-Time Filter: false +(3 rows) + +-- Test interaction of NO_GATHER with INTERSECT. By default, we expect +-- a HashSetOp with a Gather on each side, but NO_GATHER on the setop arms +-- is enough to suppress it. +BEGIN; +EXPLAIN (COSTS OFF, PLAN_ADVICE) + SELECT dim_id FROM gt_fact INTERSECT SELECT dim_id FROM gt_fact; + QUERY PLAN +---------------------------------------------------- + HashSetOp Intersect + -> Gather + Workers Planned: 1 + -> Parallel Seq Scan on gt_fact + -> Gather + Workers Planned: 1 + -> Parallel Seq Scan on gt_fact gt_fact_1 + Generated Plan Advice: + SEQ_SCAN(gt_fact@setop_1 gt_fact@setop_2) + GATHER(gt_fact@setop_1 gt_fact@setop_2) +(10 rows) + +SET LOCAL pg_plan_advice.advice = 'NO_GATHER(gt_fact@setop_1 gt_fact@setop_2)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) + SELECT dim_id FROM gt_fact INTERSECT SELECT dim_id FROM gt_fact; + QUERY PLAN +---------------------------------------------- + HashSetOp Intersect + -> Seq Scan on gt_fact + -> Seq Scan on gt_fact gt_fact_1 + Supplied Plan Advice: + NO_GATHER(gt_fact@setop_1) /* matched */ + NO_GATHER(gt_fact@setop_2) /* matched */ + Generated Plan Advice: + SEQ_SCAN(gt_fact@setop_1 gt_fact@setop_2) + NO_GATHER(gt_fact@setop_1 gt_fact@setop_2) +(9 rows) + COMMIT; -- Test interaction of NO_GATHER with a UNION ALL. This UNION ALL is -- "flattened" by the planner and the subqueries are pulled up, so the relation @@ -564,3 +611,97 @@ EXPLAIN (COSTS OFF, PLAN_ADVICE) (8 rows) COMMIT; +-- Test interaction of NO_GATHER with a UNION ALL that can be flattened, +-- but subqueries that cannot be pulled up. We must use OFFSET '0' here, +-- because writing OFFSET 0 causes the planner to think that the subquery +-- isn't parallel-safe, which is probably something that should be fixed in +-- the core planner. By default, we get a normal Gather over Parallel Append +-- plan, but by varying the advice, we can get a separate Gather over each +-- child, a Parallel Append with a non-partial child, or no parallelism at all. +BEGIN; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT * FROM ( + SELECT * FROM (SELECT dim_id FROM gt_fact OFFSET '0') x + UNION ALL + SELECT * FROM (SELECT dim_id FROM gt_fact OFFSET '0') y +) s; + QUERY PLAN +---------------------------------------------------- + Gather + Workers Planned: 1 + -> Parallel Append + -> Parallel Seq Scan on gt_fact + -> Parallel Seq Scan on gt_fact gt_fact_1 + Generated Plan Advice: + SEQ_SCAN(gt_fact@x gt_fact@y) + GATHER(s) +(8 rows) + +SET LOCAL pg_plan_advice.advice = 'NO_GATHER(s)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT * FROM ( + SELECT * FROM (SELECT dim_id FROM gt_fact OFFSET '0') x + UNION ALL + SELECT * FROM (SELECT dim_id FROM gt_fact OFFSET '0') y +) s; + QUERY PLAN +---------------------------------------------------- + Append + -> Gather + Workers Planned: 1 + -> Parallel Seq Scan on gt_fact + -> Gather + Workers Planned: 1 + -> Parallel Seq Scan on gt_fact gt_fact_1 + Supplied Plan Advice: + NO_GATHER(s) /* matched */ + Generated Plan Advice: + SEQ_SCAN(gt_fact@x gt_fact@y) + GATHER(gt_fact@x gt_fact@y) + NO_GATHER(s x y) +(13 rows) + +SET LOCAL pg_plan_advice.advice = 'NO_GATHER(gt_fact@x)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT * FROM ( + SELECT * FROM (SELECT dim_id FROM gt_fact OFFSET '0') x + UNION ALL + SELECT * FROM (SELECT dim_id FROM gt_fact OFFSET '0') y +) s; + QUERY PLAN +---------------------------------------------------- + Gather + Workers Planned: 1 + -> Parallel Append + -> Seq Scan on gt_fact + -> Parallel Seq Scan on gt_fact gt_fact_1 + Supplied Plan Advice: + NO_GATHER(gt_fact@x) /* matched */ + Generated Plan Advice: + SEQ_SCAN(gt_fact@x gt_fact@y) + GATHER(s) + NO_GATHER(x gt_fact@x) +(11 rows) + +SET LOCAL pg_plan_advice.advice = 'NO_GATHER(s gt_fact@x gt_fact@y)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT * FROM ( + SELECT * FROM (SELECT dim_id FROM gt_fact OFFSET '0') x + UNION ALL + SELECT * FROM (SELECT dim_id FROM gt_fact OFFSET '0') y +) s; + QUERY PLAN +---------------------------------------- + Append + -> Seq Scan on gt_fact + -> Seq Scan on gt_fact gt_fact_1 + Supplied Plan Advice: + NO_GATHER(s) /* matched */ + NO_GATHER(gt_fact@x) /* matched */ + NO_GATHER(gt_fact@y) /* matched */ + Generated Plan Advice: + SEQ_SCAN(gt_fact@x gt_fact@y) + NO_GATHER(s x y gt_fact@x gt_fact@y) +(10 rows) + +COMMIT; diff --git a/contrib/pg_plan_advice/pgpa_join.c b/contrib/pg_plan_advice/pgpa_join.c index e69c25551a0..acb6073ef52 100644 --- a/contrib/pg_plan_advice/pgpa_join.c +++ b/contrib/pg_plan_advice/pgpa_join.c @@ -250,7 +250,7 @@ pgpa_build_unrolled_join(pgpa_plan_walker_context *walker, ujoin->outer.scan = pgpa_build_scan(walker, ujoin->outer.plan, ujoin->outer.elided_node, - join_unroller->outer_beneath_any_gather, + !join_unroller->outer_beneath_any_gather, true); /* @@ -280,7 +280,7 @@ pgpa_build_unrolled_join(pgpa_plan_walker_context *walker, ujoin->inner[i].scan = pgpa_build_scan(walker, ujoin->inner[i].plan, ujoin->inner[i].elided_node, - join_unroller->inner_beneath_any_gather[k], + !join_unroller->inner_beneath_any_gather[k], true); } diff --git a/contrib/pg_plan_advice/pgpa_scan.c b/contrib/pg_plan_advice/pgpa_scan.c index fccc5c7a763..ca41a27fe9d 100644 --- a/contrib/pg_plan_advice/pgpa_scan.c +++ b/contrib/pg_plan_advice/pgpa_scan.c @@ -33,9 +33,8 @@ static RTEKind unique_nonjoin_rtekind(Bitmapset *relids, List *rtable); * If there is at least one ElidedNode for this plan node, pass the uppermost * one as elided_node, else pass NULL. * - * Set the 'beneath_any_gather' node if we are underneath a Gather or - * Gather Merge node (except for a single-copy Gather node, for which - * GATHER or GATHER_MERGE advice should not be emitted). + * Set the 'needs_no_gather' flag if NO_GATHER advice should be generated for + * this scan. * * Set the 'within_join_problem' flag if we're inside of a join problem and * not otherwise. @@ -43,7 +42,7 @@ static RTEKind unique_nonjoin_rtekind(Bitmapset *relids, List *rtable); pgpa_scan * pgpa_build_scan(pgpa_plan_walker_context *walker, Plan *plan, ElidedNode *elided_node, - bool beneath_any_gather, bool within_join_problem) + bool needs_no_gather, bool within_join_problem) { pgpa_scan_strategy strategy = PGPA_SCAN_ORDINARY; Bitmapset *relids = NULL; @@ -224,17 +223,12 @@ pgpa_build_scan(pgpa_plan_walker_context *walker, Plan *plan, return NULL; } - /* - * Add the appropriate set of RTIs to walker->no_gather_scans. - * - * Add nothing if we're beneath a Gather or Gather Merge node, since - * NO_GATHER advice is clearly inappropriate in that situation. - */ - if (!beneath_any_gather) + /* Caller tells us whether NO_GATHER() advice for this scan is needed. */ + if (needs_no_gather) walker->no_gather_scans = bms_add_members(walker->no_gather_scans, relids); - /* Caller tells us whether NO_GATHER() advice for this scan is needed. */ + /* Now create and return the final pgpa_scan object. */ return pgpa_make_scan(walker, plan, strategy, relids); } @@ -260,6 +254,80 @@ pgpa_make_scan(pgpa_plan_walker_context *walker, Plan *plan, return scan; } +/* + * Does this Plan node or associated ElidedNode look like the topmost node + * generated while planning an unflattened set operation? + * + * The return value of this function is a little bit inexact, because for + * example if a set operation produces a HashAggregate on top of an Append, + * this function will only identify the latter. Since the HashAggregate + * node is not RTI-bearing, the difference doesn't currently matter. + * + * Pass the plan node as 'plan', and if we're considering an elided node + * associated with that plan node, also pass it as 'elided_node'. + */ +bool +pgpa_is_setop_parent(Plan *plan, ElidedNode *elided_node, List *rtable) +{ + Bitmapset *relids; + int rti = -1; + + /* + * If we're considering a SetOp or RecursiveUnion, then this is definitely + * a setop parent. If we're considering an Append, a MergeAppend, or an + * upper Result node, this might be a setop parent. If it's anything else, + * this is not a setop parent. + */ + if (elided_node != NULL) + { + if (elided_node->elided_type == T_Append || + elided_node->elided_type == T_MergeAppend) + relids = elided_node->relids; + else + return false; + } + else + { + if (IsA(plan, SetOp) || IsA(plan, RecursiveUnion)) + return true; + else if (IsA(plan, Append) || IsA(plan, MergeAppend) || + (IsA(plan, Result) && + (((Result *) plan)->result_type == RESULT_TYPE_UPPER))) + relids = pgpa_relids(plan); + else + return false; + } + + /* + * As of this writing, we reach here with relids == NULL only for a + * partitionwise aggregate, which is not a setop parent. + */ + if (relids == NULL) + return false; + + /* + * Conceptually, this Append or MergeAppend, or the Result node that + * replaced it, could be a partitionwise scan, a partitionwise join, a + * flattened UNION ALL, or an unflattened set operation. We want to return + * true only in that last case; for our purposes, if a UNION ALL is + * flattened, it no longer counts as a set operation. + * + * An unflattened set operation has no RTI of its own, so a plan node + * implementing one carries the RTIs of its children, which must always be + * subqueries with rte->inh == false. A flattened set operation does have + * an RTI, and that one has rte->inh == true. + */ + while ((rti = bms_next_member(relids, rti)) >= 0) + { + RangeTblEntry *rte = rt_fetch(rti, rtable); + + if (rte->rtekind != RTE_SUBQUERY || rte->inh) + return false; + } + + return true; +} + /* * Determine the unique rtekind of a set of relids. */ diff --git a/contrib/pg_plan_advice/pgpa_scan.h b/contrib/pg_plan_advice/pgpa_scan.h index 391c1a4b596..492111486ca 100644 --- a/contrib/pg_plan_advice/pgpa_scan.h +++ b/contrib/pg_plan_advice/pgpa_scan.h @@ -79,7 +79,9 @@ typedef struct pgpa_scan extern pgpa_scan *pgpa_build_scan(pgpa_plan_walker_context *walker, Plan *plan, ElidedNode *elided_node, - bool beneath_any_gather, + bool needs_no_gather, bool within_join_problem); +extern bool pgpa_is_setop_parent(Plan *plan, ElidedNode *elided_node, + List *rtable); #endif diff --git a/contrib/pg_plan_advice/pgpa_walker.c b/contrib/pg_plan_advice/pgpa_walker.c index e415f880d21..3f4583361df 100644 --- a/contrib/pg_plan_advice/pgpa_walker.c +++ b/contrib/pg_plan_advice/pgpa_walker.c @@ -23,6 +23,7 @@ #include "utils/lsyscache.h" static void pgpa_walk_recursively(pgpa_plan_walker_context *walker, Plan *plan, + bool within_setop, bool within_join_problem, pgpa_join_unroller *join_unroller, List *active_query_features, @@ -90,7 +91,8 @@ pgpa_plan_walker(pgpa_plan_walker_context *walker, PlannedStmt *pstmt, walker->pstmt = pstmt; /* Walk the main plan tree. */ - pgpa_walk_recursively(walker, pstmt->planTree, false, NULL, NIL, false); + pgpa_walk_recursively(walker, pstmt->planTree, false, false, NULL, NIL, + false); /* Main plan tree walk won't reach subplans, so walk those. */ foreach(lc, pstmt->subplans) @@ -98,7 +100,8 @@ pgpa_plan_walker(pgpa_plan_walker_context *walker, PlannedStmt *pstmt, Plan *plan = lfirst(lc); if (plan != NULL) - pgpa_walk_recursively(walker, plan, false, NULL, NIL, false); + pgpa_walk_recursively(walker, plan, false, false, NULL, NIL, + false); } /* Adjust RTIs from sj_unique_rels for the flattened range table. */ @@ -165,6 +168,11 @@ pgpa_plan_walker(pgpa_plan_walker_context *walker, PlannedStmt *pstmt, * that we won't advise either GATHER or GATHER_MERGE or NO_GATHER in such * cases, which might be something we want to improve in the future. * + * The same thing happens when a Gather or Gather Merge node sits atop a + * set operation that was not flattened, because we decline to add RTIs + * from unflattened set operations to query features. In that case, + * there's no planner hook that would allow us to enforce the advice. + * * (Should the Partial Aggregates in such a case be created in an * UPPERREL_GROUP_AGG with a non-empty relid set? Right now that doesn't * happen, but it seems like it would make life easier for us if it did.) @@ -224,6 +232,9 @@ pgpa_plan_walker(pgpa_plan_walker_context *walker, PlannedStmt *pstmt, /* * Main workhorse for the plan tree walk. * + * If within_setop is true, the portion of the plan tree in which this node + * occurs was generated while planning an unflattened set operation. + * * If within_join_problem is true, we encountered a join at some higher level * of the tree walk and haven't yet descended out of the portion of the plan * tree that is part of that same join problem. We're no longer in the same @@ -247,6 +258,7 @@ pgpa_plan_walker(pgpa_plan_walker_context *walker, PlannedStmt *pstmt, */ static void pgpa_walk_recursively(pgpa_plan_walker_context *walker, Plan *plan, + bool within_setop, bool within_join_problem, pgpa_join_unroller *join_unroller, List *active_query_features, @@ -257,7 +269,6 @@ pgpa_walk_recursively(pgpa_plan_walker_context *walker, Plan *plan, bool join_unroller_toplevel = false; ListCell *lc; List *extraplans = NIL; - List *elided_nodes = NIL; int first_partial_plan = 0; Assert(within_join_problem || join_unroller == NULL); @@ -283,61 +294,59 @@ pgpa_walk_recursively(pgpa_plan_walker_context *walker, Plan *plan, } /* - * Find all elided nodes for this Plan node. + * Process elided nodes that pertain to this Plan node. In the pstmt's + * list, the logically uppermost elided node appears last, so we walk the + * list in reverse order. */ - foreach_node(ElidedNode, n, walker->pstmt->elidedNodes) + for (int n = list_length(walker->pstmt->elidedNodes) - 1; n >= 0; --n) { - if (n->plan_node_id == plan->plan_node_id) - elided_nodes = lappend(elided_nodes, n); - } + ElidedNode *elided_node = list_nth(walker->pstmt->elidedNodes, n); - /* If we found any elided_nodes, handle them. */ - if (elided_nodes != NIL) - { - int num_elided_nodes = list_length(elided_nodes); - ElidedNode *last_elided_node; + /* Ignore elided nodes that do not pertain to this plan node. */ + if (elided_node->plan_node_id != plan->plan_node_id) + continue; + + /* Did an unflattened set operation produce this ElidedNode? */ + if (!within_setop) + within_setop = pgpa_is_setop_parent(plan, elided_node, + walker->pstmt->rtable); /* - * RTIs for the final -- and thus logically uppermost -- elided node - * should be collected for query features passed down by the caller. - * However, elided nodes act as barriers to query features, which - * means that (1) the remaining elided nodes, if any, should be - * ignored for purposes of query features and (2) the list of active - * query features should be reset to empty so that we do not add RTIs - * from the plan node that is logically beneath the elided node to the - * query features passed down from the caller. + * Collect RTIs for any currently-active query features, unless we're + * within a set operation. There's no hook to enforce any advice + * pertaining to set operations, so we shouldn't generate any. */ - last_elided_node = list_nth(elided_nodes, num_elided_nodes - 1); - pgpa_qf_add_rtis(active_query_features, - pgpa_filter_out_join_relids(last_elided_node->relids, - walker->pstmt->rtable)); - active_query_features = NIL; + if (!within_setop) + pgpa_qf_add_rtis(active_query_features, + pgpa_filter_out_join_relids(elided_node->relids, + walker->pstmt->rtable)); + + /* Sanity check. */ + if (within_setop && within_join_problem) + elog(ERROR, "set operations cannot be part of a join problem"); /* - * If we're within a join problem, the join_unroller is responsible - * for building the scan for the final elided node, so throw it out. + * If we're within a join problem, the join unroller is responsible + * for building any required scan. If not, we do it here. */ - if (within_join_problem) - elided_nodes = list_truncate(elided_nodes, num_elided_nodes - 1); - - /* Build scans for all (or the remaining) elided nodes. */ - foreach_node(ElidedNode, elided_node, elided_nodes) - { + if (!within_join_problem) (void) pgpa_build_scan(walker, plan, elided_node, - beneath_any_gather, within_join_problem); - } + !beneath_any_gather && !within_setop, + false); + + /* Set operations can't span a subquery boundary. */ + if (elided_node->elided_type == T_SubqueryScan) + within_setop = false; /* - * If there were any elided nodes, then everything beneath those nodes - * is not part of the same join problem. - * - * In more detail, if an Append or MergeAppend was elided, then a - * partitionwise join was chosen and only a single child survived; if - * a SubqueryScan was elided, the subquery was planned without - * flattening it into the parent. + * Everything below an elided node is not part of the same join + * problem. */ within_join_problem = false; join_unroller = NULL; + + /* Elided nodes act as barriers to query features. */ + active_query_features = NIL; } /* @@ -366,12 +375,22 @@ pgpa_walk_recursively(pgpa_plan_walker_context *walker, Plan *plan, beneath_any_gather = true; } + /* Did an unflattened set operation produce this Plan node? */ + if (!within_setop) + within_setop = pgpa_is_setop_parent(plan, NULL, walker->pstmt->rtable); + + /* Sanity check. */ + if (within_setop && within_join_problem) + elog(ERROR, "set operations cannot be part of a join problem"); + /* * If we're within a join problem, the join unroller is responsible for * building any required scan for this node. If not, we do it here. */ if (!within_join_problem) - (void) pgpa_build_scan(walker, plan, NULL, beneath_any_gather, false); + (void) pgpa_build_scan(walker, plan, NULL, + !beneath_any_gather && !within_setop, + false); /* * If this join needs to be unrolled but there's no join unroller already @@ -393,8 +412,14 @@ pgpa_walk_recursively(pgpa_plan_walker_context *walker, Plan *plan, pgpa_unroll_join(walker, plan, beneath_any_gather, join_unroller, &outer_join_unroller, &inner_join_unroller); - /* Add RTIs from the plan node to all active query features. */ - pgpa_qf_add_plan_rtis(active_query_features, plan, walker->pstmt->rtable); + /* + * Collect RTIs for any currently-active query features, unless we're + * within a set operation. There's no hook to enforce any advice + * pertaining to set operations, so we shouldn't generate any. + */ + if (!within_setop) + pgpa_qf_add_plan_rtis(active_query_features, plan, + walker->pstmt->rtable); /* * Recurse into the outer and inner subtrees. @@ -410,13 +435,13 @@ pgpa_walk_recursively(pgpa_plan_walker_context *walker, Plan *plan, if (!IsA(plan, ForeignScan)) { if (plan->lefttree != NULL) - pgpa_walk_recursively(walker, plan->lefttree, within_join_problem, - outer_join_unroller, active_query_features, - beneath_any_gather); + pgpa_walk_recursively(walker, plan->lefttree, within_setop, + within_join_problem, outer_join_unroller, + active_query_features, beneath_any_gather); if (plan->righttree != NULL) - pgpa_walk_recursively(walker, plan->righttree, within_join_problem, - inner_join_unroller, active_query_features, - beneath_any_gather); + pgpa_walk_recursively(walker, plan->righttree, within_setop, + within_join_problem, inner_join_unroller, + active_query_features, beneath_any_gather); } /* @@ -470,7 +495,7 @@ pgpa_walk_recursively(pgpa_plan_walker_context *walker, Plan *plan, * those are specific to a subquery level. */ pgpa_walk_recursively(walker, ((SubqueryScan *) plan)->subplan, - 0, NULL, NIL, beneath_any_gather); + false, false, NULL, NIL, beneath_any_gather); break; case T_CustomScan: extraplans = ((CustomScan *) plan)->custom_plans; @@ -496,8 +521,8 @@ pgpa_walk_recursively(pgpa_plan_walker_context *walker, Plan *plan, if (foreach_current_index(lc) < first_partial_plan) child_beneath_any_gather = false; - pgpa_walk_recursively(walker, subplan, false, NULL, NIL, - child_beneath_any_gather); + pgpa_walk_recursively(walker, subplan, within_setop, false, NULL, + NIL, child_beneath_any_gather); } } diff --git a/contrib/pg_plan_advice/pgpa_walker.h b/contrib/pg_plan_advice/pgpa_walker.h index ebe850622d3..21ebf57c9ac 100644 --- a/contrib/pg_plan_advice/pgpa_walker.h +++ b/contrib/pg_plan_advice/pgpa_walker.h @@ -71,8 +71,8 @@ typedef struct pgpa_query_feature * scans is an array of lists of pgpa_scan objects. The array is indexed by * the scan's pgpa_scan_strategy. * - * no_gather_scans is the set of scan RTIs that do not appear beneath any - * Gather or Gather Merge node. + * no_gather_scans is the set of scan RTIs for which NO_GATHER advice should + * be generated. * * toplevel_unrolled_joins is a list of all pgpa_unrolled_join objects that * are not a child of some other pgpa_unrolled_join. diff --git a/contrib/pg_plan_advice/sql/gather.sql b/contrib/pg_plan_advice/sql/gather.sql index f01850285db..0ec9c790f9e 100644 --- a/contrib/pg_plan_advice/sql/gather.sql +++ b/contrib/pg_plan_advice/sql/gather.sql @@ -132,6 +132,23 @@ EXPLAIN (COSTS OFF, PLAN_ADVICE) SELECT dim_id FROM gt_fact UNION SELECT dim_id FROM gt_fact WHERE false; COMMIT; +-- Test a UNION that has no surviving arms at all. No advice should be +-- generated. +EXPLAIN (COSTS OFF, PLAN_ADVICE) + SELECT dim_id FROM gt_fact WHERE false UNION + SELECT dim_id FROM gt_fact WHERE false; + +-- Test interaction of NO_GATHER with INTERSECT. By default, we expect +-- a HashSetOp with a Gather on each side, but NO_GATHER on the setop arms +-- is enough to suppress it. +BEGIN; +EXPLAIN (COSTS OFF, PLAN_ADVICE) + SELECT dim_id FROM gt_fact INTERSECT SELECT dim_id FROM gt_fact; +SET LOCAL pg_plan_advice.advice = 'NO_GATHER(gt_fact@setop_1 gt_fact@setop_2)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) + SELECT dim_id FROM gt_fact INTERSECT SELECT dim_id FROM gt_fact; +COMMIT; + -- Test interaction of NO_GATHER with a UNION ALL. This UNION ALL is -- "flattened" by the planner and the subqueries are pulled up, so the relation -- identifiers are different from the preceding examples. In this case, @@ -147,3 +164,40 @@ SET LOCAL pg_plan_advice.advice = 'NO_GATHER(unnamed_subquery)'; EXPLAIN (COSTS OFF, PLAN_ADVICE) SELECT dim_id FROM gt_fact UNION ALL SELECT dim_id FROM gt_fact; COMMIT; + +-- Test interaction of NO_GATHER with a UNION ALL that can be flattened, +-- but subqueries that cannot be pulled up. We must use OFFSET '0' here, +-- because writing OFFSET 0 causes the planner to think that the subquery +-- isn't parallel-safe, which is probably something that should be fixed in +-- the core planner. By default, we get a normal Gather over Parallel Append +-- plan, but by varying the advice, we can get a separate Gather over each +-- child, a Parallel Append with a non-partial child, or no parallelism at all. +BEGIN; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT * FROM ( + SELECT * FROM (SELECT dim_id FROM gt_fact OFFSET '0') x + UNION ALL + SELECT * FROM (SELECT dim_id FROM gt_fact OFFSET '0') y +) s; +SET LOCAL pg_plan_advice.advice = 'NO_GATHER(s)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT * FROM ( + SELECT * FROM (SELECT dim_id FROM gt_fact OFFSET '0') x + UNION ALL + SELECT * FROM (SELECT dim_id FROM gt_fact OFFSET '0') y +) s; +SET LOCAL pg_plan_advice.advice = 'NO_GATHER(gt_fact@x)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT * FROM ( + SELECT * FROM (SELECT dim_id FROM gt_fact OFFSET '0') x + UNION ALL + SELECT * FROM (SELECT dim_id FROM gt_fact OFFSET '0') y +) s; +SET LOCAL pg_plan_advice.advice = 'NO_GATHER(s gt_fact@x gt_fact@y)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT * FROM ( + SELECT * FROM (SELECT dim_id FROM gt_fact OFFSET '0') x + UNION ALL + SELECT * FROM (SELECT dim_id FROM gt_fact OFFSET '0') y +) s; +COMMIT; -- 2.53.0