diff --git a/contrib/pg_plan_advice/README b/contrib/pg_plan_advice/README index b0e4fd1d6e1..2ea61e9bc41 100644 --- a/contrib/pg_plan_advice/README +++ b/contrib/pg_plan_advice/README @@ -109,6 +109,13 @@ Bitmap heap scans currently do not allow for an index specification: BITMAP_HEAP_SCAN(foo bar) simply means that each of foo and bar should use some sort of bitmap heap scan. +There is a special DO_NOT_SCAN() advice tag which says that a certain +relation shouldn't be scanned at all. This is used to control which of +two choices is selected when an AlternativeSubPlan is resolved, and +whether or not a MinMaxAggPath is chosen. Control over upper planner +behavior is generally out-of-scope at the moment, but these cases had +to be handled to prevent test_plan_advice failures in the buildfarm. + Join Order Advice ================= diff --git a/contrib/pg_plan_advice/expected/alternatives.out b/contrib/pg_plan_advice/expected/alternatives.out new file mode 100644 index 00000000000..a6fb296d4b4 --- /dev/null +++ b/contrib/pg_plan_advice/expected/alternatives.out @@ -0,0 +1,158 @@ +LOAD 'pg_plan_advice'; +SET max_parallel_workers_per_gather = 0; +CREATE TABLE alt_t1 (a int) WITH (autovacuum_enabled = false); +CREATE TABLE alt_t2 (a int) WITH (autovacuum_enabled = false); +CREATE INDEX ON alt_t2(a); +INSERT INTO alt_t1 SELECT generate_series(1, 1000); +INSERT INTO alt_t2 SELECT generate_series(1, 100000); +VACUUM ANALYZE alt_t1; +VACUUM ANALYZE alt_t2; +-- This query uses an OR to prevent the EXISTS from being converted to a +-- semi-join, forcing the planner through the AlternativeSubPlan path. +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT * FROM alt_t1 +WHERE EXISTS (SELECT 1 FROM alt_t2 WHERE alt_t2.a = alt_t1.a) OR alt_t1.a < 0; + QUERY PLAN +------------------------------------------------------------------- + Seq Scan on alt_t1 + Filter: ((ANY (a = (hashed SubPlan exists_2).col1)) OR (a < 0)) + SubPlan exists_2 + -> Seq Scan on alt_t2 + Generated Plan Advice: + SEQ_SCAN(alt_t1 alt_t2@exists_2) + NO_GATHER(alt_t1 alt_t2@exists_2) + DO_NOT_SCAN(alt_t2@exists_1) +(8 rows) + +-- We should be able to force either AlternativeSubPlan by advising against +-- scanning the other relation. +BEGIN; +SET LOCAL pg_plan_advice.advice = 'DO_NOT_SCAN(alt_t2@exists_1)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT * FROM alt_t1 +WHERE EXISTS (SELECT 1 FROM alt_t2 WHERE alt_t2.a = alt_t1.a) OR alt_t1.a < 0; + QUERY PLAN +------------------------------------------------------------------- + Seq Scan on alt_t1 + Filter: ((ANY (a = (hashed SubPlan exists_2).col1)) OR (a < 0)) + SubPlan exists_2 + -> Seq Scan on alt_t2 + Supplied Plan Advice: + DO_NOT_SCAN(alt_t2@exists_1) /* matched */ + Generated Plan Advice: + SEQ_SCAN(alt_t1 alt_t2@exists_2) + NO_GATHER(alt_t1 alt_t2@exists_2) + DO_NOT_SCAN(alt_t2@exists_1) +(10 rows) + +SET LOCAL pg_plan_advice.advice = 'DO_NOT_SCAN(alt_t2@exists_2)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT * FROM alt_t1 +WHERE EXISTS (SELECT 1 FROM alt_t2 WHERE alt_t2.a = alt_t1.a) OR alt_t1.a < 0; + QUERY PLAN +-------------------------------------------------------- + Seq Scan on alt_t1 + Filter: (EXISTS(SubPlan exists_1) OR (a < 0)) + SubPlan exists_1 + -> Index Only Scan using alt_t2_a_idx on alt_t2 + Index Cond: (a = alt_t1.a) + Supplied Plan Advice: + DO_NOT_SCAN(alt_t2@exists_2) /* matched */ + Generated Plan Advice: + SEQ_SCAN(alt_t1) + INDEX_ONLY_SCAN(alt_t2@exists_1 public.alt_t2_a_idx) + NO_GATHER(alt_t1 alt_t2@exists_1) + DO_NOT_SCAN(alt_t2@exists_2) +(12 rows) + +COMMIT; +-- Now let's test a case involving MinMaxAggPath, which we treat similarly +-- to the AlternativeSubPlan case. +CREATE TABLE alt_minmax (a int) WITH (autovacuum_enabled = false); +CREATE INDEX ON alt_minmax(a); +INSERT INTO alt_minmax SELECT generate_series(1, 10000); +VACUUM ANALYZE alt_minmax; +-- Using an Index Scan inside of an InitPlan should win over a full table +-- scan. +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT min(a), max(a) FROM alt_minmax; + QUERY PLAN +------------------------------------------------------------------------------------------ + Result + Replaces: MinMaxAggregate + InitPlan minmax_1 + -> Limit + -> Index Only Scan using alt_minmax_a_idx on alt_minmax + Index Cond: (a IS NOT NULL) + InitPlan minmax_2 + -> Limit + -> Index Only Scan Backward using alt_minmax_a_idx on alt_minmax alt_minmax_1 + Index Cond: (a IS NOT NULL) + Generated Plan Advice: + INDEX_ONLY_SCAN(alt_minmax@minmax_1 public.alt_minmax_a_idx + alt_minmax@minmax_2 public.alt_minmax_a_idx) + NO_GATHER(alt_minmax@minmax_1 alt_minmax@minmax_2) + DO_NOT_SCAN(alt_minmax) +(15 rows) + +-- Advising against the scan of alt_minmax at the root query level should +-- change nothing, but if we say we don't want either of or both of the +-- minmax-variant scans, the plan should switch to a full table scan. +BEGIN; +SET LOCAL pg_plan_advice.advice = 'DO_NOT_SCAN(alt_minmax)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT min(a), max(a) FROM alt_minmax; + QUERY PLAN +------------------------------------------------------------------------------------------ + Result + Replaces: MinMaxAggregate + InitPlan minmax_1 + -> Limit + -> Index Only Scan using alt_minmax_a_idx on alt_minmax + Index Cond: (a IS NOT NULL) + InitPlan minmax_2 + -> Limit + -> Index Only Scan Backward using alt_minmax_a_idx on alt_minmax alt_minmax_1 + Index Cond: (a IS NOT NULL) + Supplied Plan Advice: + DO_NOT_SCAN(alt_minmax) /* matched */ + Generated Plan Advice: + INDEX_ONLY_SCAN(alt_minmax@minmax_1 public.alt_minmax_a_idx + alt_minmax@minmax_2 public.alt_minmax_a_idx) + NO_GATHER(alt_minmax@minmax_1 alt_minmax@minmax_2) + DO_NOT_SCAN(alt_minmax) +(17 rows) + +SET LOCAL pg_plan_advice.advice = 'DO_NOT_SCAN(alt_minmax@minmax_1)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT min(a), max(a) FROM alt_minmax; + QUERY PLAN +-------------------------------------------------------- + Aggregate + -> Seq Scan on alt_minmax + Supplied Plan Advice: + DO_NOT_SCAN(alt_minmax@minmax_1) /* matched */ + Generated Plan Advice: + SEQ_SCAN(alt_minmax) + NO_GATHER(alt_minmax) + DO_NOT_SCAN(alt_minmax@minmax_1 alt_minmax@minmax_2) +(8 rows) + +SET LOCAL pg_plan_advice.advice = 'DO_NOT_SCAN(alt_minmax@minmax_1) DO_NOT_SCAN(alt_minmax@minmax_2)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT min(a), max(a) FROM alt_minmax; + QUERY PLAN +-------------------------------------------------------- + Aggregate + -> Seq Scan on alt_minmax + Supplied Plan Advice: + DO_NOT_SCAN(alt_minmax@minmax_1) /* matched */ + DO_NOT_SCAN(alt_minmax@minmax_2) /* matched */ + Generated Plan Advice: + SEQ_SCAN(alt_minmax) + NO_GATHER(alt_minmax) + DO_NOT_SCAN(alt_minmax@minmax_1 alt_minmax@minmax_2) +(9 rows) + +COMMIT; +DROP TABLE alt_t1, alt_t2, alt_minmax; diff --git a/contrib/pg_plan_advice/expected/scan.out b/contrib/pg_plan_advice/expected/scan.out index 3f9e13b6d41..44ce40f33a6 100644 --- a/contrib/pg_plan_advice/expected/scan.out +++ b/contrib/pg_plan_advice/expected/scan.out @@ -270,7 +270,8 @@ EXPLAIN (COSTS OFF, PLAN_ADVICE) SELECT * FROM scan_table WHERE a > 0; COMMIT; -- We can force a primary key lookup to use a sequential scan, but we -- can't force it to use an index-only scan (due to the column list) --- or a TID scan (due to the absence of a TID qual). +-- or a TID scan (due to the absence of a TID qual). If we apply DO_NOT_SCAN +-- here, we should get a valid plan anyway, but with the scan disabled. BEGIN; SET LOCAL pg_plan_advice.advice = 'SEQ_SCAN(scan_table)'; EXPLAIN (COSTS OFF, PLAN_ADVICE) SELECT * FROM scan_table WHERE a = 1; @@ -313,6 +314,20 @@ EXPLAIN (COSTS OFF, PLAN_ADVICE) SELECT * FROM scan_table WHERE a = 1; NO_GATHER(scan_table) (8 rows) +SET LOCAL pg_plan_advice.advice = 'DO_NOT_SCAN(scan_table)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) SELECT * FROM scan_table WHERE a = 1; + QUERY PLAN +------------------------------------------------- + Index Scan using scan_table_pkey on scan_table + Disabled: true + Index Cond: (a = 1) + Supplied Plan Advice: + DO_NOT_SCAN(scan_table) /* matched, failed */ + Generated Plan Advice: + INDEX_SCAN(scan_table public.scan_table_pkey) + NO_GATHER(scan_table) +(8 rows) + COMMIT; -- We can forcibly downgrade an index-only scan to an index scan, but we can't -- force the use of an index that the planner thinks is inapplicable. diff --git a/contrib/pg_plan_advice/meson.build b/contrib/pg_plan_advice/meson.build index 36bbc4e9826..f2098947b64 100644 --- a/contrib/pg_plan_advice/meson.build +++ b/contrib/pg_plan_advice/meson.build @@ -53,6 +53,7 @@ tests += { 'bd': meson.current_build_dir(), 'regress': { 'sql': [ + 'alternatives', 'gather', 'join_order', 'join_strategy', diff --git a/contrib/pg_plan_advice/pgpa_ast.c b/contrib/pg_plan_advice/pgpa_ast.c index f4fa6a626d4..3c340c6ae7a 100644 --- a/contrib/pg_plan_advice/pgpa_ast.c +++ b/contrib/pg_plan_advice/pgpa_ast.c @@ -32,6 +32,8 @@ pgpa_cstring_advice_tag(pgpa_advice_tag_type advice_tag) { case PGPA_TAG_BITMAP_HEAP_SCAN: return "BITMAP_HEAP_SCAN"; + case PGPA_TAG_DO_NOT_SCAN: + return "DO_NOT_SCAN"; case PGPA_TAG_FOREIGN_JOIN: return "FOREIGN_JOIN"; case PGPA_TAG_GATHER: @@ -92,6 +94,10 @@ pgpa_parse_advice_tag(const char *tag, bool *fail) if (strcmp(tag, "bitmap_heap_scan") == 0) return PGPA_TAG_BITMAP_HEAP_SCAN; break; + case 'd': + if (strcmp(tag, "do_not_scan") == 0) + return PGPA_TAG_DO_NOT_SCAN; + break; case 'f': if (strcmp(tag, "foreign_join") == 0) return PGPA_TAG_FOREIGN_JOIN; diff --git a/contrib/pg_plan_advice/pgpa_ast.h b/contrib/pg_plan_advice/pgpa_ast.h index 3c3db801926..a89f1251929 100644 --- a/contrib/pg_plan_advice/pgpa_ast.h +++ b/contrib/pg_plan_advice/pgpa_ast.h @@ -80,6 +80,7 @@ typedef struct pgpa_advice_target typedef enum pgpa_advice_tag_type { PGPA_TAG_BITMAP_HEAP_SCAN, + PGPA_TAG_DO_NOT_SCAN, PGPA_TAG_FOREIGN_JOIN, PGPA_TAG_GATHER, PGPA_TAG_GATHER_MERGE, diff --git a/contrib/pg_plan_advice/pgpa_output.c b/contrib/pg_plan_advice/pgpa_output.c index 28d2839ce1a..cd4411f350c 100644 --- a/contrib/pg_plan_advice/pgpa_output.c +++ b/contrib/pg_plan_advice/pgpa_output.c @@ -54,6 +54,8 @@ static void pgpa_output_simple_strategy(pgpa_output_context *context, List *relid_sets); static void pgpa_output_no_gather(pgpa_output_context *context, Bitmapset *relids); +static void pgpa_output_do_not_scan(pgpa_output_context *context, + List *identifiers); static void pgpa_output_relations(pgpa_output_context *context, StringInfo buf, Bitmapset *relids); @@ -156,6 +158,9 @@ pgpa_output_advice(StringInfo buf, pgpa_plan_walker_context *walker, /* Emit NO_GATHER advice. */ pgpa_output_no_gather(&context, walker->no_gather_scans); + + /* Emit DO_NOT_SCAN advice. */ + pgpa_output_do_not_scan(&context, walker->do_not_scan_identifiers); } /* @@ -395,6 +400,36 @@ pgpa_output_no_gather(pgpa_output_context *context, Bitmapset *relids) appendStringInfoChar(context->buf, ')'); } +/* + * Output DO_NOT_SCAN advice for all relations in the provided list of + * identifiers. + */ +static void +pgpa_output_do_not_scan(pgpa_output_context *context, List *identifiers) +{ + bool first = true; + + if (identifiers == NIL) + return; + if (context->buf->len > 0) + appendStringInfoChar(context->buf, '\n'); + appendStringInfoString(context->buf, "DO_NOT_SCAN("); + + foreach_ptr(pgpa_identifier, rid, identifiers) + { + if (first) + first = false; + else + { + pgpa_maybe_linebreak(context->buf, context->wrap_column); + appendStringInfoChar(context->buf, ' '); + } + appendStringInfoString(context->buf, pgpa_identifier_string(rid)); + } + + appendStringInfoChar(context->buf, ')'); +} + /* * Output the identifiers for each RTI in the provided set. * diff --git a/contrib/pg_plan_advice/pgpa_planner.c b/contrib/pg_plan_advice/pgpa_planner.c index 6b574da655f..afa9587a725 100644 --- a/contrib/pg_plan_advice/pgpa_planner.c +++ b/contrib/pg_plan_advice/pgpa_planner.c @@ -164,11 +164,13 @@ static void pgpa_planner_feedback_warning(List *feedback); static pgpa_planner_info *pgpa_planner_get_proot(pgpa_planner_state *pps, PlannerInfo *root); -static inline void pgpa_ri_checker_save(pgpa_planner_state *pps, - PlannerInfo *root, - RelOptInfo *rel); -static void pgpa_ri_checker_validate(pgpa_planner_state *pps, - PlannedStmt *pstmt); +static inline void pgpa_compute_rt_identifier(pgpa_planner_info *proot, + PlannerInfo *root, + RelOptInfo *rel); +static void pgpa_compute_rt_offsets(pgpa_planner_state *pps, + PlannedStmt *pstmt); +static void pgpa_validate_rt_identifiers(pgpa_planner_state *pps, + PlannedStmt *pstmt); static char *pgpa_bms_to_cstring(Bitmapset *bms); static const char *pgpa_jointype_to_cstring(JoinType jointype); @@ -264,20 +266,10 @@ pgpa_planner_setup(PlannerGlobal *glob, Query *parse, const char *query_string, } } -#ifdef USE_ASSERT_CHECKING - - /* - * If asserts are enabled, always build a private state object for - * cross-checks. - */ - needs_pps = true; -#endif - /* * We only create and initialize a private state object if it's needed for * some purpose. That could be (1) recording that we will need to generate - * an advice string, (2) storing a trove of supplied advice, or (3) - * facilitating debugging cross-checks when asserts are enabled. + * an advice string or (2) storing a trove of supplied advice. * * Currently, the active memory context should be one that will last for * the entire duration of query planning, but if GEQO is in use, it's @@ -321,9 +313,16 @@ pgpa_planner_shutdown(PlannerGlobal *glob, Query *parse, pps = GetPlannerGlobalExtensionState(glob, planner_extension_id); if (pps != NULL) { + /* Set up some local variables. */ trove = pps->trove; generate_advice_feedback = pps->generate_advice_feedback; generate_advice_string = pps->generate_advice_string; + + /* Compute range table offsets. */ + pgpa_compute_rt_offsets(pps, pstmt); + + /* Cross-check range table identifiers. */ + pgpa_validate_rt_identifiers(pps, pstmt); } /* @@ -394,13 +393,6 @@ pgpa_planner_shutdown(PlannerGlobal *glob, Query *parse, lappend(pstmt->extension_state, makeDefElem("pg_plan_advice", (Node *) pgpa_items, -1)); - /* - * If assertions are enabled, cross-check the generated range table - * identifiers. - */ - if (pps != NULL) - pgpa_ri_checker_validate(pps, pstmt); - /* Pass call to previous hook. */ if (prev_planner_shutdown) (*prev_planner_shutdown) (glob, parse, query_string, pstmt); @@ -408,35 +400,38 @@ pgpa_planner_shutdown(PlannerGlobal *glob, Query *parse, /* * Hook function for build_simple_rel(). - * - * We can apply scan advice at this point, and we also use this as an - * opportunity to do range-table identifier cross-checking in assert-enabled - * builds. */ static void pgpa_build_simple_rel(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte) { pgpa_planner_state *pps; + pgpa_planner_info *proot = NULL; /* Fetch our private state, set up by pgpa_planner_setup(). */ pps = GetPlannerGlobalExtensionState(root->glob, planner_extension_id); - /* Save details needed for range table identifier cross-checking. */ + /* + * Look up the pgpa_planner_info for this subquery, and make sure we've + * saved a range table identifier. + */ if (pps != NULL) - pgpa_ri_checker_save(pps, root, rel); + { + proot = pgpa_planner_get_proot(pps, root); + pgpa_compute_rt_identifier(proot, root, rel); + } /* If query advice was provided, search for relevant entries. */ if (pps != NULL && pps->trove != NULL) { - pgpa_identifier rid; + pgpa_identifier *rid; pgpa_trove_result tresult_scan; pgpa_trove_result tresult_rel; /* Search for scan advice and general rel advice. */ - pgpa_compute_identifier_by_rti(root, rel->relid, &rid); - pgpa_trove_lookup(pps->trove, PGPA_TROVE_LOOKUP_SCAN, 1, &rid, + rid = &proot->rid_array[rel->relid - 1]; + pgpa_trove_lookup(pps->trove, PGPA_TROVE_LOOKUP_SCAN, 1, rid, &tresult_scan); - pgpa_trove_lookup(pps->trove, PGPA_TROVE_LOOKUP_REL, 1, &rid, + pgpa_trove_lookup(pps->trove, PGPA_TROVE_LOOKUP_REL, 1, rid, &tresult_rel); /* If relevant entries were found, apply them. */ @@ -1626,6 +1621,8 @@ pgpa_planner_apply_scan_advice(RelOptInfo *rel, pgpa_trove_entry *rel_entries, Bitmapset *rel_indexes) { + const uint64 all_scan_mask = PGS_SCAN_ANY | PGS_APPEND | + PGS_MERGE_APPEND | PGS_CONSIDER_INDEXONLY; bool gather_conflict = false; Bitmapset *gather_partial_match = NULL; Bitmapset *gather_full_match = NULL; @@ -1636,16 +1633,18 @@ pgpa_planner_apply_scan_advice(RelOptInfo *rel, Bitmapset *scan_type_indexes = NULL; Bitmapset *scan_type_rel_indexes = NULL; uint64 gather_mask = 0; - uint64 scan_type = 0; + uint64 scan_type = all_scan_mask; /* sentinel: no advice yet */ /* Scrutinize available scan advice. */ while ((i = bms_next_member(scan_indexes, i)) >= 0) { pgpa_trove_entry *my_entry = &scan_entries[i]; - uint64 my_scan_type = 0; + uint64 my_scan_type = all_scan_mask; /* Translate our advice tags to a scan strategy advice value. */ - if (my_entry->tag == PGPA_TAG_BITMAP_HEAP_SCAN) + if (my_entry->tag == PGPA_TAG_DO_NOT_SCAN) + my_scan_type = 0; + else if (my_entry->tag == PGPA_TAG_BITMAP_HEAP_SCAN) { /* * Currently, PGS_CONSIDER_INDEXONLY can suppress Bitmap Heap @@ -1679,9 +1678,9 @@ pgpa_planner_apply_scan_advice(RelOptInfo *rel, * INDEX_SCAN(a b.c) as non-conflicting if it happens that the only * index named c is in schema b, but it doesn't seem worth the code. */ - if (my_scan_type != 0) + if (my_scan_type != all_scan_mask) { - if (scan_type != 0 && scan_type != my_scan_type) + if (scan_type != all_scan_mask && scan_type != my_scan_type) scan_type_conflict = true; if (!scan_type_conflict && scan_entry != NULL && my_entry->target->itarget != NULL && @@ -1716,7 +1715,7 @@ pgpa_planner_apply_scan_advice(RelOptInfo *rel, { const uint64 my_scan_type = PGS_APPEND | PGS_MERGE_APPEND; - if (scan_type != 0 && scan_type != my_scan_type) + if (scan_type != all_scan_mask && scan_type != my_scan_type) scan_type_conflict = true; scan_entry = my_entry; scan_type = my_scan_type; @@ -1795,7 +1794,7 @@ pgpa_planner_apply_scan_advice(RelOptInfo *rel, if (matched_index == NULL) { /* Don't force the scan type if the index doesn't exist. */ - scan_type = 0; + scan_type = all_scan_mask; /* Mark advice as inapplicable. */ pgpa_trove_set_flags(scan_entries, scan_type_indexes, @@ -1839,14 +1838,8 @@ pgpa_planner_apply_scan_advice(RelOptInfo *rel, * Only clear bits here, so that we still respect the enable_* GUCs. Do * nothing in cases where the advice on a single topic conflicts. */ - if (scan_type != 0 && !scan_type_conflict) - { - uint64 all_scan_mask; - - all_scan_mask = PGS_SCAN_ANY | PGS_APPEND | PGS_MERGE_APPEND | - PGS_CONSIDER_INDEXONLY; + if (scan_type != all_scan_mask && !scan_type_conflict) rel->pgs_mask &= ~(all_scan_mask & ~scan_type); - } if (gather_mask != 0 && !gather_conflict) { uint64 all_gather_mask; @@ -2001,9 +1994,38 @@ pgpa_planner_get_proot(pgpa_planner_state *pps, PlannerInfo *root) } } - /* Create new object, add to list, and make it most recently used. */ + /* Create new object. */ new_proot = palloc0_object(pgpa_planner_info); + + /* Set plan name and alternative plan name. */ new_proot->plan_name = root->plan_name; + new_proot->alternative_plan_name = root->alternative_plan_name; + + /* + * If the newly-created proot shares an alternative_plan_name with one or + * more others, all should have the is_alternative_plan flag set. + */ + foreach_ptr(pgpa_planner_info, other_proot, pps->proots) + { + if (strings_equal_or_both_null(new_proot->alternative_plan_name, + other_proot->alternative_plan_name)) + { + new_proot->is_alternative_plan = true; + other_proot->is_alternative_plan = true; + } + } + + /* + * Outermost query level always has rtoffset 0; other rtoffset values are + * computed later. + */ + if (root->plan_name == NULL) + { + new_proot->has_rtoffset = true; + new_proot->rtoffset = 0; + } + + /* Add to list and make it most recently used. */ pps->proots = lappend(pps->proots, new_proot); pps->last_proot = new_proot; @@ -2011,19 +2033,15 @@ pgpa_planner_get_proot(pgpa_planner_state *pps, PlannerInfo *root) } /* - * Save the range table identifier for one relation for future cross-checking. + * Compute the range table identifier for one relation and save it for future + * use. */ static void -pgpa_ri_checker_save(pgpa_planner_state *pps, PlannerInfo *root, - RelOptInfo *rel) +pgpa_compute_rt_identifier(pgpa_planner_info *proot, PlannerInfo *root, + RelOptInfo *rel) { -#ifdef USE_ASSERT_CHECKING - pgpa_planner_info *proot; pgpa_identifier *rid; - /* Get the pgpa_planner_info for this PlannerInfo. */ - proot = pgpa_planner_get_proot(pps, root); - /* Allocate or extend the proot's rid_array as necessary. */ if (proot->rid_array_size < rel->relid) { @@ -2043,7 +2061,60 @@ pgpa_ri_checker_save(pgpa_planner_state *pps, PlannerInfo *root, rid = &proot->rid_array[rel->relid - 1]; if (rid->alias_name == NULL) pgpa_compute_identifier_by_rti(root, rel->relid, rid); -#endif +} + +/* + * Compute the range table offset for each pgpa_planner_info for which it + * is possible to meaningfully do so. + */ +static void +pgpa_compute_rt_offsets(pgpa_planner_state *pps, PlannedStmt *pstmt) +{ + foreach_ptr(pgpa_planner_info, proot, pps->proots) + { + /* For the top query level, we've previously set rtoffset 0. */ + if (proot->plan_name == NULL) + { + Assert(proot->has_rtoffset); + continue; + } + + /* + * It's not guaranteed that every plan name we saw during planning has + * a SubPlanInfo, but any that do not certainly don't appear in the + * final range table. + */ + foreach_node(SubPlanRTInfo, rtinfo, pstmt->subrtinfos) + { + if (strcmp(proot->plan_name, rtinfo->plan_name) == 0) + { + /* + * If rtinfo->dummy is set, then the subquery's range table + * will only have been partially copied to the final range + * table. Specifically, only RTE_RELATION entries and + * RTE_SUBQUERY entries that were once RTE_RELATION entries + * will be copied, as per add_rtes_to_flat_rtable. Therefore, + * there's no fixed rtoffset that we can apply to the RTIs + * used during planning to locate the corresponding relations. + */ + if (rtinfo->dummy) + { + /* + * It will not be possible to make any effective use of the + * sj_unique_rels list in this case, and it also won't be + * important to do so. So just throw the list away to avoid + * confusing pgpa_plan_walker. + */ + proot->sj_unique_rels = NIL; + break; + } + Assert(!proot->has_rtoffset); + proot->has_rtoffset = true; + proot->rtoffset = rtinfo->rtoffset; + break; + } + } + } } /* @@ -2051,7 +2122,7 @@ pgpa_ri_checker_save(pgpa_planner_state *pps, PlannerInfo *root, * planning match the ones we generated from the final plan. */ static void -pgpa_ri_checker_validate(pgpa_planner_state *pps, PlannedStmt *pstmt) +pgpa_validate_rt_identifiers(pgpa_planner_state *pps, PlannedStmt *pstmt) { #ifdef USE_ASSERT_CHECKING pgpa_identifier *rt_identifiers; @@ -2063,48 +2134,12 @@ pgpa_ri_checker_validate(pgpa_planner_state *pps, PlannedStmt *pstmt) /* Iterate over identifiers created during planning, so we can compare. */ foreach_ptr(pgpa_planner_info, proot, pps->proots) { - int rtoffset = 0; - - /* - * If there's no plan name associated with this entry, then the - * rtoffset is 0. Otherwise, we can search the SubPlanRTInfo list to - * find the rtoffset. - */ - if (proot->plan_name != NULL) - { - foreach_node(SubPlanRTInfo, rtinfo, pstmt->subrtinfos) - { - /* - * If rtinfo->dummy is set, then the subquery's range table - * will only have been partially copied to the final range - * table. Specifically, only RTE_RELATION entries and - * RTE_SUBQUERY entries that were once RTE_RELATION entries - * will be copied, as per add_rtes_to_flat_rtable. Therefore, - * there's no fixed rtoffset that we can apply to the RTIs - * used during planning to locate the corresponding relations - */ - if (strcmp(proot->plan_name, rtinfo->plan_name) == 0 - && !rtinfo->dummy) - { - rtoffset = rtinfo->rtoffset; - Assert(rtoffset > 0); - break; - } - } - - /* - * It's not an error if we don't find the plan name: that just - * means that we planned a subplan by this name but it ended up - * being a dummy subplan and so wasn't included in the final plan - * tree. - */ - if (rtoffset == 0) - continue; - } + if (!proot->has_rtoffset) + continue; for (int rti = 1; rti <= proot->rid_array_size; ++rti) { - Index flat_rti = rtoffset + rti; + Index flat_rti = proot->rtoffset + rti; pgpa_identifier *rid1 = &proot->rid_array[rti - 1]; pgpa_identifier *rid2; diff --git a/contrib/pg_plan_advice/pgpa_planner.h b/contrib/pg_plan_advice/pgpa_planner.h index e9045f69bca..93fda2055b2 100644 --- a/contrib/pg_plan_advice/pgpa_planner.h +++ b/contrib/pg_plan_advice/pgpa_planner.h @@ -24,11 +24,33 @@ typedef struct pgpa_planner_info /* Plan name taken from the corresponding PlannerInfo; NULL at top level. */ char *plan_name; -#ifdef USE_ASSERT_CHECKING + /* + * If the corresponding PlannerInfo has an alternative_root, then this is + * the plan name from that PlannerInfo; otherwise, it is the same as + * plan_name. + * + * is_alternative_plan is set to true for every pgpa_planner_info that + * shares an alternative_plan_name with at least one other, and to false + * otherwise. + */ + char *alternative_plan_name; + bool is_alternative_plan; + /* Relation identifiers computed for baserels at this query level. */ pgpa_identifier *rid_array; int rid_array_size; -#endif + + /* + * If has_rtoffset is true, then rtoffset is the offset required to align + * RTIs for this query level with RTIs from the final, flattened rangetable. + * If has_rtoffset is false, then this subquery's range table wasn't copied, + * or was only partially copied, into the final range table. (Note that + * we can't determine the rtoffset values until the final range table + * actually exists; before that time, has_rtoffset will be false everywhere + * except at the top level.) + */ + bool has_rtoffset; + Index rtoffset; /* * List of Bitmapset objects. Each represents the relid set of a relation diff --git a/contrib/pg_plan_advice/pgpa_trove.c b/contrib/pg_plan_advice/pgpa_trove.c index 634ec5c4c6e..7ade0b5ca9c 100644 --- a/contrib/pg_plan_advice/pgpa_trove.c +++ b/contrib/pg_plan_advice/pgpa_trove.c @@ -162,6 +162,7 @@ pgpa_build_trove(List *advice_items) break; case PGPA_TAG_BITMAP_HEAP_SCAN: + case PGPA_TAG_DO_NOT_SCAN: case PGPA_TAG_INDEX_ONLY_SCAN: case PGPA_TAG_INDEX_SCAN: case PGPA_TAG_SEQ_SCAN: diff --git a/contrib/pg_plan_advice/pgpa_walker.c b/contrib/pg_plan_advice/pgpa_walker.c index 6fbc784bf54..0a4512d4921 100644 --- a/contrib/pg_plan_advice/pgpa_walker.c +++ b/contrib/pg_plan_advice/pgpa_walker.c @@ -59,6 +59,10 @@ static bool pgpa_walker_contains_join(pgpa_plan_walker_context *walker, Bitmapset *relids); static bool pgpa_walker_contains_no_gather(pgpa_plan_walker_context *walker, Bitmapset *relids); +static void pgpa_classify_alternative_subplans(pgpa_plan_walker_context *walker, + List *proots, + List **chosen_proots, + List **discarded_proots); /* * Top-level entrypoint for the plan tree walk. @@ -75,6 +79,8 @@ pgpa_plan_walker(pgpa_plan_walker_context *walker, PlannedStmt *pstmt, ListCell *lc; List *sj_unique_rtis = NULL; List *sj_nonunique_qfs = NULL; + List *chosen_proots; + List *discarded_proots; /* Initialization. */ memset(walker, 0, sizeof(pgpa_plan_walker_context)); @@ -95,42 +101,23 @@ pgpa_plan_walker(pgpa_plan_walker_context *walker, PlannedStmt *pstmt, /* Adjust RTIs from sj_unique_rels for the flattened range table. */ foreach_ptr(pgpa_planner_info, proot, proots) { - int rtoffset = 0; - bool dummy = false; - /* If there are no sj_unique_rels for this proot, we can skip it. */ if (proot->sj_unique_rels == NIL) continue; /* If this is a subplan, find the range table offset. */ - if (proot->plan_name != NULL) - { - foreach_node(SubPlanRTInfo, rtinfo, pstmt->subrtinfos) - { - if (strcmp(proot->plan_name, rtinfo->plan_name) == 0) - { - rtoffset = rtinfo->rtoffset; - dummy = rtinfo->dummy; - break; - } - } + if (!proot->has_rtoffset) + elog(ERROR, "no rtoffset for plan %s", proot->plan_name); - if (rtoffset == 0) - elog(ERROR, "no rtoffset for plan %s", proot->plan_name); - } - - /* If this entry pertains to a dummy subquery, ignore it. */ - if (dummy) - continue; - - /* Offset each relid set by the rtoffset we just computed. */ + /* Offset each relid set by the proot's rtoffset. */ foreach_node(Bitmapset, relids, proot->sj_unique_rels) { int rtindex = -1; Bitmapset *flat_relids = NULL; while ((rtindex = bms_next_member(relids, rtindex)) >= 0) - flat_relids = bms_add_member(flat_relids, rtindex + rtoffset); + flat_relids = bms_add_member(flat_relids, + rtindex + proot->rtoffset); sj_unique_rtis = lappend(sj_unique_rtis, flat_relids); } @@ -193,6 +180,42 @@ pgpa_plan_walker(pgpa_plan_walker_context *walker, PlannedStmt *pstmt, walker->query_features[t] = query_features; } + + /* Classify alternative subplans. */ + pgpa_classify_alternative_subplans(walker, proots, + &chosen_proots, &discarded_proots); + + /* + * Figure out which of the discarded alternatives have a non-discarded + * alternative. Those are the ones for which we want to emit DO_NOT_SCAN + * advice. (If every alternative was discarded, then there's no point.) + */ + foreach_ptr(pgpa_planner_info, discarded_proot, discarded_proots) + { + bool some_alternative_chosen = false; + + foreach_ptr(pgpa_planner_info, chosen_proot, chosen_proots) + { + if (strings_equal_or_both_null(discarded_proot->alternative_plan_name, + chosen_proot->alternative_plan_name)) + { + some_alternative_chosen = true; + break; + } + } + + if (some_alternative_chosen) + { + for (int rti = 1; rti <= discarded_proot->rid_array_size; rti++) + { + pgpa_identifier *rid = &discarded_proot->rid_array[rti - 1]; + + if (rid->alias_name != NULL) + walker->do_not_scan_identifiers = + lappend(walker->do_not_scan_identifiers, rid); + } + } + } } /* @@ -697,6 +720,30 @@ pgpa_walker_would_advise(pgpa_plan_walker_context *walker, return false; } + /* + * DO_NOT_SCAN advice targets rels that may not be in the flat range table + * (e.g. MinMaxAgg losers), so we can't use pgpa_compute_rti_from_identifier. + * Instead, check directly against the do_not_scan_identifiers list. + */ + if (tag == PGPA_TAG_DO_NOT_SCAN) + { + if (target->ttype != PGPA_TARGET_IDENTIFIER) + return false; + foreach_ptr(pgpa_identifier, rid, walker->do_not_scan_identifiers) + { + if (strcmp(rid->alias_name, target->rid.alias_name) == 0 && + rid->occurrence == target->rid.occurrence && + strings_equal_or_both_null(rid->partnsp, + target->rid.partnsp) && + strings_equal_or_both_null(rid->partrel, + target->rid.partrel) && + strings_equal_or_both_null(rid->plan_name, + target->rid.plan_name)) + return true; + } + return false; + } + if (target->ttype == PGPA_TARGET_IDENTIFIER) { Index rti; @@ -730,6 +777,10 @@ pgpa_walker_would_advise(pgpa_plan_walker_context *walker, /* should have been handled above */ pg_unreachable(); break; + case PGPA_TAG_DO_NOT_SCAN: + /* should have been handled above */ + pg_unreachable(); + break; case PGPA_TAG_BITMAP_HEAP_SCAN: return pgpa_walker_find_scan(walker, PGPA_SCAN_BITMAP_HEAP, @@ -1035,3 +1086,60 @@ pgpa_walker_contains_no_gather(pgpa_plan_walker_context *walker, { return bms_is_subset(relids, walker->no_gather_scans); } + +/* + * Classify alternative subplans as chosen or discarded. + */ +static void +pgpa_classify_alternative_subplans(pgpa_plan_walker_context *walker, + List *proots, + List **chosen_proots, + List **discarded_proots) +{ + Bitmapset *all_scan_rtis = NULL; + + /* Initialize both output lists to empty. */ + *chosen_proots = NIL; + *discarded_proots = NIL; + + /* Collect all scan RTIs. */ + for (int s = 0; s < NUM_PGPA_SCAN_STRATEGY; s++) + foreach_ptr(pgpa_scan, scan, walker->scans[s]) + all_scan_rtis = bms_add_members(all_scan_rtis, scan->relids); + + /* Now classify each subplan. */ + foreach_ptr(pgpa_planner_info, proot, proots) + { + bool chosen = false; + + /* + * We're only interested in classifying subplans for which there are + * alternatives. + */ + if (!proot->is_alternative_plan) + continue; + + /* + * A subplan has been chosen if any of its scan RTIs appear in the + * final plan. This cannot be the case if it has no RT offset. + */ + if (proot->has_rtoffset) + { + for (int rti = 1; rti <= proot->rid_array_size; rti++) + { + if (proot->rid_array[rti - 1].alias_name != NULL && + bms_is_member(proot->rtoffset + rti, all_scan_rtis)) + { + chosen = true; + break; + } + } + } + + /* Add it to the correct list. */ + if (chosen) + *chosen_proots = lappend(*chosen_proots, proot); + else + *discarded_proots = lappend(*discarded_proots, proot); + } +} diff --git a/contrib/pg_plan_advice/pgpa_walker.h b/contrib/pg_plan_advice/pgpa_walker.h index 9b74cd3ba55..47667c03374 100644 --- a/contrib/pg_plan_advice/pgpa_walker.h +++ b/contrib/pg_plan_advice/pgpa_walker.h @@ -100,6 +100,7 @@ typedef struct pgpa_plan_walker_context List *join_strategies[NUM_PGPA_JOIN_STRATEGY]; List *query_features[NUM_PGPA_QF_TYPES]; List *future_query_features; + List *do_not_scan_identifiers; } pgpa_plan_walker_context; extern void pgpa_plan_walker(pgpa_plan_walker_context *walker, diff --git a/contrib/pg_plan_advice/sql/alternatives.sql b/contrib/pg_plan_advice/sql/alternatives.sql new file mode 100644 index 00000000000..16299edd196 --- /dev/null +++ b/contrib/pg_plan_advice/sql/alternatives.sql @@ -0,0 +1,58 @@ +LOAD 'pg_plan_advice'; +SET max_parallel_workers_per_gather = 0; + +CREATE TABLE alt_t1 (a int) WITH (autovacuum_enabled = false); +CREATE TABLE alt_t2 (a int) WITH (autovacuum_enabled = false); +CREATE INDEX ON alt_t2(a); +INSERT INTO alt_t1 SELECT generate_series(1, 1000); +INSERT INTO alt_t2 SELECT generate_series(1, 100000); +VACUUM ANALYZE alt_t1; +VACUUM ANALYZE alt_t2; + +-- This query uses an OR to prevent the EXISTS from being converted to a +-- semi-join, forcing the planner through the AlternativeSubPlan path. +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT * FROM alt_t1 +WHERE EXISTS (SELECT 1 FROM alt_t2 WHERE alt_t2.a = alt_t1.a) OR alt_t1.a < 0; + +-- We should be able to force either AlternativeSubPlan by advising against +-- scanning the other relation. +BEGIN; +SET LOCAL pg_plan_advice.advice = 'DO_NOT_SCAN(alt_t2@exists_1)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT * FROM alt_t1 +WHERE EXISTS (SELECT 1 FROM alt_t2 WHERE alt_t2.a = alt_t1.a) OR alt_t1.a < 0; +SET LOCAL pg_plan_advice.advice = 'DO_NOT_SCAN(alt_t2@exists_2)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT * FROM alt_t1 +WHERE EXISTS (SELECT 1 FROM alt_t2 WHERE alt_t2.a = alt_t1.a) OR alt_t1.a < 0; +COMMIT; + +-- Now let's test a case involving MinMaxAggPath, which we treat similarly +-- to the AlternativeSubPlan case. +CREATE TABLE alt_minmax (a int) WITH (autovacuum_enabled = false); +CREATE INDEX ON alt_minmax(a); +INSERT INTO alt_minmax SELECT generate_series(1, 10000); +VACUUM ANALYZE alt_minmax; + +-- Using an Index Scan inside of an InitPlan should win over a full table +-- scan. +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT min(a), max(a) FROM alt_minmax; + +-- Advising against the scan of alt_minmax at the root query level should +-- change nothing, but if we say we don't want either of or both of the +-- minmax-variant scans, the plan should switch to a full table scan. +BEGIN; +SET LOCAL pg_plan_advice.advice = 'DO_NOT_SCAN(alt_minmax)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT min(a), max(a) FROM alt_minmax; +SET LOCAL pg_plan_advice.advice = 'DO_NOT_SCAN(alt_minmax@minmax_1)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT min(a), max(a) FROM alt_minmax; +SET LOCAL pg_plan_advice.advice = 'DO_NOT_SCAN(alt_minmax@minmax_1) DO_NOT_SCAN(alt_minmax@minmax_2)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT min(a), max(a) FROM alt_minmax; +COMMIT; + +DROP TABLE alt_t1, alt_t2, alt_minmax; diff --git a/contrib/pg_plan_advice/sql/scan.sql b/contrib/pg_plan_advice/sql/scan.sql index 4fc494c7d8e..800ff7a4622 100644 --- a/contrib/pg_plan_advice/sql/scan.sql +++ b/contrib/pg_plan_advice/sql/scan.sql @@ -79,7 +79,8 @@ COMMIT; -- We can force a primary key lookup to use a sequential scan, but we -- can't force it to use an index-only scan (due to the column list) --- or a TID scan (due to the absence of a TID qual). +-- or a TID scan (due to the absence of a TID qual). If we apply DO_NOT_SCAN +-- here, we should get a valid plan anyway, but with the scan disabled. BEGIN; SET LOCAL pg_plan_advice.advice = 'SEQ_SCAN(scan_table)'; EXPLAIN (COSTS OFF, PLAN_ADVICE) SELECT * FROM scan_table WHERE a = 1; @@ -87,6 +88,8 @@ SET LOCAL pg_plan_advice.advice = 'INDEX_ONLY_SCAN(scan_table scan_table_pkey)'; EXPLAIN (COSTS OFF, PLAN_ADVICE) SELECT * FROM scan_table WHERE a = 1; SET LOCAL pg_plan_advice.advice = 'TID_SCAN(scan_table)'; EXPLAIN (COSTS OFF, PLAN_ADVICE) SELECT * FROM scan_table WHERE a = 1; +SET LOCAL pg_plan_advice.advice = 'DO_NOT_SCAN(scan_table)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) SELECT * FROM scan_table WHERE a = 1; COMMIT; -- We can forcibly downgrade an index-only scan to an index scan, but we can't diff --git a/doc/src/sgml/pgplanadvice.sgml b/doc/src/sgml/pgplanadvice.sgml index 8df8a978ecf..c3e1ccb60a2 100644 --- a/doc/src/sgml/pgplanadvice.sgml +++ b/doc/src/sgml/pgplanadvice.sgml @@ -267,7 +267,8 @@ TID_SCAN(target [ ... ]) INDEX_SCAN(target index_name [ ... ]) INDEX_ONLY_SCAN(target index_name [ ... ]) FOREIGN_SCAN((target [ ... ]) [ ... ]) -BITMAP_HEAP_SCAN(target [ ... ]) +BITMAP_HEAP_SCAN(target [ ... ]) +DO_NOT_SCAN(target [ ... ]) SEQ_SCAN specifies that each target should be @@ -297,6 +298,17 @@ BITMAP_HEAP_SCAN(target [ ... ]) that purpose. + + DO_NOT_SCAN specifies that a particular target + should not appear in the final plan at all. In most cases, this is + impossible, and will simply cause the scan of the target relation to + be marked disabled. However, in certain cases, the planner considers + optimizations where a portion of the plan tree is copied and mutated, + and then considered as an alternative to the original. In those cases, + DO_NOT_SCAN can be used to exclude the non-preferred + alternative. + + The planner supports many types of scans other than those listed here; however, in most of those cases, there is no meaningful decision to be