diff --git a/contrib/pg_plan_advice/README b/contrib/pg_plan_advice/README index 2be40f0eba6..532d646a145 100644 --- a/contrib/pg_plan_advice/README +++ b/contrib/pg_plan_advice/README @@ -71,12 +71,10 @@ alias_name#occurrence_number/partition_schema.partition_name@plan_name All components except for the alias_name are optional and included only when required. When a component is omitted, the associated punctuation must also be omitted. Occurrence numbers are counted ignoring children of -partitioned tables. When the generated occurrence number is 1, we omit -the occurrence number. The partition schema and partition name are included -only for children of partitioned tables. In generated advice, the -partition_schema is always included whenever there is a partition_name, -but user-written advice may mention the name and omit the schema. The -plan_name is omitted for the top-level PlannerInfo. +partitioned tables. When the generated occurrence number is 1, we omit the +occurrence number. For children of partitioned tables, both the partition +schema and partition name must be included; in other cases, both are +omitted. The plan_name is omitted for the top-level PlannerInfo. Scan Advice =========== diff --git a/contrib/pg_plan_advice/expected/join_order.out b/contrib/pg_plan_advice/expected/join_order.out index a5a9728e3fd..d75c341e243 100644 --- a/contrib/pg_plan_advice/expected/join_order.out +++ b/contrib/pg_plan_advice/expected/join_order.out @@ -253,7 +253,7 @@ COMMIT; -- jo_fact is not partitioned, but let's try pretending that it is and -- verifying that the advice does not apply. BEGIN; -SET LOCAL pg_plan_advice.advice = 'join_order(f/d1 d1 d2)'; +SET LOCAL pg_plan_advice.advice = 'join_order(f/public.d1 d1 d2)'; EXPLAIN (COSTS OFF, PLAN_ADVICE) SELECT * FROM jo_fact f LEFT JOIN jo_dim1 d1 ON f.dim1_id = d1.id @@ -273,7 +273,7 @@ SELECT * FROM jo_fact f Index Cond: (id = f.dim2_id) Filter: (val2 = 1) Supplied Plan Advice: - JOIN_ORDER(f/d1 d1 d2) /* partially matched */ + JOIN_ORDER(f/public.d1 d1 d2) /* partially matched */ Generated Plan Advice: JOIN_ORDER(f d1 d2) NESTED_LOOP_PLAIN(d1 d2) @@ -282,7 +282,7 @@ SELECT * FROM jo_fact f NO_GATHER(f d1 d2) (19 rows) -SET LOCAL pg_plan_advice.advice = 'join_order(f/d1 (d1 d2))'; +SET LOCAL pg_plan_advice.advice = 'join_order(f/public.d1 (d1 d2))'; EXPLAIN (COSTS OFF, PLAN_ADVICE) SELECT * FROM jo_fact f LEFT JOIN jo_dim1 d1 ON f.dim1_id = d1.id @@ -301,7 +301,7 @@ SELECT * FROM jo_fact f Filter: (val2 = 1) -> Seq Scan on jo_fact f Supplied Plan Advice: - JOIN_ORDER(f/d1 (d1 d2)) /* partially matched */ + JOIN_ORDER(f/public.d1 (d1 d2)) /* partially matched */ Generated Plan Advice: JOIN_ORDER(d1 d2 f) NESTED_LOOP_PLAIN(f) diff --git a/contrib/pg_plan_advice/expected/partitionwise.out b/contrib/pg_plan_advice/expected/partitionwise.out index 2b3d0a82443..2be65f95e39 100644 --- a/contrib/pg_plan_advice/expected/partitionwise.out +++ b/contrib/pg_plan_advice/expected/partitionwise.out @@ -306,8 +306,7 @@ SELECT * FROM pt1, ptmismatch WHERE pt1.id = ptmismatch.id; (22 rows) COMMIT; --- Force join order for a particular branch of the partitionwise join with --- and without mentioning the schema name. +-- Force join order for a particular branch of the partitionwise join. BEGIN; SET LOCAL pg_plan_advice.advice = 'JOIN_ORDER(pt3/public.pt3a pt2/public.pt2a pt1/public.pt1a)'; EXPLAIN (PLAN_ADVICE, COSTS OFF) @@ -366,61 +365,4 @@ 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) (49 rows) -SET LOCAL pg_plan_advice.advice = 'JOIN_ORDER(pt3/pt3a pt2/pt2a pt1/pt1a)'; -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 -------------------------------------------------------------------------------------- - Append - -> Nested Loop - -> Hash Join - Hash Cond: (pt3_1.id = pt2_1.id) - -> Seq Scan on pt3a pt3_1 - Filter: (val3 = 1) - -> Hash - -> Seq Scan on pt2a pt2_1 - Filter: (val2 = 1) - -> Index Scan using pt1a_pkey on pt1a pt1_1 - Index Cond: (id = pt2_1.id) - Filter: (val1 = 1) - -> Nested Loop - -> Hash Join - Hash Cond: (pt2_2.id = pt3_2.id) - -> Seq Scan on pt2b pt2_2 - Filter: (val2 = 1) - -> Hash - -> Seq Scan on pt3b pt3_2 - Filter: (val3 = 1) - -> Index Scan using pt1b_pkey on pt1b pt1_2 - Index Cond: (id = pt2_2.id) - Filter: (val1 = 1) - -> Nested Loop - -> Hash Join - Hash Cond: (pt2_3.id = pt3_3.id) - -> Seq Scan on pt2c pt2_3 - Filter: (val2 = 1) - -> Hash - -> Seq Scan on pt3c pt3_3 - Filter: (val3 = 1) - -> Index Scan using pt1c_pkey on pt1c pt1_3 - Index Cond: (id = pt2_3.id) - Filter: (val1 = 1) - Supplied Plan Advice: - JOIN_ORDER(pt3/pt3a pt2/pt2a pt1/pt1a) /* matched */ - Generated Plan Advice: - JOIN_ORDER(pt3/public.pt3a pt2/public.pt2a pt1/public.pt1a) - JOIN_ORDER(pt2/public.pt2b pt3/public.pt3b pt1/public.pt1b) - JOIN_ORDER(pt2/public.pt2c pt3/public.pt3c pt1/public.pt1c) - NESTED_LOOP_PLAIN(pt1/public.pt1a pt1/public.pt1b pt1/public.pt1c) - HASH_JOIN(pt2/public.pt2a pt3/public.pt3b pt3/public.pt3c) - SEQ_SCAN(pt3/public.pt3a pt2/public.pt2a pt2/public.pt2b pt3/public.pt3b - pt2/public.pt2c pt3/public.pt3c) - 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) - COMMIT; diff --git a/contrib/pg_plan_advice/expected/syntax.out b/contrib/pg_plan_advice/expected/syntax.out index 6062f245307..b699653b7e9 100644 --- a/contrib/pg_plan_advice/expected/syntax.out +++ b/contrib/pg_plan_advice/expected/syntax.out @@ -74,15 +74,6 @@ EXPLAIN (COSTS OFF) SELECT 1; SEQ_SCAN(x#10) /* not matched */ (3 rows) -SET pg_plan_advice.advice = 'SEQ_SCAN (x/y)'; -EXPLAIN (COSTS OFF) SELECT 1; - QUERY PLAN ------------------------------------ - Result - Supplied Plan Advice: - SEQ_SCAN(x/y) /* not matched */ -(3 rows) - SET pg_plan_advice.advice = ' SEQ_SCAN ( x / y . z ) '; EXPLAIN (COSTS OFF) SELECT 1; QUERY PLAN @@ -136,6 +127,10 @@ DETAIL: Could not parse advice: syntax error at or near "123" SET pg_plan_advice.advice = 'SEQ_SCAN(x#99999999999_99)'; ERROR: invalid value for parameter "pg_plan_advice.advice": "SEQ_SCAN(x#99999999999_99)" DETAIL: Could not parse advice: integer out of range at or near "99999999999_99" +-- Partition schema is missing. +SET pg_plan_advice.advice = 'SEQ_SCAN(x/y)'; +ERROR: invalid value for parameter "pg_plan_advice.advice": "SEQ_SCAN(x/y)" +DETAIL: Could not parse advice: syntax error at or near ")" -- Tags like SEQ_SCAN and NO_GATHER don't allow sublists at all; other tags, -- except for JOIN_ORDER, allow at most one level of sublist. Hence, these -- examples should error out. @@ -174,13 +169,13 @@ EXPLAIN (COSTS OFF) SELECT 1; HASH_JOIN(y) /* not matched */ (3 rows) -SET pg_plan_advice.advice = '/* comment */ HASH_JOIN(y//*x*/z)'; +SET pg_plan_advice.advice = '/* comment */ HASH_JOIN(y//*x*/z.zz)'; EXPLAIN (COSTS OFF) SELECT 1; - QUERY PLAN ------------------------------------- + QUERY PLAN +--------------------------------------- Result Supplied Plan Advice: - HASH_JOIN(y/z) /* not matched */ + HASH_JOIN(y/z.zz) /* not matched */ (3 rows) -- Unterminated comments. diff --git a/contrib/pg_plan_advice/pgpa_ast.c b/contrib/pg_plan_advice/pgpa_ast.c index 308ac21e6fe..943ea9d3d3f 100644 --- a/contrib/pg_plan_advice/pgpa_ast.c +++ b/contrib/pg_plan_advice/pgpa_ast.c @@ -257,19 +257,17 @@ pgpa_identifier_matches_target(pgpa_identifier *rid, pgpa_advice_target *target) return false; /* - * If a relation identifier mentions a partition name, it should also - * specify a partition schema. But the target may leave the schema NULL to - * match anything. + * A relation identifier should either include both of partition name and + * partition schema, or neither one. */ - Assert(rid->partnsp != NULL || rid->partrel == NULL); - if (rid->partnsp != NULL && target->rid.partnsp != NULL && - strcmp(rid->partnsp, target->rid.partnsp) != 0) - return false; + Assert((rid->partnsp == NULL) == (rid->partrel == NULL)); /* * These fields can be NULL on either side, but NULL only matches another * NULL. */ + if (!strings_equal_or_both_null(rid->partnsp, target->rid.partnsp)) + return false; if (!strings_equal_or_both_null(rid->partrel, target->rid.partrel)) return false; if (!strings_equal_or_both_null(rid->plan_name, target->rid.plan_name)) diff --git a/contrib/pg_plan_advice/pgpa_ast.h b/contrib/pg_plan_advice/pgpa_ast.h index 4bd6ffa5e3a..17b40efadaf 100644 --- a/contrib/pg_plan_advice/pgpa_ast.h +++ b/contrib/pg_plan_advice/pgpa_ast.h @@ -52,8 +52,8 @@ typedef struct pgpa_advice_target * This field is meaningful when ttype is PGPA_TARGET_IDENTIFIER. * * All identifiers must have an alias name and an occurrence number; the - * remaining fields can be NULL. Note that it's possible to specify a - * partition name without a partition schema, but not the reverse. + * remaining fields can be NULL. Note that a partition name and a + * partition schema must either both be specified or both be omitted. */ pgpa_identifier rid; diff --git a/contrib/pg_plan_advice/pgpa_identifier.c b/contrib/pg_plan_advice/pgpa_identifier.c index 21392b8e0d7..df5b524023f 100644 --- a/contrib/pg_plan_advice/pgpa_identifier.c +++ b/contrib/pg_plan_advice/pgpa_identifier.c @@ -20,11 +20,10 @@ * within the same subquery, child_table_name is omitted for relations that * are not child tables, and subquery_name is omitted for the topmost * query level. Whenever an item is omitted, the preceding punctuation mark - * is also omitted. Identifier-style escaping is applied to alias_name and - * subquery_name. In generated advice, child table names are always - * schema-qualified, but users can supply advice where the schema name is - * not mentioned. Identifier-style escaping is applied to the schema and to - * the relation name separately. + * is also omitted. Identifier-style escaping is applied to alias_name and + * subquery_name. Child table names are always schema-qualified. + * Identifier-style escaping is applied to the schema and to the relation + * name separately. * * The upshot of all of these rules is that in simple cases, the relation * identifier is textually identical to the alias name, making life easier @@ -88,16 +87,11 @@ pgpa_identifier_string(const pgpa_identifier *rid) if (rid->occurrence > 1) result = psprintf("%s#%d", result, rid->occurrence); + Assert((rid->partnsp == NULL) == (rid->partrel == NULL)); if (rid->partrel != NULL) - { - if (rid->partnsp == NULL) - result = psprintf("%s/%s", result, - quote_identifier(rid->partrel)); - else - result = psprintf("%s/%s.%s", result, - quote_identifier(rid->partnsp), - quote_identifier(rid->partrel)); - } + result = psprintf("%s/%s.%s", result, + quote_identifier(rid->partnsp), + quote_identifier(rid->partrel)); if (rid->plan_name != NULL) result = psprintf("%s@%s", result, quote_identifier(rid->plan_name)); @@ -364,14 +358,13 @@ pgpa_compute_rti_from_identifier(int rtable_length, continue; /* - * If it matches, return this RTI. As usual, an omitted partition - * schema matches anything, but partition and plan names must either - * match exactly or be omitted on both sides. + * If it matches, return this RTI. The partition schema, partition + * name, and plan name must either match exactly or be omitted on both + * sides. */ if (strcmp(rid->alias_name, rti_rid->alias_name) == 0 && rid->occurrence == rti_rid->occurrence && - (rid->partnsp == NULL || rti_rid->partnsp == NULL || - strcmp(rid->partnsp, rti_rid->partnsp) == 0) && + strings_equal_or_both_null(rid->partnsp, rti_rid->partnsp) && strings_equal_or_both_null(rid->partrel, rti_rid->partrel) && strings_equal_or_both_null(rid->plan_name, rti_rid->plan_name)) { diff --git a/contrib/pg_plan_advice/pgpa_parser.y b/contrib/pg_plan_advice/pgpa_parser.y index 8ffa1f1dd40..82c04b4b547 100644 --- a/contrib/pg_plan_advice/pgpa_parser.y +++ b/contrib/pg_plan_advice/pgpa_parser.y @@ -151,13 +151,12 @@ relation_identifier: identifier opt_ri_occurrence opt_partition opt_plan_name $$->ttype = PGPA_TARGET_IDENTIFIER; $$->rid.alias_name = $1; $$->rid.occurrence = $2; - if (list_length($3) == 2) + if ($3 != NIL) { + Assert(list_length($3) == 2); $$->rid.partnsp = linitial($3); $$->rid.partrel = lsecond($3); } - else if ($3 != NIL) - $$->rid.partrel = linitial($3); $$->rid.plan_name = $4; } ; @@ -198,14 +197,13 @@ identifier: TOK_IDENT ; /* - * When generating advice, we always schema-qualify the partition name, but - * when parsing advice, we accept a specification that lacks one. + * The partition name must always be schema-qualified. Otherwise, a relation + * identifier could refer to more than one partition of the same partitioned + * table, and relation identifiers are required to be unique. */ opt_partition: '/' identifier '.' identifier { $$ = list_make2($2, $4); } - | '/' identifier - { $$ = list_make1($2); } | { $$ = NIL; } ; diff --git a/contrib/pg_plan_advice/pgpa_trove.c b/contrib/pg_plan_advice/pgpa_trove.c index e5840051386..5cf73c768e2 100644 --- a/contrib/pg_plan_advice/pgpa_trove.c +++ b/contrib/pg_plan_advice/pgpa_trove.c @@ -59,18 +59,12 @@ struct pgpa_trove /* * We're going to build a hash table to allow clients of this module to find * relevant advice for a given part of the query quickly. However, we're going - * to use only three of the five key fields as hash keys. There are two reasons - * for this. - * - * First, it's allowable to set partition_schema to NULL to match a partition - * with the correct name in any schema. - * - * Second, we expect the "occurrence" and "partition_schema" portions of the - * relation identifiers to be mostly uninteresting. Most of the time, the - * occurrence field will be 1 and the partition_schema values will all be the - * same. Even when there is some variation, the absolute number of entries - * that have the same values for all three of these key fields should be - * quite small. + * to use only three of the five key fields as hash keys. That's because we + * expect the "occurrence" and "partition_schema" portions of the relation + * identifiers to be mostly uninteresting. Most of the time, the occurrence + * field will be 1 and the partition_schema values will all be the same. Even + * when there is some variation, the absolute number of entries that have the + * same values for all three of these key fields should be quite small. */ typedef struct { diff --git a/contrib/pg_plan_advice/sql/join_order.sql b/contrib/pg_plan_advice/sql/join_order.sql index 88d90de9cc6..b8219b2add8 100644 --- a/contrib/pg_plan_advice/sql/join_order.sql +++ b/contrib/pg_plan_advice/sql/join_order.sql @@ -85,13 +85,13 @@ COMMIT; -- jo_fact is not partitioned, but let's try pretending that it is and -- verifying that the advice does not apply. BEGIN; -SET LOCAL pg_plan_advice.advice = 'join_order(f/d1 d1 d2)'; +SET LOCAL pg_plan_advice.advice = 'join_order(f/public.d1 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 (d1 d2))'; +SET LOCAL pg_plan_advice.advice = 'join_order(f/public.d1 (d1 d2))'; EXPLAIN (COSTS OFF, PLAN_ADVICE) SELECT * FROM jo_fact f LEFT JOIN jo_dim1 d1 ON f.dim1_id = d1.id diff --git a/contrib/pg_plan_advice/sql/partitionwise.sql b/contrib/pg_plan_advice/sql/partitionwise.sql index c51456dbbb5..e836acc2a15 100644 --- a/contrib/pg_plan_advice/sql/partitionwise.sql +++ b/contrib/pg_plan_advice/sql/partitionwise.sql @@ -85,15 +85,10 @@ EXPLAIN (PLAN_ADVICE, COSTS OFF) SELECT * FROM pt1, ptmismatch WHERE pt1.id = ptmismatch.id; COMMIT; --- Force join order for a particular branch of the partitionwise join with --- and without mentioning the schema name. +-- Force join order for a particular branch of the partitionwise join. BEGIN; SET LOCAL pg_plan_advice.advice = 'JOIN_ORDER(pt3/public.pt3a pt2/public.pt2a pt1/public.pt1a)'; 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; -SET LOCAL pg_plan_advice.advice = 'JOIN_ORDER(pt3/pt3a pt2/pt2a pt1/pt1a)'; -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; diff --git a/contrib/pg_plan_advice/sql/syntax.sql b/contrib/pg_plan_advice/sql/syntax.sql index c762b8b19b2..58685b9372a 100644 --- a/contrib/pg_plan_advice/sql/syntax.sql +++ b/contrib/pg_plan_advice/sql/syntax.sql @@ -21,8 +21,6 @@ SET pg_plan_advice.advice = 'SEQ_scan(x#2)'; EXPLAIN (COSTS OFF) SELECT 1; SET pg_plan_advice.advice = 'SEQ_SCAN(x#1_0)'; EXPLAIN (COSTS OFF) SELECT 1; -SET pg_plan_advice.advice = 'SEQ_SCAN (x/y)'; -EXPLAIN (COSTS OFF) SELECT 1; SET pg_plan_advice.advice = ' SEQ_SCAN ( x / y . z ) '; EXPLAIN (COSTS OFF) SELECT 1; SET pg_plan_advice.advice = 'SEQ_SCAN("x"#2/"y"."z"@"t")'; @@ -43,6 +41,9 @@ SET pg_plan_advice.advice = '123'; -- Out of range values. SET pg_plan_advice.advice = 'SEQ_SCAN(x#99999999999_99)'; +-- Partition schema is missing. +SET pg_plan_advice.advice = 'SEQ_SCAN(x/y)'; + -- Tags like SEQ_SCAN and NO_GATHER don't allow sublists at all; other tags, -- except for JOIN_ORDER, allow at most one level of sublist. Hence, these -- examples should error out. @@ -57,7 +58,7 @@ SET pg_plan_advice.advice = 'HASH_JOIN(_)/***/'; EXPLAIN (COSTS OFF) SELECT 1; SET pg_plan_advice.advice = '/* comment */ HASH_JOIN(/*x*/y)'; EXPLAIN (COSTS OFF) SELECT 1; -SET pg_plan_advice.advice = '/* comment */ HASH_JOIN(y//*x*/z)'; +SET pg_plan_advice.advice = '/* comment */ HASH_JOIN(y//*x*/z.zz)'; EXPLAIN (COSTS OFF) SELECT 1; -- Unterminated comments. diff --git a/doc/src/sgml/pgplanadvice.sgml b/doc/src/sgml/pgplanadvice.sgml index 0116b0d88f7..4592a5ced54 100644 --- a/doc/src/sgml/pgplanadvice.sgml +++ b/doc/src/sgml/pgplanadvice.sgml @@ -223,11 +223,10 @@ alias_name#occurrence_number/partition_schema.partition_name@plan_name and are included only when required. When a component is omitted, the preceding punctuation must also be omitted. For the first occurrence of a relation within a given subquery, generated advice will omit the occurrence - number, but it is legal to write #1, if desired. The - partition schema and partition name are included only for children of - partitioned tables. In generated advice, pg_plan_advice - always includes both, but it is legal to omit the schema. The plan name is - omitted for the top-level plan, and must be included for any subplan. + number, but it is legal to write #1, if desired. For + children of partitioned tables, the partition schema and partition name are + required; otherwise, both are omitted. The plan name is omitted for the + top-level plan, and must be included for any subplan.