Re: [PATCH] Fix duplicate errmsg in ALTER TABLE SPLIT PARTITION

On Thu, Apr 23, 2026 at 11:04 AM Ayush Tiwari
<ayushtiwari(dot)slg01(at)gmail(dot)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(dot)universality(at)gmail(dot)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