public inbox for [email protected]
help / color / mirror / Atom feedFrom: Chao Li <[email protected]>
To: PostgreSQL-development <[email protected]>
Cc: Dmitry Koval <[email protected]>
Cc: Alexander Korotkov <[email protected]>
Subject: Fix SPLIT PARTITION bound-overlap bug and other improvements
Date: Wed, 13 May 2026 12:38:44 +0800
Message-ID: <[email protected]> (raw)
Hi,
While testing ALTER TABLE ... SPLIT PARTITION, I found a bug and a few behaviors and messages that seem worth improving.
0. A bound-overlap bug
I numbered this item as 0 because I found it after finishing items 1, 2, and 3. While doing a final verification before sending this email, I was surprised to find that the partitioned table ended up with two overlapping partitions.
Here is a simple repro:
```
evantest=# drop table t;
DROP TABLE
evantest=# CREATE TABLE t (i int) PARTITION BY RANGE(i);
CREATE TABLE
evantest=# CREATE TABLE p0a PARTITION OF t FOR VALUES FROM (0) TO (51);
CREATE TABLE
evantest=# CREATE TABLE p0b PARTITION OF t FOR VALUES FROM (51) TO (100);
CREATE TABLE
evantest=# \d+ t;
Partitioned table "public.t"
Column | Type | Collation | Nullable | Default | Storage | Compression | Stats target | Description
--------+---------+-----------+----------+---------+---------+-------------+--------------+-------------
i | integer | | | | plain | | |
Partition key: RANGE (i)
Partitions:
p0a FOR VALUES FROM (0) TO (51)
p0b FOR VALUES FROM (51) TO (100)
evantest=# ALTER TABLE t SPLIT PARTITION p0a INTO
evantest-# (PARTITION p0a FOR VALUES FROM (0) TO (53),
evantest(# PARTITION pdef DEFAULT);
ALTER TABLE
evantest=#
evantest=#
evantest=# \d+ t;
Partitioned table "public.t"
Column | Type | Collation | Nullable | Default | Storage | Compression | Stats target | Description
--------+---------+-----------+----------+---------+---------+-------------+--------------+-------------
i | integer | | | | plain | | |
Partition key: RANGE (i)
Partitions:
p0a FOR VALUES FROM (0) TO (53)
p0b FOR VALUES FROM (51) TO (100)
pdef DEFAULT
```
As shown above, p0a and p0b now overlap. I think this is a real bug.
The problem seems to be in check_partition_bounds_for_split_range(). If the only non-default replacement partition is both first and last, the function checks only the lower bound because it uses if (first) ... else .... It never checks the upper bound in that case.
1. The documentation about splitting with a DEFAULT partition is a bit unclear
Since SPLIT PARTITION allows one of the new partitions to be specified as DEFAULT, I wondered whether that new DEFAULT partition means the remaining part of the split partition's bound, or the partitioned table's global DEFAULT partition.
The current documentation says:
```
<para>
This form splits a single partition of the target table into new
partitions. Hash-partitioned target table is 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>.
If the partitioned table does not have a <literal>DEFAULT</literal>
partition, a <literal>DEFAULT</literal> partition can be defined as one
of the new partitions.
</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>
<replaceable class="parameter">partition_name1</replaceable>,
<replaceable class="parameter">partition_name2</replaceable>[, ...]
</literal> should be equal to 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).
</para>
```
From the first paragraph, it seems that the new DEFAULT partition is a table-level default partition. However, the second paragraph says that the combined bounds of the new partitions should be equal to the bounds of the split partition, which can make it look as if the new DEFAULT partition only covers the remaining part of the split partition's bound.
My tests show that the new DEFAULT partition is the partitioned table's global DEFAULT partition. So I think the documentation can be improved to make that clearer.
2. I found this hint message confusing:
```
evantest=# ALTER TABLE t SPLIT PARTITION p0a INTO (PARTITION p0a1 FOR VALUES FROM (0) TO (5), PARTITION p0a2 FOR VALUES FROM (6) to (51), PARTITION pdef DEFAULT);
ERROR: upper bound of partition "p0a2" is greater than upper bound of split partition "p0a"
LINE 1: ...0) TO (5), PARTITION p0a2 FOR VALUES FROM (6) to (51), PARTI...
^
HINT: ALTER TABLE ... SPLIT PARTITION require combined bounds of new partitions must exactly match the bound of the split partition.
```
In this command, one of the new explicit partition bounds exceeds the original partition bound, but the command also specifies a DEFAULT partition. In this case, the combined explicit bounds do not need to exactly match the original partition bound, they only need to stay within it. So the hint is not quite accurate for this case.
3. SPLIT PARTITION currently provides another way to add a DEFAULT partition:
```
evantest=# \d+ t;
Partitioned table "public.t"
Column | Type | Collation | Nullable | Default | Storage | Compression | Stats target | Description
--------+---------+-----------+----------+---------+---------+-------------+--------------+-------------
i | integer | | | | plain | | |
Partition key: RANGE (i)
Partitions:
p0a FOR VALUES FROM (0) TO (50)
p0b FOR VALUES FROM (51) TO (100)
p200 FOR VALUES FROM (200) TO (300)
evantest=# ALTER TABLE t SPLIT PARTITION p0a INTO (PARTITION p0a FOR VALUES FROM (0) TO (50), PARTITION pdef DEFAULT);
ALTER TABLE
```
Here, the new p0a partition has the same bound as the original p0a partition, so no real split happens. The command effectively only adds a new DEFAULT partition. However, it still goes through the full split-partition path: creating a new partition, moving data, attaching the new partition, and dropping the old partition.
Initially, I considered adding a fast path for this case so that it would only add the new DEFAULT partition. But after thinking about it more, I think it is better to reject this degenerate form instead. We already has direct ways to add a DEFAULT partition:
```
CREATE TABLE p PARTITION OF t DEFAULT;
or
CREATE TABLE p;
ALTER TABLE t ATTACH PARTITION p DEFAULT;
```
So I do not think SPLIT PARTITION needs to become another syntax for adding a DEFAULT partition when no actual split is performed. Accepting this form could also raise another question later: if this is allowed, why does the user have to repeat the original bound at all? Why not allow something like this?
```
ALTER TABLE t SPLIT PARTITION p0a INTO (PARTITION pdef DEFAULT);
```
That seems like an awkward direction.
The attached patch tries to address all these issues.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
[application/octet-stream] v1-0001-Fix-SPLIT-PARTITION-validation-with-DEFAULT.patch (12.9K, 2-v1-0001-Fix-SPLIT-PARTITION-validation-with-DEFAULT.patch)
download | inline diff:
From 4fc92276d96195c38aec4477bd14c00a514173df Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Wed, 13 May 2026 11:17:19 +0800
Subject: [PATCH v1] Fix SPLIT PARTITION validation with DEFAULT
When ALTER TABLE ... SPLIT PARTITION specified a DEFAULT partition, range
bound validation checked the lower bound for the first explicit partition
and the upper bound for non-first explicit partitions. As a result, when
there was only one explicit non-DEFAULT partition, its upper bound was not
checked. This could allow the new partition to extend beyond the split
partition's bound and overlap another existing partition.
Fix this by checking the upper bound whenever the explicit partition is
the last one, rather than only when it is not the first one.
While here, reject the degenerate form where a non-DEFAULT partition is
split into one non-DEFAULT partition with exactly the same bound plus a
DEFAULT partition. That form performs no real split and merely adds a
DEFAULT partition through the split-partition path, for which existing
commands should be used instead.
Also clarify the documentation for SPLIT PARTITION with DEFAULT, and
adjust the hint for explicit bounds that extend outside the split
partition when a DEFAULT partition is specified.
Author: Chao Li <[email protected]>
---
doc/src/sgml/ref/alter_table.sgml | 31 ++++----
src/backend/partitioning/partbounds.c | 78 +++++++++++++++++--
src/test/regress/expected/partition_split.out | 43 ++++++++++
src/test/regress/sql/partition_split.sql | 37 +++++++++
4 files changed, 171 insertions(+), 18 deletions(-)
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 1f9a456fd33..97a6d331776 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -1293,28 +1293,33 @@ 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.
- 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>.
- If the partitioned table does not have a <literal>DEFAULT</literal>
+ partitions. Hash-partitioned target table is 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>. If the
+ partitioned table does not have a <literal>DEFAULT</literal>
partition, a <literal>DEFAULT</literal> partition can be defined as one
of the new partitions.
</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 not extend outside 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>
diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c
index 9b4277a4987..6ca6d8b719f 100644
--- a/src/backend/partitioning/partbounds.c
+++ b/src/backend/partitioning/partbounds.c
@@ -36,6 +36,7 @@
#include "utils/datum.h"
#include "utils/fmgroids.h"
#include "utils/lsyscache.h"
+#include "utils/memutils.h"
#include "utils/partcache.h"
#include "utils/ruleutils.h"
#include "utils/snapmgr.h"
@@ -5415,11 +5416,11 @@ 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)));
}
- else
+
+ if (last)
{
PartitionRangeBound *split_upper;
@@ -5457,8 +5458,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)));
}
}
@@ -5701,6 +5701,70 @@ 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.
+ */
+static void
+check_split_partition_not_same_bound(Relation parent,
+ Oid splitPartOid,
+ SinglePartitionSpec **parts,
+ int nparts,
+ ParseState *pstate)
+{
+ PartitionKey key = RelationGetPartitionKey(parent);
+ PartitionBoundSpec *split_spec;
+ PartitionBoundSpec *new_specs[1];
+ PartitionBoundSpec *old_specs[1];
+ PartitionBoundInfo new_boundinfo;
+ PartitionBoundInfo old_boundinfo;
+ int *new_mapping;
+ int *old_mapping;
+ MemoryContext old_cxt;
+ MemoryContext tmp_cxt;
+ bool same_bound;
+
+ if (nparts != 1)
+ return;
+
+ tmp_cxt = AllocSetContextCreate(CurrentMemoryContext,
+ "split partition bound comparison",
+ ALLOCSET_SMALL_SIZES);
+ old_cxt = MemoryContextSwitchTo(tmp_cxt);
+
+ split_spec = get_partition_bound_spec(splitPartOid);
+
+ new_specs[0] = parts[0]->bound;
+ new_boundinfo = partition_bounds_create(new_specs, 1, key, &new_mapping);
+
+ old_specs[0] = split_spec;
+ old_boundinfo = partition_bounds_create(old_specs, 1, key, &old_mapping);
+
+ same_bound = partition_bounds_equal(key->partnatts, key->parttyplen,
+ key->parttypbyval,
+ new_boundinfo, old_boundinfo);
+
+ MemoryContextSwitchTo(old_cxt);
+ MemoryContextDelete(tmp_cxt);
+
+ if (!same_bound)
+ 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
*
@@ -5775,6 +5839,10 @@ check_partitions_for_split(Relation parent,
Assert(nparts == list_length(partlist) - 1);
}
+ if (!isSplitPartDefault && createDefaultPart)
+ check_split_partition_not_same_bound(parent, splitPartOid, new_parts,
+ nparts, pstate);
+
if (strategy == PARTITION_STRATEGY_RANGE)
{
PartitionRangeBound **lower_bounds;
diff --git a/src/test/regress/expected/partition_split.out b/src/test/regress/expected/partition_split.out
index 961b37953c8..5d7380e28bb 100644
--- a/src/test/regress/expected/partition_split.out
+++ b/src/test/regress/expected/partition_split.out
@@ -1188,6 +1188,49 @@ 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.
+-- ERROR
+ALTER TABLE t SPLIT PARTITION tp_0_50 INTO
+ (PARTITION tp_0_5 FOR VALUES FROM (0) TO (5),
+ PARTITION tp_6_51 FOR VALUES FROM (6) TO (51),
+ PARTITION tp_default DEFAULT);
+ERROR: upper bound of partition "tp_6_51" is greater than upper bound of split partition "tp_0_50"
+LINE 3: PARTITION tp_6_51 FOR VALUES FROM (6) TO (51),
+ ^
+HINT: Explicit partition bounds must be contained within the bounds of the split partition when a DEFAULT partition is specified.
+DROP TABLE t;
+--
+-- 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: 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.
--
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..a925dacd205 100644
--- a/src/test/regress/sql/partition_split.sql
+++ b/src/test/regress/sql/partition_split.sql
@@ -834,6 +834,43 @@ 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
+ALTER TABLE t SPLIT PARTITION tp_0_50 INTO
+ (PARTITION tp_0_5 FOR VALUES FROM (0) TO (5),
+ PARTITION tp_6_51 FOR VALUES FROM (6) TO (51),
+ 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.
+--
+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.50.1 (Apple Git-155)
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: <[email protected]>
* 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