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

From: John Naylor <johncnaylorls(at)gmail(dot)com>
To: Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Daniel Gustafsson <daniel(at)yesql(dot)se>
Subject: Re: [PATCH] Fix duplicate errmsg in ALTER TABLE SPLIT PARTITION
Date: 2026-04-21 06:45:46
Message-ID: CANWCAZap26BRLwtd+A7GFDSD6-+C3F0NVdUGUAu2LUfvpOTy=w@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Smith 2026-04-21 06:45:58 Re: EXCEPT TABLE - Case inconsistency for describe \d and \dRp+
Previous Message vignesh C 2026-04-21 06:38:24 Re: xact_rollback spikes when logical walsender exits