public inbox for [email protected]  
help / color / mirror / Atom feed
From: Alexander Korotkov <[email protected]>
To: Chao Li <[email protected]>
Cc: Dmitry Koval <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: Fix SPLIT PARTITION bound-overlap bug and other improvements
Date: Mon, 18 May 2026 12:16:19 +0300
Message-ID: <CAPpHfdsLnD3NGnPC32oeg4ZWWX441M=SfH0-7bG652G40TpEEQ@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<CAPpHfdvdevTMpjQe09aWdK_nbrE4+M1z09H2SUe3bgxY3_QUdA@mail.gmail.com>
	<[email protected]>

On Mon, May 18, 2026 at 2:15 AM Chao Li <[email protected]> wrote:
> > On May 18, 2026, at 05:45, Alexander Korotkov <[email protected]> wrote:
> >
> > Hi, Chao!
> >
> > On Thu, May 14, 2026 at 9:59 AM Chao Li <[email protected]> wrote:
> >> To make this patch easier to process, I split it into 4 commits:
> >>
> >> 0001 - Fixes the bound-overlap bug
> >> 0002 - Fix the incorrect HINT message for the DEFAULT case
> >> 0003 - Fix the incorrect description about combined bound in the SGML doc
> >> 0004 - Reject only-create-default-partition usage
> >
> > Thank you for your work.  I've revised the patchset.
> > 0002 - I've also fixed gramma of hints in other branches
> > 0004 - In the check_split_partition_not_same_bound(), calling
> > partition_bounds_create() and partition_bounds_equal() looks a bit
> > heavyweight.  It doesn't matter much performance-wise, but it feels
> > like start processing from scratch while we're on quite late stage
> > already.  I've replaced that with more lightweight check.  Also I
> > removed dealing with memory context.  This code implies small
> > non-repetitive memory allocations which only lives during DDL
> > operation, no need to wrap them with memory context as we don't do so
> > in other places.
> >
> > Any objections if I commit this?
> >
> > ------
> > Regards,
> > Alexander Korotkov
> > Supabase
>
> Hi Alexander,
>
> Thanks for the revisions. I think you may have missed the attachments, so I cannot review the changes.

Sorry. Here it is.

------
Regards,
Alexander Korotkov
Supabase


Attachments:

  [application/octet-stream] v3-0003-Clarify-SPLIT-PARTITION-bound-requirements-in-doc.patch (3.8K, 2-v3-0003-Clarify-SPLIT-PARTITION-bound-requirements-in-doc.patch)
  download | inline diff:
From 6749ef320e085e613f17ff917560b06d751f56b4 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Mon, 18 May 2026 00:37:06 +0300
Subject: [PATCH v3 3/4] Clarify SPLIT PARTITION bound requirements in docs

The documentation said that the bounds of new partitions should not
overlap and that their combined bounds should equal the bounds of the
split partition.  That is misleading when a new DEFAULT partition is
specified, because the explicit partitions may cover only part of the
split partition while the DEFAULT partition covers the rest.

Clarify that new non-DEFAULT partition bounds must not overlap with
other new or existing partitions and must be contained within the bounds
of the split partition.  Also state that the combined bounds must exactly
match the split partition only when no new DEFAULT partition is specified.

While here, improve nearby wording about hash-partitioned target tables
and splitting a DEFAULT partition with the same partition name.

Author: Chao Li <[email protected]>
Reviewed-by: Alexander Korotkov <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
 doc/src/sgml/ref/alter_table.sgml | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 1f9a456fd33..dec34337d1a 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -1293,7 +1293,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
     <listitem>
      <para>
       This form splits a single partition of the target table into new
-      partitions. Hash-partitioned target table is not supported.
+      partitions.  Hash-partitioned target tables are not supported.
       Only a simple, non-partitioned partition can be split.
       If the split partition is the <literal>DEFAULT</literal> partition,
       one of the new partitions must be <literal>DEFAULT</literal>.
