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

On Tue, 21 Apr 2026 at 14:21, Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com>
wrote:

> 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.
>
>
Reattaching patch with right format for cfbot.

Regards,
Ayush