Hi,

On Tue, 5 May 2026 at 12:45, John Naylor <[email protected]> wrote:

> On Wed, Apr 29, 2026 at 5:03 PM Ayush Tiwari
> <[email protected]> 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: v6-0001-Fix-errmsg-issues-in-ALTER-TABLE-SPLIT-MERGE-PARTITION.patch
Description: Binary data

Attachment: v6-0002-Simplify-error-comments-in-partition-split-merge-tests.patch
Description: Binary data

Reply via email to