From 38be2b496cb4f61817fccf85640935fda59e98df Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Wed, 9 Sep 2026 14:54:59 -0400 Subject: [PATCH v4 3/3] pg_plan_advice: Fix defects in JOIN_ORDER advice feedback. This commit fixes several related problems. All of these problems escaped testing for the same reason: they do not occur when only generated advice is supplied, and therefore test_plan_advice was incapable of detecting these shortcomings. First, prior to this commit, the advice feedback code never regarded an unordered sublist of a JOIN_ORDER specification as matching an unrolled join. As a result, when JOIN_ORDER() advice contained unordered sublists, the advice feedback tended to be "matched, failed" even when the advice worked exactly as intended. Only the case where an unordered sublist was implemented by a plan shape not subject to unrolling, such as a partitionwise join, worked properly. Second, prior to this commit, a join order sublist in the initial position wasn't properly handled. Note that this overlaps with the problem described in the previous paragraph; JOIN_ORDER({a b} c) was broken both because of the unordered sublist and because of the sublist being in the initial position. However, there's more to this case: JOIN_ORDER((a b) c) means the same as JOIN_ORDER(a b c), but the advice feedback code didn't know that, and would generate "matched, failed" for the former case even when everything was working. Finally, prior to this commit, advice feedback didn't correctly handle single-element sublists. Those are pretty nonsensical, since by definition a join involves at least 2 tables, so we could just ban that case. Instead, at least for now, I've chosen to make advice feedback handle such cases in the same way that advice enforcement already does: the extra grouping levels are simply disregarded, so that JOIN_ORDER({a} ((b))) is enforced in the same way as, and also gets the same advice feedback as, JOIN_ORDER(a b). Reported-by: Noah Misch --- .../pg_plan_advice/expected/join_order.out | 205 +++++++++++- .../pg_plan_advice/expected/partitionwise.out | 58 ++++ contrib/pg_plan_advice/pgpa_walker.c | 296 ++++++++++++++---- contrib/pg_plan_advice/sql/join_order.sql | 51 +++ contrib/pg_plan_advice/sql/partitionwise.sql | 8 + 5 files changed, 554 insertions(+), 64 deletions(-) diff --git a/contrib/pg_plan_advice/expected/join_order.out b/contrib/pg_plan_advice/expected/join_order.out index a5a9728e3fd..850fa6bef16 100644 --- a/contrib/pg_plan_advice/expected/join_order.out +++ b/contrib/pg_plan_advice/expected/join_order.out @@ -181,7 +181,210 @@ SELECT * FROM jo_fact f -> Seq Scan on jo_dim2 d2 Filter: (val2 = 1) Supplied Plan Advice: - JOIN_ORDER(f {d1 d2}) /* matched, failed */ + JOIN_ORDER(f {d1 d2}) /* matched */ + Generated Plan Advice: + JOIN_ORDER(f (d1 d2)) + NESTED_LOOP_MATERIALIZE(d2) + HASH_JOIN((d1 d2)) + SEQ_SCAN(f d1 d2) + NO_GATHER(f d1 d2) +(18 rows) + +COMMIT; +-- Test cases for initial sublists, which require special handling in the code. +BEGIN; +SET LOCAL pg_plan_advice.advice = 'join_order((f d1) d2)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT * FROM jo_fact f + LEFT JOIN jo_dim1 d1 ON f.dim1_id = d1.id + LEFT JOIN jo_dim2 d2 ON f.dim2_id = d2.id + WHERE val1 = 1 AND val2 = 1; + QUERY PLAN +------------------------------------------ + Hash Join + Hash Cond: (f.dim2_id = d2.id) + -> Hash Join + Hash Cond: (f.dim1_id = d1.id) + -> Seq Scan on jo_fact f + -> Hash + -> Seq Scan on jo_dim1 d1 + Filter: (val1 = 1) + -> Hash + -> Seq Scan on jo_dim2 d2 + Filter: (val2 = 1) + Supplied Plan Advice: + JOIN_ORDER((f d1) d2) /* matched */ + Generated Plan Advice: + JOIN_ORDER(f d1 d2) + HASH_JOIN(d1 d2) + SEQ_SCAN(f d1 d2) + NO_GATHER(f d1 d2) +(18 rows) + +SET LOCAL pg_plan_advice.advice = 'join_order({f d1} d2)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT * FROM jo_fact f + LEFT JOIN jo_dim1 d1 ON f.dim1_id = d1.id + LEFT JOIN jo_dim2 d2 ON f.dim2_id = d2.id + WHERE val1 = 1 AND val2 = 1; + QUERY PLAN +------------------------------------------ + Hash Join + Hash Cond: (f.dim2_id = d2.id) + -> Hash Join + Hash Cond: (f.dim1_id = d1.id) + -> Seq Scan on jo_fact f + -> Hash + -> Seq Scan on jo_dim1 d1 + Filter: (val1 = 1) + -> Hash + -> Seq Scan on jo_dim2 d2 + Filter: (val2 = 1) + Supplied Plan Advice: + JOIN_ORDER({f d1} d2) /* matched */ + Generated Plan Advice: + JOIN_ORDER(f d1 d2) + HASH_JOIN(d1 d2) + SEQ_SCAN(f d1 d2) + NO_GATHER(f d1 d2) +(18 rows) + +COMMIT; +-- Test cases for single-element groupings. The extra grouping levels should +-- be ignored. +BEGIN; +SET LOCAL pg_plan_advice.advice = 'join_order((f) d1 d2)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT * FROM jo_fact f + LEFT JOIN jo_dim1 d1 ON f.dim1_id = d1.id + LEFT JOIN jo_dim2 d2 ON f.dim2_id = d2.id + WHERE val1 = 1 AND val2 = 1; + QUERY PLAN +------------------------------------------ + Hash Join + Hash Cond: (f.dim2_id = d2.id) + -> Hash Join + Hash Cond: (f.dim1_id = d1.id) + -> Seq Scan on jo_fact f + -> Hash + -> Seq Scan on jo_dim1 d1 + Filter: (val1 = 1) + -> Hash + -> Seq Scan on jo_dim2 d2 + Filter: (val2 = 1) + Supplied Plan Advice: + JOIN_ORDER((f) d1 d2) /* matched */ + Generated Plan Advice: + JOIN_ORDER(f d1 d2) + HASH_JOIN(d1 d2) + SEQ_SCAN(f d1 d2) + NO_GATHER(f d1 d2) +(18 rows) + +SET LOCAL pg_plan_advice.advice = 'join_order({f} d1 d2)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT * FROM jo_fact f + LEFT JOIN jo_dim1 d1 ON f.dim1_id = d1.id + LEFT JOIN jo_dim2 d2 ON f.dim2_id = d2.id + WHERE val1 = 1 AND val2 = 1; + QUERY PLAN +------------------------------------------ + Hash Join + Hash Cond: (f.dim2_id = d2.id) + -> Hash Join + Hash Cond: (f.dim1_id = d1.id) + -> Seq Scan on jo_fact f + -> Hash + -> Seq Scan on jo_dim1 d1 + Filter: (val1 = 1) + -> Hash + -> Seq Scan on jo_dim2 d2 + Filter: (val2 = 1) + Supplied Plan Advice: + JOIN_ORDER({f} d1 d2) /* matched */ + Generated Plan Advice: + JOIN_ORDER(f d1 d2) + HASH_JOIN(d1 d2) + SEQ_SCAN(f d1 d2) + NO_GATHER(f d1 d2) +(18 rows) + +SET LOCAL pg_plan_advice.advice = 'join_order(f (d1) d2)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT * FROM jo_fact f + LEFT JOIN jo_dim1 d1 ON f.dim1_id = d1.id + LEFT JOIN jo_dim2 d2 ON f.dim2_id = d2.id + WHERE val1 = 1 AND val2 = 1; + QUERY PLAN +------------------------------------------ + Hash Join + Hash Cond: (f.dim2_id = d2.id) + -> Hash Join + Hash Cond: (f.dim1_id = d1.id) + -> Seq Scan on jo_fact f + -> Hash + -> Seq Scan on jo_dim1 d1 + Filter: (val1 = 1) + -> Hash + -> Seq Scan on jo_dim2 d2 + Filter: (val2 = 1) + Supplied Plan Advice: + JOIN_ORDER(f (d1) d2) /* matched */ + Generated Plan Advice: + JOIN_ORDER(f d1 d2) + HASH_JOIN(d1 d2) + SEQ_SCAN(f d1 d2) + NO_GATHER(f d1 d2) +(18 rows) + +SET LOCAL pg_plan_advice.advice = 'join_order(f {d1} d2)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT * FROM jo_fact f + LEFT JOIN jo_dim1 d1 ON f.dim1_id = d1.id + LEFT JOIN jo_dim2 d2 ON f.dim2_id = d2.id + WHERE val1 = 1 AND val2 = 1; + QUERY PLAN +------------------------------------------ + Hash Join + Hash Cond: (f.dim2_id = d2.id) + -> Hash Join + Hash Cond: (f.dim1_id = d1.id) + -> Seq Scan on jo_fact f + -> Hash + -> Seq Scan on jo_dim1 d1 + Filter: (val1 = 1) + -> Hash + -> Seq Scan on jo_dim2 d2 + Filter: (val2 = 1) + Supplied Plan Advice: + JOIN_ORDER(f {d1} d2) /* matched */ + Generated Plan Advice: + JOIN_ORDER(f d1 d2) + HASH_JOIN(d1 d2) + SEQ_SCAN(f d1 d2) + NO_GATHER(f d1 d2) +(18 rows) + +SET LOCAL pg_plan_advice.advice = 'join_order(f ({d1 d2}))'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT * FROM jo_fact f + LEFT JOIN jo_dim1 d1 ON f.dim1_id = d1.id + LEFT JOIN jo_dim2 d2 ON f.dim2_id = d2.id + WHERE val1 = 1 AND val2 = 1; + QUERY PLAN +------------------------------------------------------------ + Hash Join + Hash Cond: ((f.dim1_id = d1.id) AND (f.dim2_id = d2.id)) + -> Seq Scan on jo_fact f + -> Hash + -> Nested Loop + -> Seq Scan on jo_dim1 d1 + Filter: (val1 = 1) + -> Materialize + -> Seq Scan on jo_dim2 d2 + Filter: (val2 = 1) + Supplied Plan Advice: + JOIN_ORDER(f ({d1 d2})) /* matched */ Generated Plan Advice: JOIN_ORDER(f (d1 d2)) NESTED_LOOP_MATERIALIZE(d2) diff --git a/contrib/pg_plan_advice/expected/partitionwise.out b/contrib/pg_plan_advice/expected/partitionwise.out index 2b3d0a82443..b055fcc7baa 100644 --- a/contrib/pg_plan_advice/expected/partitionwise.out +++ b/contrib/pg_plan_advice/expected/partitionwise.out @@ -273,6 +273,64 @@ SELECT * FROM pt1, pt2, pt3 WHERE pt1.id = pt2.id AND pt2.id = pt3.id pt2/public.pt2b pt2/public.pt2c pt3/public.pt3a pt3/public.pt3b pt3/public.pt3c) (51 rows) +COMMIT; +-- Test use of join order for the partitionwise join case. +BEGIN; +SET LOCAL pg_plan_advice.advice = 'PARTITIONWISE((pt1 pt2)) JOIN_ORDER({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 +------------------------------------------------------------------------------------- + Hash Join + Hash Cond: (pt1.id = pt3.id) + -> Append + -> Hash Join + Hash Cond: (pt1_1.id = pt2_1.id) + -> Seq Scan on pt1a pt1_1 + Filter: (val1 = 1) + -> Hash + -> Seq Scan on pt2a pt2_1 + Filter: (val2 = 1) + -> Hash Join + Hash Cond: (pt1_2.id = pt2_2.id) + -> Seq Scan on pt1b pt1_2 + Filter: (val1 = 1) + -> Hash + -> Seq Scan on pt2b pt2_2 + Filter: (val2 = 1) + -> Hash Join + Hash Cond: (pt1_3.id = pt2_3.id) + -> Seq Scan on pt1c pt1_3 + Filter: (val1 = 1) + -> Hash + -> Seq Scan on pt2c pt2_3 + Filter: (val2 = 1) + -> Hash + -> Append + -> Seq Scan on pt3a pt3_1 + Filter: (val3 = 1) + -> Seq Scan on pt3b pt3_2 + Filter: (val3 = 1) + -> Seq Scan on pt3c pt3_3 + Filter: (val3 = 1) + Supplied Plan Advice: + JOIN_ORDER({pt1 pt2} pt3) /* matched */ + PARTITIONWISE((pt1 pt2)) /* matched */ + Generated Plan Advice: + JOIN_ORDER(pt1/public.pt1a pt2/public.pt2a) + JOIN_ORDER(pt1/public.pt1b pt2/public.pt2b) + JOIN_ORDER(pt1/public.pt1c pt2/public.pt2c) + JOIN_ORDER({pt1 pt2} pt3) + HASH_JOIN(pt2/public.pt2a pt2/public.pt2b pt2/public.pt2c pt3) + SEQ_SCAN(pt1/public.pt1a pt2/public.pt2a pt1/public.pt1b pt2/public.pt2b + 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) + COMMIT; -- Can't force a partitionwise join with a mismatched table. BEGIN; diff --git a/contrib/pg_plan_advice/pgpa_walker.c b/contrib/pg_plan_advice/pgpa_walker.c index 7b299440d4d..bfd1a8e3be1 100644 --- a/contrib/pg_plan_advice/pgpa_walker.c +++ b/contrib/pg_plan_advice/pgpa_walker.c @@ -17,6 +17,7 @@ #include "pgpa_walker.h" #include "access/tsmapi.h" +#include "miscadmin.h" #include "nodes/plannodes.h" #include "parser/parsetree.h" #include "utils/lsyscache.h" @@ -38,15 +39,16 @@ static void pgpa_qf_add_rtis(List *active_query_features, Bitmapset *relids); static void pgpa_qf_add_plan_rtis(List *active_query_features, Plan *plan, List *rtable); -static bool pgpa_walker_join_order_matches(pgpa_unrolled_join *ujoin, - Index rtable_length, - pgpa_identifier *rt_identifiers, - pgpa_advice_target *target, - bool toplevel); -static bool pgpa_walker_join_order_matches_member(pgpa_join_member *member, - Index rtable_length, - pgpa_identifier *rt_identifiers, - pgpa_advice_target *target); +static unsigned pgpa_walker_join_order_matches(pgpa_unrolled_join *ujoin, + Index rtable_length, + pgpa_identifier *rt_identifiers, + pgpa_advice_target *target); +static bool pgpa_walker_join_order_matches_members(pgpa_unrolled_join *ujoin, + unsigned *match_position, + Index rtable_length, + pgpa_identifier *rt_identifiers, + pgpa_advice_target *target); +static Bitmapset *pgpa_walker_join_member_relids(pgpa_join_member *member); static pgpa_scan *pgpa_walker_find_scan(pgpa_plan_walker_context *walker, pgpa_scan_strategy strategy, Bitmapset *relids); @@ -741,8 +743,12 @@ pgpa_walker_would_advise(pgpa_plan_walker_context *walker, { foreach_ptr(pgpa_unrolled_join, ujoin, walker->toplevel_unrolled_joins) { + /* + * The advice need not account for every member of the join, so + * any non-zero result is good enough. + */ if (pgpa_walker_join_order_matches(ujoin, rtable_length, - rt_identifiers, target, true)) + rt_identifiers, target) != 0) return true; } @@ -936,83 +942,149 @@ pgpa_walker_index_target_matches_plan(pgpa_index_target *itarget, Plan *plan) /* * Does an unrolled join match the join order specified by an advice target? + * + * The return value is the number of join members matched, or 0 if they do not + * match. This allows the caller to distinguish between a complete match + * (where the return value will be ujoin->ninner + 1) and a partial match + * (where the return value will be some smaller positive integer), if desired. */ -static bool +static unsigned pgpa_walker_join_order_matches(pgpa_unrolled_join *ujoin, Index rtable_length, pgpa_identifier *rt_identifiers, - pgpa_advice_target *target, - bool toplevel) + pgpa_advice_target *target) { - int nchildren = list_length(target->children); + unsigned match_position = 0; Assert(target->ttype == PGPA_TARGET_ORDERED_LIST); - /* At toplevel, we allow a prefix match. */ - if (toplevel) + foreach_ptr(pgpa_advice_target, child_target, target->children) { - if (nchildren > ujoin->ninner + 1) - return false; - } - else - { - if (nchildren != ujoin->ninner + 1) - return false; - } - - /* Outermost rel must match. */ - if (!pgpa_walker_join_order_matches_member(&ujoin->outer, - rtable_length, - rt_identifiers, - linitial(target->children))) - return false; - - /* Each inner rel must match. */ - for (int n = 0; n < nchildren - 1; ++n) - { - pgpa_advice_target *child_target = list_nth(target->children, n + 1); - - if (!pgpa_walker_join_order_matches_member(&ujoin->inner[n], - rtable_length, - rt_identifiers, - child_target)) - return false; + if (match_position > ujoin->ninner) + return 0; + if (!pgpa_walker_join_order_matches_members(ujoin, &match_position, + rtable_length, + rt_identifiers, + child_target)) + return 0; } - return true; + return match_position; } /* - * Does one member of an unrolled join match an advice target? + * Does the specified portion of an unrolled join match an advice target? + * + * We'll look for a match within ujoin beginning at *match_position, where 0 + * means a match starting with the outer member, and a positive value of N + * means a match starting with the inner member at index N - 1. If a match is + * found, returns true and *match_position is incremented by the number of + * pgpa_join_member objects consumed; if not, returns false and the value + * of *match_position is undefined. */ static bool -pgpa_walker_join_order_matches_member(pgpa_join_member *member, - Index rtable_length, - pgpa_identifier *rt_identifiers, - pgpa_advice_target *target) +pgpa_walker_join_order_matches_members(pgpa_unrolled_join *ujoin, + unsigned *match_position, + Index rtable_length, + pgpa_identifier *rt_identifiers, + pgpa_advice_target *target) { - Bitmapset *relids = NULL; + pgpa_join_member *member; - if (member->unrolled_join != NULL) + check_stack_depth(); + + /* + * Find the pgpa_join_member to which *match_position refers. + */ + if (*match_position == 0) + member = &ujoin->outer; + else { - if (target->ttype != PGPA_TARGET_ORDERED_LIST) - return false; - return pgpa_walker_join_order_matches(member->unrolled_join, - rtable_length, - rt_identifiers, - target, - false); + Assert(*match_position <= ujoin->ninner); + member = &ujoin->inner[*match_position - 1]; } - Assert(member->scan != NULL); + /* + * Single-element lists within a join order specification have no clear + * meaning, since a join intrinsically involves at least two tables, but + * enforcement treats them as if the extra list levels were not present. + * That is, JOIN_ORDER((({a})) b) is elsewhere treated as synonymous with + * JOIN_ORDER(a b), so we do that here as well. + */ + while (target->ttype != PGPA_TARGET_IDENTIFIER && + list_length(target->children) == 1) + target = linitial(target->children); + + /* Now do the real work. */ switch (target->ttype) { case PGPA_TARGET_ORDERED_LIST: - /* Could only match an unrolled join */ + + /* + * Since outer-deep joins are flattened, a sublist that begins at + * the outer member describes the start of this unrolled join. For + * instance, JOIN_ORDER((a b) c d) is a less-convenient but still + * acceptable way of writing JOIN_ORDER(a b c d). + */ + if (*match_position == 0) + { + unsigned nmatched; + + nmatched = pgpa_walker_join_order_matches(ujoin, + rtable_length, + rt_identifiers, + target); + if (nmatched == 0) + return false; + *match_position += nmatched; + return true; + } + + /* + * In contrast, a sublist being matched to an inner member can + * only ever match that one member, which must therefore be an + * unrolled join. + */ + if (member->unrolled_join != NULL) + { + pgpa_unrolled_join *nested = member->unrolled_join; + unsigned nmatched; + + nmatched = pgpa_walker_join_order_matches(nested, + rtable_length, + rt_identifiers, + target); + + /* + * Only a complete match suffices. Something like JOIN_ORDER(a + * (b c d) e) still matches if, after those five tables are + * joined as shown, there are additional joins to other + * tables. But table a must be joined first to a three-way + * join between exactly b, c, and d: no additional tables are + * allowed beyond those named in the sublist. + */ + if (nmatched == nested->ninner + 1) + { + *match_position += 1; + return true; + } + } + return false; case PGPA_TARGET_UNORDERED_LIST: { + Bitmapset *relids = NULL; + Bitmapset *member_relids; + Bitmapset *accumulated_relids; + BMS_Comparison comparison; + unsigned ninner = 0; + + /* + * Convert this unordered sublist to a set of RTIs; but, if + * any relation identifier can't be mapped to an RTI, then + * there is no match. + */ foreach_ptr(pgpa_advice_target, child_target, target->children) { Index rti; @@ -1024,24 +1096,122 @@ pgpa_walker_join_order_matches_member(pgpa_join_member *member, return false; relids = bms_add_member(relids, rti); } - break; + + /* See whether it matches the set of RTIs for this member. */ + member_relids = pgpa_walker_join_member_relids(member); + comparison = bms_subset_compare(member_relids, relids); + if (comparison == BMS_EQUAL) + { + /* Exact match: we're done! */ + *match_position += 1; + return true; + } + + /* + * If we're matching this target against an inner member, the + * target must match exactly one member, or else it's not a + * match at all. + */ + if (*match_position != 0) + return false; + + /* + * Since outer-deep joins are flattened, a sublist that begins + * at the outer member describes the start of this unrolled + * join. + * + * For instance, JOIN_ORDER({a b} c d) allows for an unrolled + * join with either a or b as the outer rel and the other as + * the first inner rel. + * + * This means we need to iterate to figure out how many inner + * members this advice target matches (or to discover that + * there is no match). + */ + accumulated_relids = bms_copy(member_relids); + while (comparison == BMS_SUBSET1 && ninner < ujoin->ninner) + { + member = &ujoin->inner[ninner++]; + member_relids = pgpa_walker_join_member_relids(member); + accumulated_relids = bms_add_members(accumulated_relids, + member_relids); + comparison = bms_subset_compare(accumulated_relids, + relids); + } + + /* + * If we found a number of inner members such that the union + * of all their RTIs exactly matches the set that the advice + * target must cover, then consume them all and return true. + * If not, it's not a match, so return false. + */ + if (comparison == BMS_EQUAL) + { + *match_position += 1 + ninner; + return true; + } + return false; } case PGPA_TARGET_IDENTIFIER: { Index rti; + int scan_rti; + + /* Could only match a scan */ + if (member->unrolled_join != NULL) + return false; rti = pgpa_compute_rti_from_identifier(rtable_length, rt_identifiers, &target->rid); if (rti == 0) return false; - relids = bms_make_singleton(rti); - break; + + if (!bms_get_singleton_member(member->scan->relids, &scan_rti)) + return false; + if (rti != (Index) scan_rti) + return false; + + *match_position += 1; + return true; } } - return bms_equal(member->scan->relids, relids); + pg_unreachable(); + return false; +} + +/* + * Compute the set of relations covered by one member of an unrolled join. + */ +static Bitmapset * +pgpa_walker_join_member_relids(pgpa_join_member *member) +{ + pgpa_unrolled_join *ujoin; + Bitmapset *all_relids; + + check_stack_depth(); + + /* If it's a scan, this is easy. */ + if (member->scan != NULL) + return member->scan->relids; + + /* Otherwise, it's an unrolled join. */ + ujoin = member->unrolled_join; + Assert(ujoin != NULL); + + /* Collect outer relids (which must be from a scan). */ + Assert(ujoin->outer.unrolled_join == NULL); + all_relids = bms_copy(ujoin->outer.scan->relids); + + /* Collect each set of inner relids. */ + for (unsigned k = 0; k < ujoin->ninner; ++k) + all_relids = + bms_add_members(all_relids, + pgpa_walker_join_member_relids(&ujoin->inner[k])); + + return all_relids; } /* diff --git a/contrib/pg_plan_advice/sql/join_order.sql b/contrib/pg_plan_advice/sql/join_order.sql index 88d90de9cc6..8602def4559 100644 --- a/contrib/pg_plan_advice/sql/join_order.sql +++ b/contrib/pg_plan_advice/sql/join_order.sql @@ -66,6 +66,57 @@ SELECT * FROM jo_fact f WHERE val1 = 1 AND val2 = 1; COMMIT; +-- Test cases for initial sublists, which require special handling in the code. +BEGIN; +SET LOCAL pg_plan_advice.advice = 'join_order((f d1) d2)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT * FROM jo_fact f + LEFT JOIN jo_dim1 d1 ON f.dim1_id = d1.id + LEFT JOIN jo_dim2 d2 ON f.dim2_id = d2.id + WHERE val1 = 1 AND val2 = 1; +SET LOCAL pg_plan_advice.advice = 'join_order({f d1} d2)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT * FROM jo_fact f + LEFT JOIN jo_dim1 d1 ON f.dim1_id = d1.id + LEFT JOIN jo_dim2 d2 ON f.dim2_id = d2.id + WHERE val1 = 1 AND val2 = 1; +COMMIT; + +-- Test cases for single-element groupings. The extra grouping levels should +-- be ignored. +BEGIN; +SET LOCAL pg_plan_advice.advice = 'join_order((f) d1 d2)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT * FROM jo_fact f + LEFT JOIN jo_dim1 d1 ON f.dim1_id = d1.id + LEFT JOIN jo_dim2 d2 ON f.dim2_id = d2.id + WHERE val1 = 1 AND val2 = 1; +SET LOCAL pg_plan_advice.advice = 'join_order({f} d1 d2)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT * FROM jo_fact f + LEFT JOIN jo_dim1 d1 ON f.dim1_id = d1.id + LEFT JOIN jo_dim2 d2 ON f.dim2_id = d2.id + WHERE val1 = 1 AND val2 = 1; +SET LOCAL pg_plan_advice.advice = 'join_order(f (d1) d2)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT * FROM jo_fact f + LEFT JOIN jo_dim1 d1 ON f.dim1_id = d1.id + LEFT JOIN jo_dim2 d2 ON f.dim2_id = d2.id + WHERE val1 = 1 AND val2 = 1; +SET LOCAL pg_plan_advice.advice = 'join_order(f {d1} d2)'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT * FROM jo_fact f + LEFT JOIN jo_dim1 d1 ON f.dim1_id = d1.id + LEFT JOIN jo_dim2 d2 ON f.dim2_id = d2.id + WHERE val1 = 1 AND val2 = 1; +SET LOCAL pg_plan_advice.advice = 'join_order(f ({d1 d2}))'; +EXPLAIN (COSTS OFF, PLAN_ADVICE) +SELECT * FROM jo_fact f + LEFT JOIN jo_dim1 d1 ON f.dim1_id = d1.id + LEFT JOIN jo_dim2 d2 ON f.dim2_id = d2.id + WHERE val1 = 1 AND val2 = 1; +COMMIT; + -- Force a join order by mentioning just a prefix of the join list. BEGIN; SET LOCAL pg_plan_advice.advice = 'join_order(d2)'; diff --git a/contrib/pg_plan_advice/sql/partitionwise.sql b/contrib/pg_plan_advice/sql/partitionwise.sql index c51456dbbb5..20575e4edc3 100644 --- a/contrib/pg_plan_advice/sql/partitionwise.sql +++ b/contrib/pg_plan_advice/sql/partitionwise.sql @@ -78,6 +78,14 @@ SELECT * FROM pt1, pt2, pt3 WHERE pt1.id = pt2.id AND pt2.id = pt3.id AND val1 = 1 AND val2 = 1 AND val3 = 1; COMMIT; +-- Test use of join order for the partitionwise join case. +BEGIN; +SET LOCAL pg_plan_advice.advice = 'PARTITIONWISE((pt1 pt2)) JOIN_ORDER({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; +COMMIT; + -- Can't force a partitionwise join with a mismatched table. BEGIN; SET LOCAL pg_plan_advice.advice = 'PARTITIONWISE((pt1 ptmismatch))'; -- 2.51.0