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 <lic@highgo.com>
Reviewed-by: Alexander Korotkov <aekorotkov@gmail.com>
Discussion: https://postgr.es/m/C18878AB-DEB2-4A61-9995-A035DD644B81@gmail.com

src/backend/partitioning/partbounds.c

@@ -5700,6 +5700,146 @@ 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 (the containment is then bidirectional).  Both checks honor
+ * the partition key collation via the operator-family comparators
+ * (partition_rbound_cmp / find_value_in_new_partitions_list), so e.g.
+ * ('a','b') and ('A','B') under a case-insensitive ICU collation are
+ * correctly recognised as the same bound.
+ */
+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 +6011,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);
 }

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

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.