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: jian he <jian(dot)universality(at)gmail(dot)com>, 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-05-07 12:29:19
Message-ID: CANWCAZbeOvr=9YAB9+eFTDn5JNmUpCXewEupuqHP1ozHUwaRhw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, May 5, 2026 at 3:57 PM Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com> wrote:
> v6 attached, addressing the remaining point.

I've pushed these with some changes:

Additional corrections:

"can only merge partitions don't have sub-partitions"
-> "that don't...".

"new partition \"%s\" cannot have this value because split partition
\"%s\" does not have"
-> "does not have it"

ERROR: cannot split DEFAULT partition "sales_others"
LINE 2: (PARTITION sales_dec2021 FOR VALUES FROM ('2021-12-01') TO...
^
HINT: To split a DEFAULT partition, one of the new partitions must be DEFAULT.

-> The caret above was pointing to a seemingly-random non-default partition.

"new partitions combined partition bounds..."
-> needed an apostrophe

> In 0001,
> check_two_partitions_bounds_range() no longer takes an is_merge
> argument. The merge call site passes InvalidOid, the split call site
> passes splitPartOid, and the helper derives a local is_merge value from
> that. I used OidIsValid(splitPartOid) rather than a NULL comparison,
> since splitPartOid is an Oid.

In the end, I decided to split this part out into the attached, and
have not committed it since the original wasn't really in error, just
sounded a bit off. I also found a couple other places that could use
wordsmithing as well, but it's not as clear-cut:

ERROR: new partition "sales_west" cannot have this value because
split partition "sales_all" does not have
LINE 2: ...st FOR VALUES IN ('Lisbon', 'New York', 'Madrid', 'Melbourne...
^
-> It seems weird to have "this value" in the errmsg. Sure, the caret
points to the right place, but the message seems better to state "new
partition X contains a value not found in split partition Y". Other
places in the split/merge code do quote values in messages, so maybe
we can here as well? Not sure if it matters much.

"new partition \"%s\" would overlap with another (not split) partition \"%s\"
-> "another (not split)" might be better as "an existing", but I'm
open to other opinions.

--
John Naylor
Amazon Web Services

Attachment Content-Type Size
v7-0001-Message-improvements.patch text/x-patch 10.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2026-05-07 12:36:08 Re: Include schema-qualified names in publication error messages.
Previous Message Euler Taveira 2026-05-07 12:27:09 Re: Include schema-qualified names in publication error messages.