| 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-05 08:56:46 |
| Message-ID: | CAJTYsWWvnvEEnHfKXiqFvSyVjnrqJzbz0kXbWJkmZnne_8z2eQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On Tue, 5 May 2026 at 12:45, John Naylor <johncnaylorls(at)gmail(dot)com> wrote:
> On Wed, Apr 29, 2026 at 5:03 PM Ayush Tiwari
> <ayushtiwari(dot)slg01(at)gmail(dot)com> wrote:
> > - For SPLIT, switch the adjacency error to
> > errmsg("cannot split partition \"%s\"",
> > get_rel_name(splitPartOid)),
> > so it names the old partition, matching the merge wording style.
> > To make splitPartOid available there, I added an Oid splitPartOid
> > parameter to check_two_partitions_bounds_range() and pass
> > InvalidOid from the merge call site (where is_merge is true so
> > the parameter is unused).
>
> If we have splitPartOid, then the boolean is_merge is redundant and
> can be removed, right? To keep the intent clear we can add a local
> variable
>
> bool is_merge = (splitPartOid == NULL ? true : false);
>
> Other than that, both patches LGTM.
>
Thanks for reviewing.
v6 attached, addressing the remaining point. 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.
> If there are two errmsg's back-to-back, only the second one displays.
> Maybe some automated tooling can detect cases like this going forward?
>
I agree on this, I do not know much about Linters PG has, can check.
Regards,
Ayush
| Attachment | Content-Type | Size |
|---|---|---|
| v6-0001-Fix-errmsg-issues-in-ALTER-TABLE-SPLIT-MERGE-PARTITION.patch | application/octet-stream | 17.6 KB |
| v6-0002-Simplify-error-comments-in-partition-split-merge-tests.patch | application/octet-stream | 53.3 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Andrey Borodin | 2026-05-05 09:07:17 | Re: [PATCH] Fix ProcKill lock-group vs procLatch recycle race |
| Previous Message | Álvaro Herrera | 2026-05-05 08:27:57 | Re: [Bug]Vacuum full silently NULL out fast default columns |