@@ -1303,18 +1303,23 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
      </para>
 
      <para>
-      The bounds of new partitions should not overlap with those of new or
-      existing partitions (except <replaceable class="parameter">partition_name</replaceable>).
-      The combined bounds of new partitions <literal>
+      The bounds of new non-<literal>DEFAULT</literal> partitions must not
+      overlap with those of new or existing partitions, except
+      <replaceable class="parameter">partition_name</replaceable>, and must be
+      contained within the bounds of the split partition
+      <replaceable class="parameter">partition_name</replaceable>.
+      If no new <literal>DEFAULT</literal> partition is specified, the
+      combined bounds of the new partitions
+      <literal>
       <replaceable class="parameter">partition_name1</replaceable>,
       <replaceable class="parameter">partition_name2</replaceable>[, ...]
-      </literal> should be equal to the bounds of the split partition
+      </literal> must exactly match the bounds of the split partition
       <replaceable class="parameter">partition_name</replaceable>.
       One of the new partitions can have the same name as the split partition
-      <replaceable class="parameter">partition_name</replaceable>
-      (this is suitable in case of splitting the <literal>DEFAULT</literal>
-      partition: after the split, the <literal>DEFAULT</literal> partition
-      remains with the same name, but its partition bound changes).
+      <replaceable class="parameter">partition_name</replaceable>.
+      This is useful when splitting the <literal>DEFAULT</literal> partition,
+      so that after the split, the <literal>DEFAULT</literal> partition
+      keeps the same name but its partition bound changes.
      </para>
 
      <para>
-- 
2.39.5 (Apple Git-154)



  [application/octet-stream] v3-0001-Fix-SPLIT-PARTITION-range-bound-validation-with-D.patch (4.1K, 3-v3-0001-Fix-SPLIT-PARTITION-range-bound-validation-with-D.patch)
  download | inline diff:
From 52b66023c56f79ce955d351013b51d91b227e628 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Mon, 18 May 2026 00:36:23 +0300
Subject: [PATCH v3 1/4] Fix SPLIT PARTITION range bound validation with
 DEFAULT

When splitting a range partition and defining a new DEFAULT partition, the
validation checked the lower bound of the first explicit partition and the
upper bound of explicit partitions only when they were not first.  If there
was exactly one explicit non-DEFAULT partition, its upper bound was therefore
not checked.

This could allow the replacement partition to extend beyond the upper bound
of the partition being split, potentially overlapping another existing
partition.

Fix this by checking the upper bound whenever the explicit partition is the
last one.  Add a regression test covering the single explicit partition plus
DEFAULT case.

Author: Chao Li <[email protected]>
Reviewed-by: Kirill Reshke <[email protected]>
Reviewed-by: Zhenwei Shang <[email protected]>
Reviewed-by: Dmitry Koval <[email protected]>
Reviewed-by: Alexander Korotkov <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
 src/backend/partitioning/partbounds.c         |  3 ++-
 src/test/regress/expected/partition_split.out | 16 ++++++++++++++++
 src/test/regress/sql/partition_split.sql      | 15 +++++++++++++++
 3 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c
index a09beec34d8..73dea0375be 100644
--- a/src/backend/partitioning/partbounds.c
+++ b/src/backend/partitioning/partbounds.c
@@ -5419,7 +5419,8 @@ check_partition_bounds_for_split_range(Relation parent,
 								"ALTER TABLE ... SPLIT PARTITION"),
 						parser_errposition(pstate, exprLocation((Node *) datum)));
 		}
