From 3c033ade957420ffcc5fcac8f925a6c9a7a9f009 Mon Sep 17 00:00:00 2001 From: Alexander Korotkov Date: Mon, 18 May 2026 00:37:52 +0300 Subject: [PATCH v4 4/4] Reject degenerate SPLIT PARTITION with DEFAULT partition ALTER TABLE ... SPLIT PARTITION allows a DEFAULT partition to be created as one of the replacement partitions when the parent table does not already have one. However, it should not allow the degenerate case where a non-DEFAULT partition keeps exactly the same bound as the split partition and the command merely adds a DEFAULT partition through the SPLIT PARTITION path. Detect that case by comparing the bound of the split partition with the bound of the only non-DEFAULT replacement partition, and raise an error when they are the same. Users should add a DEFAULT partition directly with CREATE TABLE ... PARTITION OF ... DEFAULT or ALTER TABLE ... ATTACH PARTITION ... DEFAULT instead. Author: Chao Li Reviewed-by: Alexander Korotkov Discussion: https://postgr.es/m/C18878AB-DEB2-4A61-9995-A035DD644B81@gmail.com --- src/backend/partitioning/partbounds.c | 145 ++++++++++++++++++ src/test/regress/expected/partition_split.out | 59 +++++++ src/test/regress/sql/partition_split.sql | 52 +++++++ 3 files changed, 256 insertions(+) diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c index 7d3580cbc10..126a9be15ba 100644 --- a/src/backend/partitioning/partbounds.c +++ b/src/backend/partitioning/partbounds.c @@ -5700,6 +5700,141 @@ check_parent_values_in_new_partitions(Relation parent, } } +/* + * split_partition_values_contained_in_new_part + * + * (function for BY LIST partitioning) + * + * Returns true if all values in the LIST bound of the partition being split + * are contained in the specified non-DEFAULT replacement partition's bound. + * + * The caller must already have verified containment in the other direction, + * so this check is sufficient to prove that the two LIST bounds are equal. + */ +static bool +split_partition_values_contained_in_new_part(Relation parent, + Oid splitPartOid, + SinglePartitionSpec *part) +{ + PartitionKey key = RelationGetPartitionKey(parent); + PartitionDesc partdesc = RelationGetPartitionDesc(parent, false); + PartitionBoundInfo boundinfo = partdesc->boundinfo; + SinglePartitionSpec *parts[1]; + Datum datum = PointerGetDatum(NULL); + + Assert(key->strategy == PARTITION_STRATEGY_LIST); + + parts[0] = part; + + /* + * Special processing for NULL value. Search for a NULL value if the + * split partition contains it. + */ + if (partition_bound_accepts_nulls(boundinfo) && + partdesc->oids[boundinfo->null_index] == splitPartOid) + { + if (!find_value_in_new_partitions_list(&key->partsupfunc[0], + key->partcollation, parts, 1, + datum, true)) + return false; + } + + /* + * Search all values of the split partition in the single non-DEFAULT + * replacement partition. + */ + for (int i = 0; i < boundinfo->ndatums; i++) + { + if (partdesc->oids[boundinfo->indexes[i]] == splitPartOid) + { + datum = boundinfo->datums[i][0]; + + if (!find_value_in_new_partitions_list(&key->partsupfunc[0], + key->partcollation, parts, 1, + datum, false)) + return false; + } + } + + return true; +} + +/* + * check_split_partition_not_same_bound + * + * Reject splitting a non-DEFAULT partition into one non-DEFAULT partition + * with the original bound plus a DEFAULT partition. That form does not + * perform a real split; it merely adds a DEFAULT partition to the parent + * table through the split-partition path. Users should use + * CREATE TABLE ... PARTITION OF ... DEFAULT or ALTER TABLE ... ATTACH + * PARTITION ... DEFAULT for that. + * + * Must be called after the per-partition bound validation in + * check_partitions_for_split() so that containment of new bounds within the + * split partition is already established. Given containment, RANGE bounds + * are equal iff their lower and upper rbounds match; LIST bound sets are + * equal iff the split partition's values are also contained in the new + * partition. + */ +static void +check_split_partition_not_same_bound(Relation parent, + Oid splitPartOid, + SinglePartitionSpec **parts, + int nparts, + ParseState *pstate) +{ + PartitionKey key = RelationGetPartitionKey(parent); + PartitionBoundSpec *new_spec; + PartitionBoundSpec *split_spec; + + if (nparts != 1) + return; + + new_spec = parts[0]->bound; + split_spec = get_partition_bound_spec(splitPartOid); + + Assert(new_spec->strategy == split_spec->strategy); + + if (key->strategy == PARTITION_STRATEGY_RANGE) + { + PartitionRangeBound *new_lower; + PartitionRangeBound *new_upper; + PartitionRangeBound *split_lower; + PartitionRangeBound *split_upper; + + new_lower = make_one_partition_rbound(key, -1, new_spec->lowerdatums, true); + new_upper = make_one_partition_rbound(key, -1, new_spec->upperdatums, false); + split_lower = make_one_partition_rbound(key, -1, split_spec->lowerdatums, true); + split_upper = make_one_partition_rbound(key, -1, split_spec->upperdatums, false); + + if (partition_rbound_cmp(key->partnatts, key->partsupfunc, + key->partcollation, + new_lower->datums, new_lower->kind, true, + split_lower) != 0) + return; + if (partition_rbound_cmp(key->partnatts, key->partsupfunc, + key->partcollation, + new_upper->datums, new_upper->kind, false, + split_upper) != 0) + return; + } + else + { + Assert(key->strategy == PARTITION_STRATEGY_LIST); + + if (!split_partition_values_contained_in_new_part(parent, splitPartOid, parts[0])) + return; + } + + ereport(ERROR, + errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("cannot split partition \"%s\" only to add a DEFAULT partition", + get_rel_name(splitPartOid)), + errdetail("The non-DEFAULT partition would keep the same partition bound."), + errhint("Use CREATE TABLE ... PARTITION OF ... DEFAULT to add a DEFAULT partition."), + parser_errposition(pstate, parts[0]->name->location)); +} + /* * check_partitions_for_split * @@ -5871,5 +6006,15 @@ check_partitions_for_split(Relation parent, new_parts, nparts, pstate); } + /* + * Reject the degenerate form where the single non-DEFAULT replacement + * partition keeps the bound of the split partition; the command then does + * nothing beyond adding a DEFAULT partition. Containment was established + * by the per-partition validation above, so an equality check is enough. + */ + if (!isSplitPartDefault && createDefaultPart) + check_split_partition_not_same_bound(parent, splitPartOid, new_parts, + nparts, pstate); + pfree(new_parts); } diff --git a/src/test/regress/expected/partition_split.out b/src/test/regress/expected/partition_split.out index 2b9a6aa50ed..2fd9aee1dcc 100644 --- a/src/test/regress/expected/partition_split.out +++ b/src/test/regress/expected/partition_split.out @@ -1188,6 +1188,65 @@ SELECT tableoid::regclass, * FROM sales_range ORDER BY tableoid::regclass::text DROP TABLE sales_range; -- +-- Test that SPLIT PARTITION rejects the degenerate case where the only +-- non-DEFAULT replacement partition keeps the original bound and the command +-- merely adds a DEFAULT partition. +-- +CREATE TABLE t (i int) PARTITION BY RANGE (i); +CREATE TABLE tp_0_50 PARTITION OF t FOR VALUES FROM (0) TO (50); +INSERT INTO t VALUES (1); +-- ERROR +ALTER TABLE t SPLIT PARTITION tp_0_50 INTO + (PARTITION tp_0_50 FOR VALUES FROM (0) TO (50), + PARTITION tp_default DEFAULT); +ERROR: cannot split partition "tp_0_50" only to add a DEFAULT partition +LINE 2: (PARTITION tp_0_50 FOR VALUES FROM (0) TO (50), + ^ +DETAIL: The non-DEFAULT partition would keep the same partition bound. +HINT: Use CREATE TABLE ... PARTITION OF ... DEFAULT to add a DEFAULT partition. +DROP TABLE t; +-- +-- Test that a LIST split with DEFAULT is not considered degenerate when +-- only NULL is removed from the explicit replacement partition. +-- +CREATE TABLE t (i int) PARTITION BY LIST (i); +CREATE TABLE tp_null_1 PARTITION OF t FOR VALUES IN (NULL, 1); +ALTER TABLE t SPLIT PARTITION tp_null_1 INTO + (PARTITION tp_1 FOR VALUES IN (1), + PARTITION tp_default DEFAULT); +INSERT INTO t VALUES (NULL), (1), (2); +SELECT tableoid::regclass, i FROM t ORDER BY tableoid::regclass::text COLLATE "C", i NULLS FIRST; + tableoid | i +------------+--- + tp_1 | 1 + tp_default | + tp_default | 2 +(3 rows) + +DROP TABLE t; +-- +-- Test that the same-bound check for LIST partitioning uses partition +-- comparison semantics, not raw list length. The case-insensitive collation +-- treats 'a' and 'A' as equal, so the non-DEFAULT replacement partition +-- covers only the 'a' group and the DEFAULT partition covers the rest. +-- +CREATE COLLATION case_insensitive (provider = icu, locale = 'und-u-ks-level2', deterministic = false); +CREATE TABLE t (b text COLLATE case_insensitive) PARTITION BY LIST (b); +CREATE TABLE tp_ab PARTITION OF t FOR VALUES IN ('a', 'b'); +ALTER TABLE t SPLIT PARTITION tp_ab INTO + (PARTITION tp_a FOR VALUES IN ('a', 'A'), + PARTITION tp_default DEFAULT); +INSERT INTO t VALUES ('a'), ('A'), ('b'), ('c'); +SELECT tableoid::regclass, count(*) FROM t GROUP BY 1 ORDER BY 1; + tableoid | count +------------+------- + tp_a | 2 + tp_default | 2 +(2 rows) + +DROP TABLE t; +DROP COLLATION case_insensitive; +-- -- Test that the explicit partition bound cannot extend outside the split -- partition's bound when a DEFAULT partition is specified. -- diff --git a/src/test/regress/sql/partition_split.sql b/src/test/regress/sql/partition_split.sql index d9821c5e2a3..ede89ad0228 100644 --- a/src/test/regress/sql/partition_split.sql +++ b/src/test/regress/sql/partition_split.sql @@ -834,6 +834,58 @@ SELECT tableoid::regclass, * FROM sales_range ORDER BY tableoid::regclass::text DROP TABLE sales_range; +-- +-- Test that SPLIT PARTITION rejects the degenerate case where the only +-- non-DEFAULT replacement partition keeps the original bound and the command +-- merely adds a DEFAULT partition. +-- +CREATE TABLE t (i int) PARTITION BY RANGE (i); +CREATE TABLE tp_0_50 PARTITION OF t FOR VALUES FROM (0) TO (50); +INSERT INTO t VALUES (1); + +-- ERROR +ALTER TABLE t SPLIT PARTITION tp_0_50 INTO + (PARTITION tp_0_50 FOR VALUES FROM (0) TO (50), + PARTITION tp_default DEFAULT); + +DROP TABLE t; + +-- +-- Test that a LIST split with DEFAULT is not considered degenerate when +-- only NULL is removed from the explicit replacement partition. +-- +CREATE TABLE t (i int) PARTITION BY LIST (i); +CREATE TABLE tp_null_1 PARTITION OF t FOR VALUES IN (NULL, 1); + +ALTER TABLE t SPLIT PARTITION tp_null_1 INTO + (PARTITION tp_1 FOR VALUES IN (1), + PARTITION tp_default DEFAULT); + +INSERT INTO t VALUES (NULL), (1), (2); +SELECT tableoid::regclass, i FROM t ORDER BY tableoid::regclass::text COLLATE "C", i NULLS FIRST; + +DROP TABLE t; + +-- +-- Test that the same-bound check for LIST partitioning uses partition +-- comparison semantics, not raw list length. The case-insensitive collation +-- treats 'a' and 'A' as equal, so the non-DEFAULT replacement partition +-- covers only the 'a' group and the DEFAULT partition covers the rest. +-- +CREATE COLLATION case_insensitive (provider = icu, locale = 'und-u-ks-level2', deterministic = false); +CREATE TABLE t (b text COLLATE case_insensitive) PARTITION BY LIST (b); +CREATE TABLE tp_ab PARTITION OF t FOR VALUES IN ('a', 'b'); + +ALTER TABLE t SPLIT PARTITION tp_ab INTO + (PARTITION tp_a FOR VALUES IN ('a', 'A'), + PARTITION tp_default DEFAULT); + +INSERT INTO t VALUES ('a'), ('A'), ('b'), ('c'); +SELECT tableoid::regclass, count(*) FROM t GROUP BY 1 ORDER BY 1; + +DROP TABLE t; +DROP COLLATION case_insensitive; + -- -- Test that the explicit partition bound cannot extend outside the split -- partition's bound when a DEFAULT partition is specified. -- 2.50.1 (Apple Git-155)