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
v6-0001-Fix-errmsg-issues-in-ALTER-TABLE-SPLIT-MERGE-PARTITION.patch
Description: Binary data
v6-0002-Simplify-error-comments-in-partition-split-merge-tests.patch
Description: Binary data