-		else
+
+		if (last)
 		{
 			PartitionRangeBound *split_upper;
 
diff --git a/src/test/regress/expected/partition_split.out b/src/test/regress/expected/partition_split.out
index 961b37953c8..a2ccbe5138b 100644
--- a/src/test/regress/expected/partition_split.out
+++ b/src/test/regress/expected/partition_split.out
@@ -1188,6 +1188,22 @@ SELECT tableoid::regclass, * FROM sales_range ORDER BY tableoid::regclass::text
 
 DROP TABLE sales_range;
 --
+-- Test that the explicit partition bound cannot extend outside the split
+-- partition's bound when a DEFAULT partition is specified.
+--
+CREATE TABLE t (i int) PARTITION BY RANGE (i);
+CREATE TABLE tp_0_51 PARTITION OF t FOR VALUES FROM (0) TO (51);
+CREATE TABLE tp_51_100 PARTITION OF t FOR VALUES FROM (51) TO (100);
+-- ERROR
+ALTER TABLE t SPLIT PARTITION tp_0_51 INTO
+  (PARTITION tp_0_51 FOR VALUES FROM (0) TO (53),
+   PARTITION tp_default DEFAULT);
+ERROR:  upper bound of partition "tp_0_51" is greater than upper bound of split partition "tp_0_51"
+LINE 2:   (PARTITION tp_0_51 FOR VALUES FROM (0) TO (53),
+                                                     ^
+HINT:  ALTER TABLE ... SPLIT PARTITION require combined bounds of new partitions must exactly match the bound of the split partition.
+DROP TABLE t;
+--
 -- Try to SPLIT partition of another table.
 --
 CREATE TABLE t1(i int, t text) PARTITION BY LIST (t);
diff --git a/src/test/regress/sql/partition_split.sql b/src/test/regress/sql/partition_split.sql
index a110fc87867..d9821c5e2a3 100644
--- a/src/test/regress/sql/partition_split.sql
+++ b/src/test/regress/sql/partition_split.sql
@@ -834,6 +834,21 @@ SELECT tableoid::regclass, * FROM sales_range ORDER BY tableoid::regclass::text
 
 DROP TABLE sales_range;
 
+--
+-- Test that the explicit partition bound cannot extend outside the split
+-- partition's bound when a DEFAULT partition is specified.
+--
+CREATE TABLE t (i int) PARTITION BY RANGE (i);
+CREATE TABLE tp_0_51 PARTITION OF t FOR VALUES FROM (0) TO (51);
+CREATE TABLE tp_51_100 PARTITION OF t FOR VALUES FROM (51) TO (100);
+
+-- ERROR
+ALTER TABLE t SPLIT PARTITION tp_0_51 INTO
+  (PARTITION tp_0_51 FOR VALUES FROM (0) TO (53),
+   PARTITION tp_default DEFAULT);
+
+DROP TABLE t;
+
 --
 -- Try to SPLIT partition of another table.
 --
-- 
2.39.5 (Apple Git-154)



  [application/octet-stream] v3-0002-Fix-SPLIT-PARTITION-hint-for-DEFAULT-partition-bo.patch (10.1K, 4-v3-0002-Fix-SPLIT-PARTITION-hint-for-DEFAULT-partition-bo.patch)
  download | inline diff:
From 8b93d47913ead017bbe182b115d1069d514410eb Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Mon, 18 May 2026 00:36:44 +0300
Subject: [PATCH v3 2/4] Fix SPLIT PARTITION hint for DEFAULT partition bounds

When ALTER TABLE ... SPLIT PARTITION specifies a DEFAULT partition, the
explicit partitions do not need to cover the split partition's bound
exactly.  They may cover only part of it, with the DEFAULT partition
covering the remaining range.

However, the existing hint said that the combined bounds of the new
partitions must exactly match the bound of the split partition, which is
misleading for this case and inconsistent with the code comment.

Fix the hint to state the actual requirement: explicit partition bounds
must stay within the bounds of the split partition when a DEFAULT
partition is specified.

Author: Chao Li <[email protected]>
Reviewed-by: Alexander Korotkov <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
 src/backend/partitioning/partbounds.c         | 14 ++++++--------
 src/test/regress/expected/partition_split.out | 14 +++++++-------
 2 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c
index 73dea0375be..7d3580cbc10 100644
--- a/src/backend/partitioning/partbounds.c
+++ b/src/backend/partitioning/partbounds.c
@@ -5405,7 +5405,7 @@ check_partition_bounds_for_split_range(Relation parent,
 							errmsg("lower bound of partition \"%s\" is not equal to lower bound of split partition \"%s\"",
 								   relname,
 								   get_rel_name(splitPartOid)),
-							errhint("%s require combined bounds of new partitions must exactly match the bound of the split partition.",
+							errhint("%s requires the combined bounds of the new partitions to exactly match the bound of the split partition.",
 									"ALTER TABLE ... SPLIT PARTITION"),
 							parser_errposition(pstate, exprLocation((Node *) datum)));
 			}
@@ -5415,8 +5415,7 @@ check_partition_bounds_for_split_range(Relation parent,
 						errmsg("lower bound of partition \"%s\" is less than lower bound of split partition \"%s\"",
 							   relname,
 							   get_rel_name(splitPartOid)),
-						errhint("%s require combined bounds of new partitions must exactly match the bound of the split partition.",
-								"ALTER TABLE ... SPLIT PARTITION"),
+						errhint("Explicit partition bounds must be contained within the bounds of the split partition when a DEFAULT partition is specified."),
 						parser_errposition(pstate, exprLocation((Node *) datum)));
 		}
 
@@ -5448,7 +5447,7 @@ check_partition_bounds_for_split_range(Relation parent,
 							errmsg("upper bound of partition \"%s\" is not equal to upper bound of split partition \"%s\"",
 								   relname,
 								   get_rel_name(splitPartOid)),
-							errhint("%s require combined bounds of new partitions must exactly match the bound of the split partition.",
+							errhint("%s requires the combined bounds of the new partitions to exactly match the bound of the split partition.",
 									"ALTER TABLE ... SPLIT PARTITION"),
 							parser_errposition(pstate, exprLocation((Node *) datum)));
 			}
@@ -5458,8 +5457,7 @@ check_partition_bounds_for_split_range(Relation parent,
 						errmsg("upper bound of partition \"%s\" is greater than upper bound of split partition \"%s\"",
 							   relname,
 							   get_rel_name(splitPartOid)),
-						errhint("%s require combined bounds of new partitions must exactly match the bound of the split partition.",
-								"ALTER TABLE ... SPLIT PARTITION"),
+						errhint("Explicit partition bounds must be contained within the bounds of the split partition when a DEFAULT partition is specified."),
 						parser_errposition(pstate, exprLocation((Node *) datum)));
 		}
 	}
