[PATCH] Fix duplicate errmsg in ALTER TABLE SPLIT PARTITION

Started by Ayush Tiwari3 days ago12 messageshackers
Jump to latest
#1Ayush Tiwari
ayushtiwari.slg01@gmail.com

Hi hackers,

The ereport() in transformPartitionCmdForSplit() that fires when
splitting a non-default partition while a default partition already
exists has two errmsg() calls:

ereport(ERROR,
errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
errmsg("can not split non-DEFAULT partition \"%s\"", ...),
errmsg("new partition cannot be DEFAULT because ..."),
parser_errposition(...));

The second one should had been errdetail()

The attached patch changes the second errmsg() to errdetail().

Also, I think "can not" can be replaced with "cannot" for consistency
throughout? Havent added is as part of this patch though. Thoughts?

Regards,
Ayush

Attachments:

0001-Fix-duplicate-errmsg-in-ALTER-TABLE-SPLIT-PARTITION.patchapplication/octet-stream; name=0001-Fix-duplicate-errmsg-in-ALTER-TABLE-SPLIT-PARTITION.patchDownload+2-3
#2Yuchen Li
liyuchen_xyz@163.com
In reply to: Ayush Tiwari (#1)
Re: [PATCH] Fix duplicate errmsg in ALTER TABLE SPLIT PARTITION

On 4/21/2026 12:34 PM, Ayush Tiwari wrote:

Hi hackers,

The ereport() in transformPartitionCmdForSplit() that fires when
splitting a non-default partition while a default partition already
exists has two errmsg() calls:

    ereport(ERROR,
            errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
            errmsg("can not split non-DEFAULT partition \"%s\"", ...),
            errmsg("new partition cannot be DEFAULT because ..."),
            parser_errposition(...));

The second one should had been errdetail()

The attached patch changes the second errmsg() to errdetail().

Also, I think "can not" can be replaced with "cannot" for consistency
throughout? Havent added is as part of this patch though. Thoughts?

Regards,
Ayush

Good catch. I agree the second one should be a detail. The first letter
has been capitalized, and a period is added. So, the patch looks good to me.

Regards,
Yuchen Li

#3John Naylor
john.naylor@enterprisedb.com
In reply to: Ayush Tiwari (#1)
Re: [PATCH] Fix duplicate errmsg in ALTER TABLE SPLIT PARTITION

On Tue, Apr 21, 2026 at 11:34 AM Ayush Tiwari
<ayushtiwari.slg01@gmail.com> wrote:

Hi hackers,

The ereport() in transformPartitionCmdForSplit() that fires when
splitting a non-default partition while a default partition already
exists has two errmsg() calls:

ereport(ERROR,
errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
errmsg("can not split non-DEFAULT partition \"%s\"", ...),
errmsg("new partition cannot be DEFAULT because ..."),
parser_errposition(...));

The second one should had been errdetail()

Seems right.

Also, I think "can not" can be replaced with "cannot" for consistency
throughout? Havent added is as part of this patch though. Thoughts?

Not just consistency, the first spelling is simply wrong, and this
isn't the only place.

Taking a brief look at the test files added for this feature

src/test/regress/sql/partition_merge.sql/.out
src/test/regress/sql/partition_split.sql/.out

...I noticed that the .sql file has "-- ERROR:" comments that are
exact copies of the error message, which can't be great for
maintenance. We don't seem to do that anywhere else, so I'm not sure
what the motivation was.

Case in point, I noticed at least one other grammatical error in a message:

$ git grep 'DEFAULT partition should be one'
src/backend/parser/parse_utilcmd.c: errmsg("DEFAULT partition
should be one"),
src/backend/po/de.po:msgid "DEFAULT partition should be one"
src/test/regress/expected/partition_split.out:-- ERROR DEFAULT
partition should be one
src/test/regress/expected/partition_split.out:ERROR: DEFAULT
partition should be one
src/test/regress/sql/partition_split.sql:-- ERROR DEFAULT partition
should be one

That's makes four places that will need to be changed, instead of two,
and it's easy to overlook the comments.

While I'm looking,

ALTER TABLE sales_range SPLIT PARTITION sales_others INTO
(PARTITION sales_dec2021 FOR VALUES FROM ('2021-12-01') TO ('2022-01-01'),
PARTITION sales_error FOR VALUES FROM ('2021-12-30') TO ('2022-02-01'),
PARTITION sales_feb2022 FOR VALUES FROM ('2022-02-01') TO ('2022-03-01'),
PARTITION sales_others DEFAULT);

ERROR: can not split to partition "sales_error" together with
partition "sales_dec2021"

This seems like it should be
ERROR: cannot split partition sales_others
DETAIL: partition "sales_error" overlaps with partition "sales_dec2021"

...or something like that, maybe others have a better idea, but I find
the current message confusing.

In short, I think there is a lot more here that needs attention.

--
John Naylor
Amazon Web Services

#4Ayush Tiwari
ayushtiwari.slg01@gmail.com
In reply to: John Naylor (#3)
Re: [PATCH] Fix duplicate errmsg in ALTER TABLE SPLIT PARTITION

Hi,

Thanks for the detailed review.

On Tue, 21 Apr 2026 at 12:15, John Naylor <johncnaylorls@gmail.com> wrote:

On Tue, Apr 21, 2026 at 11:34 AM Ayush Tiwari
<ayushtiwari.slg01@gmail.com> wrote:

Also, I think "can not" can be replaced with "cannot" for consistency
throughout? Havent added is as part of this patch though. Thoughts?

Not just consistency, the first spelling is simply wrong, and this
isn't the only place.

Agreed. I've attached a v2 draft patch fixes all "can not" -> "cannot"
occurrences across
the split/merge partition code:

- parse_utilcmd.c: "can not split DEFAULT partition" and
"can not split non-DEFAULT partition"
- partbounds.c: "can not merge partition ... together with ..." and
"can not split to partition ... together with ..."
- tablecmds.c: "can not find partition for split partition row"

Taking a brief look at the test files added for this feature

src/test/regress/sql/partition_merge.sql/.out
src/test/regress/sql/partition_split.sql/.out

...I noticed that the .sql file has "-- ERROR:" comments that are
exact copies of the error message, which can't be great for
maintenance. We don't seem to do that anywhere else, so I'm not sure
what the motivation was.

Good point. It make sense to remove "-- ERROR:", "-- DETAIL:", and
"-- HINT:" comment lines from both partition_split.sql and
partition_merge.sql (and their corresponding .out files). I've kept the
Descriptive
comments like "-- (space between sections ...)" and
"-- sales_error intersects with ..." . Incorporated this too in patch.

Case in point, I noticed at least one other grammatical error in a message:

$ git grep 'DEFAULT partition should be one'
src/backend/parser/parse_utilcmd.c: errmsg("DEFAULT partition
should be one"),
src/backend/po/de.po:msgid "DEFAULT partition should be one"
src/test/regress/expected/partition_split.out:-- ERROR DEFAULT
partition should be one
src/test/regress/expected/partition_split.out:ERROR: DEFAULT
partition should be one
src/test/regress/sql/partition_split.sql:-- ERROR DEFAULT partition
should be one

Fixed: "DEFAULT partition should be one" is now
"cannot specify more than one DEFAULT partition".

That's makes four places that will need to be changed, instead of two,
and it's easy to overlook the comments.

While I'm looking,

ALTER TABLE sales_range SPLIT PARTITION sales_others INTO
(PARTITION sales_dec2021 FOR VALUES FROM ('2021-12-01') TO
('2022-01-01'),
PARTITION sales_error FOR VALUES FROM ('2021-12-30') TO ('2022-02-01'),
PARTITION sales_feb2022 FOR VALUES FROM ('2022-02-01') TO
('2022-03-01'),
PARTITION sales_others DEFAULT);

ERROR: can not split to partition "sales_error" together with
partition "sales_dec2021"

This seems like it should be
ERROR: cannot split partition sales_others
DETAIL: partition "sales_error" overlaps with partition "sales_dec2021"

...or something like that, maybe others have a better idea, but I find
the current message confusing.

In short, I think there is a lot more here that needs attention.

Hmm, I changed the confusing "can not split to partition X
together with partition Y" messages (for both split and merge) to:

errmsg: "cannot split non-adjacent partitions \"%s\" and \"%s\""
errdetail: "Lower bound of partition \"%s\" is not equal to upper
bound of partition \"%s\"."

errmsg: "cannot merge non-adjacent partitions \"%s\" and \"%s\""
errdetail: "Lower bound of partition \"%s\" is not equal to upper
bound of partition \"%s\"."

I also removed the
now-redundant errhint lines ("ALTER TABLE ... SPLIT/MERGE PARTITION
requires the partition bounds to be adjacent.") since
the errmsg itself says "non-adjacent".

Additionally, the errhint for splitting a DEFAULT partition had a
grammar error: "To split DEFAULT partition one of the new partition
must be DEFAULT." is now "To split a DEFAULT partition, one of the new
partitions must be DEFAULT."

And of course the original fix: the duplicate errmsg() -> errdetail()
in transformPartitionCmdForSplit() with proper capitalization and
trailing period.

Regards,
Ayush

Attachments:

v2-0001-Fix-duplicate-errmsg-in-ALTER-TABLE-SPLIT-PARTITION.patchapplication/octet-stream; name=v2-0001-Fix-duplicate-errmsg-in-ALTER-TABLE-SPLIT-PARTITION.patchDownload+30-38
#5Ayush Tiwari
ayushtiwari.slg01@gmail.com
In reply to: Ayush Tiwari (#4)
Re: [PATCH] Fix duplicate errmsg in ALTER TABLE SPLIT PARTITION

On Tue, 21 Apr 2026 at 14:21, Ayush Tiwari <ayushtiwari.slg01@gmail.com>
wrote:

Hi,

Thanks for the detailed review.

On Tue, 21 Apr 2026 at 12:15, John Naylor <johncnaylorls@gmail.com> wrote:

On Tue, Apr 21, 2026 at 11:34 AM Ayush Tiwari
<ayushtiwari.slg01@gmail.com> wrote:

Also, I think "can not" can be replaced with "cannot" for consistency
throughout? Havent added is as part of this patch though. Thoughts?

Not just consistency, the first spelling is simply wrong, and this
isn't the only place.

Agreed. I've attached a v2 draft patch fixes all "can not" -> "cannot"
occurrences across
the split/merge partition code:

- parse_utilcmd.c: "can not split DEFAULT partition" and
"can not split non-DEFAULT partition"
- partbounds.c: "can not merge partition ... together with ..." and
"can not split to partition ... together with ..."
- tablecmds.c: "can not find partition for split partition row"

Taking a brief look at the test files added for this feature

src/test/regress/sql/partition_merge.sql/.out
src/test/regress/sql/partition_split.sql/.out

...I noticed that the .sql file has "-- ERROR:" comments that are
exact copies of the error message, which can't be great for
maintenance. We don't seem to do that anywhere else, so I'm not sure
what the motivation was.

Good point. It make sense to remove "-- ERROR:", "-- DETAIL:", and
"-- HINT:" comment lines from both partition_split.sql and
partition_merge.sql (and their corresponding .out files). I've kept the
Descriptive
comments like "-- (space between sections ...)" and
"-- sales_error intersects with ..." . Incorporated this too in patch.

Case in point, I noticed at least one other grammatical error in a
message:

$ git grep 'DEFAULT partition should be one'
src/backend/parser/parse_utilcmd.c: errmsg("DEFAULT partition
should be one"),
src/backend/po/de.po:msgid "DEFAULT partition should be one"
src/test/regress/expected/partition_split.out:-- ERROR DEFAULT
partition should be one
src/test/regress/expected/partition_split.out:ERROR: DEFAULT
partition should be one
src/test/regress/sql/partition_split.sql:-- ERROR DEFAULT partition
should be one

Fixed: "DEFAULT partition should be one" is now
"cannot specify more than one DEFAULT partition".

That's makes four places that will need to be changed, instead of two,
and it's easy to overlook the comments.

While I'm looking,

ALTER TABLE sales_range SPLIT PARTITION sales_others INTO
(PARTITION sales_dec2021 FOR VALUES FROM ('2021-12-01') TO
('2022-01-01'),
PARTITION sales_error FOR VALUES FROM ('2021-12-30') TO ('2022-02-01'),
PARTITION sales_feb2022 FOR VALUES FROM ('2022-02-01') TO
('2022-03-01'),
PARTITION sales_others DEFAULT);

ERROR: can not split to partition "sales_error" together with
partition "sales_dec2021"

This seems like it should be
ERROR: cannot split partition sales_others
DETAIL: partition "sales_error" overlaps with partition "sales_dec2021"

...or something like that, maybe others have a better idea, but I find
the current message confusing.

In short, I think there is a lot more here that needs attention.

Hmm, I changed the confusing "can not split to partition X
together with partition Y" messages (for both split and merge) to:

errmsg: "cannot split non-adjacent partitions \"%s\" and \"%s\""
errdetail: "Lower bound of partition \"%s\" is not equal to upper
bound of partition \"%s\"."

errmsg: "cannot merge non-adjacent partitions \"%s\" and \"%s\""
errdetail: "Lower bound of partition \"%s\" is not equal to upper
bound of partition \"%s\"."

I also removed the
now-redundant errhint lines ("ALTER TABLE ... SPLIT/MERGE PARTITION
requires the partition bounds to be adjacent.") since
the errmsg itself says "non-adjacent".

Additionally, the errhint for splitting a DEFAULT partition had a
grammar error: "To split DEFAULT partition one of the new partition
must be DEFAULT." is now "To split a DEFAULT partition, one of the new
partitions must be DEFAULT."

And of course the original fix: the duplicate errmsg() -> errdetail()
in transformPartitionCmdForSplit() with proper capitalization and
trailing period.

Reattaching patch with right format for cfbot.

Regards,
Ayush

Attachments:

v2-0001-Fix-errmsg-issues-in-ALTER-TABLE-SPLIT-MERGE-PARTITION.patchapplication/octet-stream; name=v2-0001-Fix-errmsg-issues-in-ALTER-TABLE-SPLIT-MERGE-PARTITION.patchDownload+30-39
#6jian he
jian.universality@gmail.com
In reply to: Ayush Tiwari (#5)
Re: [PATCH] Fix duplicate errmsg in ALTER TABLE SPLIT PARTITION

On Tue, Apr 21, 2026 at 5:37 PM Ayush Tiwari
<ayushtiwari.slg01@gmail.com> wrote:

Reattaching patch with right format for cfbot.

--- a/src/backend/partitioning/partbounds.c
+++ b/src/backend/partitioning/partbounds.c
@@ -5030,20 +5030,18 @@ check_two_partitions_bounds_range(Relation parent,
  if (is_merge)
  ereport(ERROR,
  errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("can not merge partition \"%s\" together with partition \"%s\"",
+ errmsg("cannot merge non-adjacent partitions \"%s\" and \"%s\"",
    second_name->relname, first_name->relname),
- errdetail("lower bound of partition \"%s\" is not equal to the upper
bound of partition \"%s\"",
+ errdetail("Lower bound of partition \"%s\" is not equal to upper
bound of partition \"%s\".",
   second_name->relname, first_name->relname),
- errhint("ALTER TABLE ... MERGE PARTITIONS requires the partition
bounds to be adjacent."),

- errhint("ALTER TABLE ... SPLIT PARTITION requires the partition
bounds to be adjacent."),

I am not so sure these errhint are redundant, maybe the errdeatil is redundant.
I am ok with:
+ errmsg("cannot merge non-adjacent partitions \"%s\" and \"%s\"",
    second_name->relname, first_name->relname),
+ errhint("ALTER TABLE ... MERGE PARTITIONS requires the partition
bounds to be adjacent."),

in partition_split.sql, partition_merge.sql,
I agree with that.sql file has "-- ERROR:" comments that are
exact copies of the error message, is not great. But you suddenly delete all
these comments seems not good.

We can add the `-- ERROR` comment suffix, as used in
contrib/file_fdw/sql/file_fdw.sql
or group them and add a comment like ``-- none of the following should
be accepted``,
as seen in src/test/regress/sql/arrays.sql.

--
jian
https://www.enterprisedb.com/

#7Ayush Tiwari
ayushtiwari.slg01@gmail.com
In reply to: jian he (#6)
Re: [PATCH] Fix duplicate errmsg in ALTER TABLE SPLIT PARTITION

Hi,

Thanks for taking a look at the patch.

On Wed, 22 Apr 2026 at 12:59, jian he <jian.universality@gmail.com> wrote:

On Tue, Apr 21, 2026 at 5:37 PM Ayush Tiwari
<ayushtiwari.slg01@gmail.com> wrote:

Reattaching patch with right format for cfbot.

I am not so sure these errhint are redundant, maybe the errdeatil is
redundant.
I am ok with:
+ errmsg("cannot merge non-adjacent partitions \"%s\" and \"%s\"",
second_name->relname, first_name->relname),
+ errhint("ALTER TABLE ... MERGE PARTITIONS requires the partition
bounds to be adjacent."),

I see your point, but I'd lean toward keeping errdetail over errhint
here. The errdetail tells the user *which* specific bounds don't match
("lower bound of partition B is not equal to upper bound of partition
A"), which is useful when merging 3+ partitions — the user can identify
the exact problem pair. The errhint ("bounds must be adjacent") mostly
restates what the errmsg ("non-adjacent") already says.

That said, I don't feel strongly about it. I could also keep both
(errdetail + errhint) if you think that's better, though it does get
verbose. What do you think?

in partition_split.sql, partition_merge.sql,
I agree with that.sql file has "-- ERROR:" comments that are
exact copies of the error message, is not great. But you suddenly delete
all
these comments seems not good.

We can add the `-- ERROR` comment suffix, as used in
contrib/file_fdw/sql/file_fdw.sql
or group them and add a comment like ``-- none of the following should
be accepted``,
as seen in src/test/regress/sql/arrays.sql.

Good point, removing them all without replacement does make the tests
harder to skim. I'll update v3 to use short markers like "-- should
fail" instead of the exact error text, similar to what file_fdw.sql
does. That way there's still a visual signal that the statement is
expected to error, without the maintenance burden of duplicating the
exact message.

Regards,
Ayush

#8Ayush Tiwari
ayushtiwari.slg01@gmail.com
In reply to: Ayush Tiwari (#7)
Re: [PATCH] Fix duplicate errmsg in ALTER TABLE SPLIT PARTITION

On Wed, 22 Apr 2026 at 13:49, Ayush Tiwari <ayushtiwari.slg01@gmail.com>
wrote:

Hi,

Thanks for taking a look at the patch.

On Wed, 22 Apr 2026 at 12:59, jian he <jian.universality@gmail.com> wrote:

On Tue, Apr 21, 2026 at 5:37 PM Ayush Tiwari
<ayushtiwari.slg01@gmail.com> wrote:

Reattaching patch with right format for cfbot.

I am not so sure these errhint are redundant, maybe the errdeatil is
redundant.
I am ok with:
+ errmsg("cannot merge non-adjacent partitions \"%s\" and \"%s\"",
second_name->relname, first_name->relname),
+ errhint("ALTER TABLE ... MERGE PARTITIONS requires the partition
bounds to be adjacent."),

I see your point, but I'd lean toward keeping errdetail over errhint
here. The errdetail tells the user *which* specific bounds don't match
("lower bound of partition B is not equal to upper bound of partition
A"), which is useful when merging 3+ partitions — the user can identify
the exact problem pair. The errhint ("bounds must be adjacent") mostly
restates what the errmsg ("non-adjacent") already says.

That said, I don't feel strongly about it. I could also keep both
(errdetail + errhint) if you think that's better, though it does get
verbose. What do you think?

in partition_split.sql, partition_merge.sql,
I agree with that.sql file has "-- ERROR:" comments that are
exact copies of the error message, is not great. But you suddenly delete
all
these comments seems not good.

We can add the `-- ERROR` comment suffix, as used in
contrib/file_fdw/sql/file_fdw.sql
or group them and add a comment like ``-- none of the following should
be accepted``,
as seen in src/test/regress/sql/arrays.sql.

Good point, removing them all without replacement does make the tests
harder to skim. I'll update v3 to use short markers like "-- should
fail" instead of the exact error text, similar to what file_fdw.sql
does. That way there's still a visual signal that the statement is
expected to error, without the maintenance burden of duplicating the
exact message.

Attaching v3 patch.

Regards,
Ayush

Attachments:

v3-0001-Fix-errmsg-issues-in-ALTER-TABLE-SPLIT-MERGE-PARTITION.patchapplication/octet-stream; name=v3-0001-Fix-errmsg-issues-in-ALTER-TABLE-SPLIT-MERGE-PARTITION.patchDownload+139-38
#9jian he
jian.universality@gmail.com
In reply to: Ayush Tiwari (#8)
Re: [PATCH] Fix duplicate errmsg in ALTER TABLE SPLIT PARTITION

On Wed, Apr 22, 2026 at 7:06 PM Ayush Tiwari
<ayushtiwari.slg01@gmail.com> wrote:

Attaching v3 patch.

hi.

V3 looks good to me.
The error message already conveyed the failure reasoning.
Just adding comment `-- ERROR` to partition_merge.sql, partition_split.sql
is enough, I think.

- errdetail("lower bound of partition \"%s\" is not equal to the upper
bound of partition \"%s\"",
+ errdetail("Lower bound of partition \"%s\" is not equal to upper
bound of partition \"%s\".",
   second_name->relname, first_name->relname),
- errhint("ALTER TABLE ... MERGE PARTITIONS requires the partition
bounds to be adjacent."),

The errdetail already explicitly explains the failure reason, removing
the errhint should be fine, I think.
I noticed you removed "the", I'm not sure if that's okay since I am
not a native English speaker.

--
jian
https://www.enterprisedb.com/

#10Ayush Tiwari
ayushtiwari.slg01@gmail.com
In reply to: jian he (#9)
Re: [PATCH] Fix duplicate errmsg in ALTER TABLE SPLIT PARTITION

Hi,

Thanks for the review.

On Thu, 23 Apr 2026 at 08:28, jian he <jian.universality@gmail.com> wrote:

I noticed you removed "the", I'm not sure if that's okay since I am

not a native English speaker.

Regarding the removal of "the" from "the upper bound"; I dropped it
to match the style of the errmsg, which says "upper bound of partition"
without the article. Happy to restore it if someone prefers the
original phrasing.

Regards,
Ayush

#11John Naylor
john.naylor@enterprisedb.com
In reply to: Ayush Tiwari (#10)
Re: [PATCH] Fix duplicate errmsg in ALTER TABLE SPLIT PARTITION

On Thu, Apr 23, 2026 at 11:04 AM Ayush Tiwari
<ayushtiwari.slg01@gmail.com> wrote:

--- a/src/backend/partitioning/partbounds.c
+++ b/src/backend/partitioning/partbounds.c
@@ -5030,20 +5030,18 @@ check_two_partitions_bounds_range(Relation parent,
   if (is_merge)
    ereport(ERROR,
      errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-     errmsg("can not merge partition \"%s\" together with partition \"%s\"",
+     errmsg("cannot merge non-adjacent partitions \"%s\" and \"%s\"",
          second_name->relname, first_name->relname),
-     errdetail("lower bound of partition \"%s\" is not equal to the
upper bound of partition \"%s\"",
+     errdetail("Lower bound of partition \"%s\" is not equal to upper
bound of partition \"%s\".",
          second_name->relname, first_name->relname),
-     errhint("ALTER TABLE ... MERGE PARTITIONS requires the partition
bounds to be adjacent."),

I don't see anything wrong with the original errmsg (aside from the
spelling correction.) -- "merge X together with Y" is not wrong. Nor
the errhint -- it's somewhat redundant, but it's also general, while
the errdetail is specific.

On Thu, 23 Apr 2026 at 08:28, jian he <jian.universality@gmail.com> wrote:

I noticed you removed "the", I'm not sure if that's okay since I am
not a native English speaker.

Regarding the removal of "the" from "the upper bound"; I dropped it
to match the style of the errmsg, which says "upper bound of partition"
without the article. Happy to restore it if someone prefers the
original phrasing.

This is not an improvement to my ears. Omitting the article at the
beginning would be okay, since it can be found in technical/newspaper
style, but with two things the errdetail is a bit awkward without an
article for each thing.

   else
    ereport(ERROR,
      errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-     errmsg("can not split to partition \"%s\" together with partition \"%s\"",
+     errmsg("cannot split non-adjacent partitions \"%s\" and \"%s\"",
          second_name->relname, first_name->relname),

This new language is backwards.

I would just do

errmsg("cannot split partition \"%s\"",
get_rel_name(splitPartOid)),

...that way the errmsg's mention the old partition(s), whether the
action is splitting or merging.

-     errdetail("lower bound of partition \"%s\" is not equal to the
upper bound of partition \"%s\"",
+     errdetail("Lower bound of partition \"%s\" is not equal to upper
bound of partition \"%s\".",
          second_name->relname, first_name->relname),
-     errhint("ALTER TABLE ... SPLIT PARTITION requires the partition
bounds to be adjacent."),
      parser_errposition(pstate, datum->location));

Ditto here: Two articles for the errdetail, and the errhint is not a
problem. Although, perhaps it'd be better if the two errhints said
"old/new partition bounds", respectively, for clarity.

Also, this patch is getting big and unfocused. Let's split out the
removal of copied ERROR messages in the tests to a separate second
patch.

--
John Naylor
Amazon Web Services

#12Ayush Tiwari
ayushtiwari.slg01@gmail.com
In reply to: John Naylor (#11)
Re: [PATCH] Fix duplicate errmsg in ALTER TABLE SPLIT PARTITION

Hi,

On Thu, 23 Apr 2026 at 12:24, John Naylor <johncnaylorls@gmail.com> wrote:

On Thu, Apr 23, 2026 at 11:04 AM Ayush Tiwari
<ayushtiwari.slg01@gmail.com> wrote:

--- a/src/backend/partitioning/partbounds.c
+++ b/src/backend/partitioning/partbounds.c
@@ -5030,20 +5030,18 @@ check_two_partitions_bounds_range(Relation parent,
if (is_merge)
ereport(ERROR,
errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-     errmsg("can not merge partition \"%s\" together with partition
\"%s\"",
+     errmsg("cannot merge non-adjacent partitions \"%s\" and \"%s\"",
second_name->relname, first_name->relname),
-     errdetail("lower bound of partition \"%s\" is not equal to the
upper bound of partition \"%s\"",
+     errdetail("Lower bound of partition \"%s\" is not equal to upper
bound of partition \"%s\".",
second_name->relname, first_name->relname),
-     errhint("ALTER TABLE ... MERGE PARTITIONS requires the partition
bounds to be adjacent."),

I don't see anything wrong with the original errmsg (aside from the
spelling correction.) -- "merge X together with Y" is not wrong. Nor
the errhint -- it's somewhat redundant, but it's also general, while
the errdetail is specific.

Makes sense.

On Thu, 23 Apr 2026 at 08:28, jian he <jian.universality@gmail.com>

wrote:

I noticed you removed "the", I'm not sure if that's okay since I am
not a native English speaker.

Regarding the removal of "the" from "the upper bound"; I dropped it
to match the style of the errmsg, which says "upper bound of partition"
without the article. Happy to restore it if someone prefers the
original phrasing.

This is not an improvement to my ears. Omitting the article at the
beginning would be okay, since it can be found in technical/newspaper
style, but with two things the errdetail is a bit awkward without an
article for each thing.

else
ereport(ERROR,
errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-     errmsg("can not split to partition \"%s\" together with partition
\"%s\"",
+     errmsg("cannot split non-adjacent partitions \"%s\" and \"%s\"",
second_name->relname, first_name->relname),

This new language is backwards.

I would just do

errmsg("cannot split partition \"%s\"",
get_rel_name(splitPartOid)),

...that way the errmsg's mention the old partition(s), whether the
action is splitting or merging.

-     errdetail("lower bound of partition \"%s\" is not equal to the
upper bound of partition \"%s\"",
+     errdetail("Lower bound of partition \"%s\" is not equal to upper
bound of partition \"%s\".",
second_name->relname, first_name->relname),
-     errhint("ALTER TABLE ... SPLIT PARTITION requires the partition
bounds to be adjacent."),
parser_errposition(pstate, datum->location));

Ditto here: Two articles for the errdetail, and the errhint is not a
problem. Although, perhaps it'd be better if the two errhints said
"old/new partition bounds", respectively, for clarity.

Also, this patch is getting big and unfocused. Let's split out the
removal of copied ERROR messages in the tests to a separate second
patch.

Attaching separate patches one with edits, and the other for ERRORs.
Please review and let me know.

Regards,
Ayush

Attachments:

v4-0002-Simplify-error-comments-in-partition-split-merge-tests.patchapplication/octet-stream; name=v4-0002-Simplify-error-comments-in-partition-split-merge-tests.patchDownload+110-1
v4-0001-Fix-errmsg-issues-in-ALTER-TABLE-SPLIT-MERGE-PARTITION.patchapplication/octet-stream; name=v4-0001-Fix-errmsg-issues-in-ALTER-TABLE-SPLIT-MERGE-PARTITION.patchDownload+29-29