database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2026-10-23 13:29:09 +03:00
Code Activity

pg_plan_advice: Disallow partition name without partition schema.

Browse Source 
Up until now, pg_plan_advice has had a feature that allows an advice
target to mention a partition name but omit the partition schema;
that is, something like SEQ_SCAN(foo/bar) forces a sequential scan on
every child of table foo whose partition name is bar, regardless of
the schema in which bar appears. However, that feature turns out to
have a nasty design flaw: while advice enforcement handles this case
just fine, the advice feedback code doesn't know about it and will
mark such advice as "matched, failed" even when everything worked
perfectly. Unfortunately, there seems to be no simple code fix for
this problem.

Since the release of PostgreSQL 19 is imminent, take the conservative
course and revert this feature of pg_plan_advice. In other words,
require the partition schema whenever the partition name is present.
You must now write e.g. SEQ_SCAN(foo/public.bar) rather than just
SEQ_SCAN(foo/bar). This doesn't affect any cases where automatically
generated advice is supplied, since generated advice has always
included the partition schema anyway. Manually written advice will
have to conform to the new, stricter rule. For a later release, we
can consider whether to again relax this restriction in some way,
but now is not the time to design new things.

Reported-by: Noah Misch <noah@leadboat.com>
Backpatch-through: 19
Discussion: https://postgr.es/m/CA+TgmoYmXy-jiP5qDhqNEiYFEBzQsArO6O2d9E8szNZqi1bePQ@mail.gmail.com
This commit is contained in:
Robert Haas
2026-09-16 21:03:58 -04:00
parent 4a9a6c5a69
commit 3201ea8f17
13 changed files with 58 additions and 145 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
contrib/pg_plan_advice/README
View File

@@ -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
===========

8
contrib/pg_plan_advice/expected/join_order.out
View File

@@ -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)

60
contrib/pg_plan_advice/expected/partitionwise.out
View File

@@ -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;

21
contrib/pg_plan_advice/expected/syntax.out
View File

@@ -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.

12
contrib/pg_plan_advice/pgpa_ast.c
View File

@@ -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))

4
contrib/pg_plan_advice/pgpa_ast.h
View File

@@ -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;

31
contrib/pg_plan_advice/pgpa_identifier.c
View File

@@ -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))
{

12
contrib/pg_plan_advice/pgpa_parser.y
View File

@@ -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; }
;

18
contrib/pg_plan_advice/pgpa_trove.c
View File

@@ -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
{

4
contrib/pg_plan_advice/sql/join_order.sql
View File

@@ -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

7
contrib/pg_plan_advice/sql/partitionwise.sql
View File

@@ -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;

7
contrib/pg_plan_advice/sql/syntax.sql
View File

@@ -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.

9
doc/src/sgml/pgplanadvice.sgml
View File

@@ -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 <literal>#1</literal>, if desired. The
partition schema and partition name are included only for children of
partitioned tables. In generated advice, <literal>pg_plan_advice</literal>
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 <literal>#1</literal>, 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.
</para>
<para>