| From: | Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com> |
|---|---|
| To: | John Naylor <johncnaylorls(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 13:06:37 |
| Message-ID: | CAJTYsWUa7D1cZCCKEh8MRvDh5vSTsoQ4fZ2EMYAuhYUA52Yazw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On Thu, 7 May 2026 at 17:59, John Naylor <johncnaylorls(at)gmail(dot)com> wrote:
> 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.
>
>
Thank you so much for the updates and help with this John!
Regards,
Ayush
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Andrew Dunstan | 2026-05-07 13:35:29 | Re: Postgresql 18 Linux (all flavors) - with installation, create new instance/main database |
| Previous Message | Laurenz Albe | 2026-05-07 13:05:29 | Re: Wrong results with equality search using trigram index and non-deterministic collation |