Hi, On Thu, 7 May 2026 at 17:59, John Naylor <[email protected]> wrote:
> On Tue, May 5, 2026 at 3:57 PM Ayush Tiwari <[email protected]> > 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
