Hi, Thanks for the detailed review.
On Tue, 21 Apr 2026 at 12:15, John Naylor <[email protected]> wrote: > On Tue, Apr 21, 2026 at 11:34 AM Ayush Tiwari > <[email protected]> wrote: > > > > Also, I think "can not" can be replaced with "cannot" for consistency > > throughout? Havent added is as part of this patch though. Thoughts? > > Not just consistency, the first spelling is simply wrong, and this > isn't the only place. > Agreed. I've attached a v2 draft patch fixes all "can not" -> "cannot" occurrences across the split/merge partition code: - parse_utilcmd.c: "can not split DEFAULT partition" and "can not split non-DEFAULT partition" - partbounds.c: "can not merge partition ... together with ..." and "can not split to partition ... together with ..." - tablecmds.c: "can not find partition for split partition row" > Taking a brief look at the test files added for this feature > > src/test/regress/sql/partition_merge.sql/.out > src/test/regress/sql/partition_split.sql/.out > > ...I noticed that the .sql file has "-- ERROR:" comments that are > exact copies of the error message, which can't be great for > maintenance. We don't seem to do that anywhere else, so I'm not sure > what the motivation was. > Good point. It make sense to remove "-- ERROR:", "-- DETAIL:", and "-- HINT:" comment lines from both partition_split.sql and partition_merge.sql (and their corresponding .out files). I've kept the Descriptive comments like "-- (space between sections ...)" and "-- sales_error intersects with ..." . Incorporated this too in patch. > > Case in point, I noticed at least one other grammatical error in a message: > > $ git grep 'DEFAULT partition should be one' > src/backend/parser/parse_utilcmd.c: errmsg("DEFAULT partition > should be one"), > src/backend/po/de.po:msgid "DEFAULT partition should be one" > src/test/regress/expected/partition_split.out:-- ERROR DEFAULT > partition should be one > src/test/regress/expected/partition_split.out:ERROR: DEFAULT > partition should be one > src/test/regress/sql/partition_split.sql:-- ERROR DEFAULT partition > should be one > Fixed: "DEFAULT partition should be one" is now "cannot specify more than one DEFAULT partition". > > That's makes four places that will need to be changed, instead of two, > and it's easy to overlook the comments. > > While I'm looking, > > ALTER TABLE sales_range SPLIT PARTITION sales_others INTO > (PARTITION sales_dec2021 FOR VALUES FROM ('2021-12-01') TO > ('2022-01-01'), > PARTITION sales_error FOR VALUES FROM ('2021-12-30') TO ('2022-02-01'), > PARTITION sales_feb2022 FOR VALUES FROM ('2022-02-01') TO > ('2022-03-01'), > PARTITION sales_others DEFAULT); > > ERROR: can not split to partition "sales_error" together with > partition "sales_dec2021" > > This seems like it should be > ERROR: cannot split partition sales_others > DETAIL: partition "sales_error" overlaps with partition "sales_dec2021" > > ...or something like that, maybe others have a better idea, but I find > the current message confusing. > > In short, I think there is a lot more here that needs attention. > > Hmm, I changed the confusing "can not split to partition X together with partition Y" messages (for both split and merge) to: errmsg: "cannot split non-adjacent partitions \"%s\" and \"%s\"" errdetail: "Lower bound of partition \"%s\" is not equal to upper bound of partition \"%s\"." errmsg: "cannot merge non-adjacent partitions \"%s\" and \"%s\"" errdetail: "Lower bound of partition \"%s\" is not equal to upper bound of partition \"%s\"." I also removed the now-redundant errhint lines ("ALTER TABLE ... SPLIT/MERGE PARTITION requires the partition bounds to be adjacent.") since the errmsg itself says "non-adjacent". Additionally, the errhint for splitting a DEFAULT partition had a grammar error: "To split DEFAULT partition one of the new partition must be DEFAULT." is now "To split a DEFAULT partition, one of the new partitions must be DEFAULT." And of course the original fix: the duplicate errmsg() -> errdetail() in transformPartitionCmdForSplit() with proper capitalization and trailing period. Regards, Ayush
v2-0001-Fix-duplicate-errmsg-in-ALTER-TABLE-SPLIT-PARTITION.patch
Description: Binary data
