On Tue, Apr 21, 2026 at 5:37 PM Ayush Tiwari <ayushtiwari.slg01@gmail.com> wrote: > > Reattaching patch with right format for cfbot. >
I am not so sure these errhint are redundant, maybe the errdeatil is redundant. I am ok with: + errmsg("cannot merge non-adjacent partitions \"%s\" and \"%s\"", second_name->relname, first_name->relname), + errhint("ALTER TABLE ... MERGE PARTITIONS requires the partition bounds to be adjacent."),
I see your point, but I'd lean toward keeping errdetail over errhint here. The errdetail tells the user *which* specific bounds don't match ("lower bound of partition B is not equal to upper bound of partition A"), which is useful when merging 3+ partitions — the user can identify the exact problem pair. The errhint ("bounds must be adjacent") mostly restates what the errmsg ("non-adjacent") already says.
That said, I don't feel strongly about it. I could also keep both (errdetail + errhint) if you think that's better, though it does get verbose. What do you think?
in partition_split.sql, partition_merge.sql, I agree with that.sql file has "-- ERROR:" comments that are exact copies of the error message, is not great. But you suddenly delete all these comments seems not good.
We can add the `-- ERROR` comment suffix, as used in contrib/file_fdw/sql/file_fdw.sql or group them and add a comment like ``-- none of the following should be accepted``, as seen in src/test/regress/sql/arrays.sql.
Good point, removing them all without replacement does make the tests
harder to skim. I'll update v3 to use short markers like "-- should fail" instead of the exact error text, similar to what file_fdw.sql does. That way there's still a visual signal that the statement is expected to error, without the maintenance burden of duplicating the exact message.