@@ -5654,7 +5652,7 @@ check_parent_values_in_new_partitions(Relation parent,
 				errmsg("new partitions' combined partition bounds do not contain value (%s) but split partition \"%s\" does",
 					   "NULL",
 					   get_rel_name(partOid)),
-				errhint("%s require combined bounds of new partitions must exactly match the bound of the split partition.",
+				errhint("%s requires the combined bounds of the new partitions to exactly match the bound of the split partition.",
 						"ALTER TABLE ... SPLIT PARTITION"));
 
 	/*
@@ -5697,7 +5695,7 @@ check_parent_values_in_new_partitions(Relation parent,
 				errmsg("new partitions' combined partition bounds do not contain value (%s) but split partition \"%s\" does",
 					   deparse_expression((Node *) notFoundVal, NIL, false, false),
 					   get_rel_name(partOid)),
-				errhint("%s require combined bounds of new partitions must exactly match the bound of the split partition.",
+				errhint("%s requires the combined bounds of the new partitions to exactly match the bound of the split partition.",
 						"ALTER TABLE ... SPLIT PARTITION"));
 	}
 }
diff --git a/src/test/regress/expected/partition_split.out b/src/test/regress/expected/partition_split.out
index a2ccbe5138b..2b9a6aa50ed 100644
--- a/src/test/regress/expected/partition_split.out
+++ b/src/test/regress/expected/partition_split.out
@@ -56,7 +56,7 @@ ALTER TABLE sales_range SPLIT PARTITION sales_feb_mar_apr2022 INTO
 ERROR:  lower bound of partition "sales_feb2022" is not equal to lower bound of split partition "sales_feb_mar_apr2022"
 LINE 2:   (PARTITION sales_feb2022 FOR VALUES FROM ('2022-01-01') TO...
                                                     ^
-HINT:  ALTER TABLE ... SPLIT PARTITION require combined bounds of new partitions must exactly match the bound of the split partition.
+HINT:  ALTER TABLE ... SPLIT PARTITION requires the combined bounds of the new partitions to exactly match the bound of the split partition.
 -- ERROR
 -- (We can create partition with the same name as split partition, but can't create two partitions with the same name)
 ALTER TABLE sales_range SPLIT PARTITION sales_feb_mar_apr2022 INTO
@@ -97,7 +97,7 @@ ALTER TABLE sales_range SPLIT PARTITION sales_feb_mar_apr2022 INTO
 ERROR:  upper bound of partition "sales_apr2022" is not equal to upper bound of split partition "sales_feb_mar_apr2022"
 LINE 4: ... sales_apr2022 FOR VALUES FROM ('2022-04-01') TO ('2022-06-0...
                                                              ^
-HINT:  ALTER TABLE ... SPLIT PARTITION require combined bounds of new partitions must exactly match the bound of the split partition.
+HINT:  ALTER TABLE ... SPLIT PARTITION requires the combined bounds of the new partitions to exactly match the bound of the split partition.
 -- ERROR
 ALTER TABLE sales_range SPLIT PARTITION sales_feb_mar_apr2022 INTO
   (PARTITION sales_feb2022 FOR VALUES FROM ('2022-02-01') TO ('2022-03-01'),
@@ -118,7 +118,7 @@ ALTER TABLE sales_range SPLIT PARTITION sales_feb_mar_apr2022 INTO
 ERROR:  lower bound of partition "sales_feb2022" is not equal to lower bound of split partition "sales_feb_mar_apr2022"
 LINE 2:   (PARTITION sales_feb2022 FOR VALUES FROM ('2022-02-02') TO...
                                                     ^
-HINT:  ALTER TABLE ... SPLIT PARTITION require combined bounds of new partitions must exactly match the bound of the split partition.
+HINT:  ALTER TABLE ... SPLIT PARTITION requires the combined bounds of the new partitions to exactly match the bound of the split partition.
 -- Check the source partition not in the search path
 SET search_path = partition_split_schema2, public;
 ALTER TABLE partition_split_schema.sales_range
@@ -154,7 +154,7 @@ ALTER TABLE sales_range SPLIT PARTITION sales_feb_mar_apr2022 INTO
 ERROR:  upper bound of partition "sales_apr2022" is not equal to upper bound of split partition "sales_feb_mar_apr2022"
 LINE 4: ... sales_apr2022 FOR VALUES FROM ('2022-04-01') TO ('2022-06-0...
                                                              ^
-HINT:  ALTER TABLE ... SPLIT PARTITION require combined bounds of new partitions must exactly match the bound of the split partition.
+HINT:  ALTER TABLE ... SPLIT PARTITION requires the combined bounds of the new partitions to exactly match the bound of the split partition.
 DROP TABLE sales_range;
 --
 -- Add rows into partitioned table then split partition
@@ -917,14 +917,14 @@ ALTER TABLE sales_list SPLIT PARTITION sales_all INTO
    PARTITION sales_east FOR VALUES IN ('Beijing', 'Delhi', 'Vladivostok'),
    PARTITION sales_central FOR VALUES IN ('Warsaw', 'Berlin', 'Kyiv'));
 ERROR:  new partitions' combined partition bounds do not contain value (NULL) but split partition "sales_all" does
-HINT:  ALTER TABLE ... SPLIT PARTITION require combined bounds of new partitions must exactly match the bound of the split partition.
+HINT:  ALTER TABLE ... SPLIT PARTITION requires the combined bounds of the new partitions to exactly match the bound of the split partition.
 -- ERROR
 ALTER TABLE sales_list SPLIT PARTITION sales_all INTO
   (PARTITION sales_west FOR VALUES IN ('Lisbon', 'New York', 'Madrid'),
    PARTITION sales_east FOR VALUES IN ('Beijing', 'Delhi', 'Vladivostok'),
    PARTITION sales_central FOR VALUES IN ('Warsaw', 'Berlin', NULL));
 ERROR:  new partitions' combined partition bounds do not contain value ('Kyiv'::character varying(20)) but split partition "sales_all" does
-HINT:  ALTER TABLE ... SPLIT PARTITION require combined bounds of new partitions must exactly match the bound of the split partition.
+HINT:  ALTER TABLE ... SPLIT PARTITION requires the combined bounds of the new partitions to exactly match the bound of the split partition.
 -- ERROR
 ALTER TABLE sales_list SPLIT PARTITION sales_all INTO
   (PARTITION sales_west FOR VALUES IN ('Lisbon', 'New York', 'Madrid'),
@@ -1201,7 +1201,7 @@ ALTER TABLE t SPLIT PARTITION tp_0_51 INTO
 ERROR:  upper bound of partition "tp_0_51" is greater than upper bound of split partition "tp_0_51"
 LINE 2:   (PARTITION tp_0_51 FOR VALUES FROM (0) TO (53),
                                                      ^
-HINT:  ALTER TABLE ... SPLIT PARTITION require combined bounds of new partitions must exactly match the bound of the split partition.
+HINT:  Explicit partition bounds must be contained within the bounds of the split partition when a DEFAULT partition is specified.
 DROP TABLE t;
 --
 -- Try to SPLIT partition of another table.
-- 
2.39.5 (Apple Git-154)



  [application/octet-stream] v3-0004-Reject-degenerate-SPLIT-PARTITION-with-DEFAULT-pa.patch (7.4K, 5-v3-0004-Reject-degenerate-SPLIT-PARTITION-with-DEFAULT-pa.patch)
  download | inline diff:
From 866b82fdc77b6e5420b335588681f25acd20758c Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Mon, 18 May 2026 00:37:52 +0300
Subject: [PATCH v3 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 <[email protected]>
Reviewed-by: Alexander Korotkov <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
 src/backend/partitioning/partbounds.c         | 91 +++++++++++++++++++
 src/test/regress/expected/partition_split.out | 18 ++++
 src/test/regress/sql/partition_split.sql      | 16 ++++
 3 files changed, 125 insertions(+)

diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c
index 7d3580cbc10..f626dc019af 100644
--- a/src/backend/partitioning/partbounds.c
+++ b/src/backend/partitioning/partbounds.c
@@ -5700,6 +5700,87 @@ check_parent_values_in_new_partitions(Relation parent,
 	}
 }
 
+/*
+ * 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 their listdatums lengths match.
+ */
+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);
+
+		/*
+		 * Containment of the new partition's values within the split
+		 * partition was established by the per-partition validation; thus
+		 * equality of value sets reduces to equality of cardinality.
+		 */
+		if (list_length(new_spec->listdatums) !=
+			list_length(split_spec->listdatums))
+			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 +5952,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..7216bd9d4f9 100644
--- a/src/test/regress/expected/partition_split.out
+++ b/src/test/regress/expected/partition_split.out
@@ -1188,6 +1188,24 @@ 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 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..e7bbcc9f054 100644
--- a/src/test/regress/sql/partition_split.sql
+++ b/src/test/regress/sql/partition_split.sql
@@ -834,6 +834,22 @@ 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 the explicit partition bound cannot extend outside the split
 -- partition's bound when a DEFAULT partition is specified.
-- 
2.39.5 (Apple Git-154)



reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Fix SPLIT PARTITION bound-overlap bug and other improvements
  In-Reply-To: <CAPpHfdsLnD3NGnPC32oeg4ZWWX441M=SfH0-7bG652G40TpEEQ@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox