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(at)gmail(dot)com> wrote:

> On Tue, Apr 21, 2026 at 11:34 AM Ayush Tiwari
> <ayushtiwari(dot)slg01(at)gmail(dot)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