Thanks Tom for the review. On Thu, 29 Jan 2026 at 21:54, Tom Lane <[email protected]> wrote: > > Andrew Dunstan <[email protected]> writes: > > These patches need a little copy editing (e.g. > > "check_database_role_names_in_old_cluser" seems to be missing a "t") and > > the error messages and comments need some tidying, but I think they are > > basically sound.
Thanks Andrew for the review. Fixed this typo. > > Is there any objection to them in principle? > > +1 in principle. As you say, there's some tidying needed. > A couple of points I noted: > > 1. check_lfcr_in_objname is about as unmusical a name as I can readily > imagine. I was thinking about proposing "reject_newline_in_name" > instead, but really I would drop that subroutine altogether and just > code the checks in-line, because: Fixed. As of now, I renamed it to reject_newline_in_name and changed the error message as per suggestion but I kept this function as previously suggested by some reviewers but I can remove this if this looks odd. > > 2. I don't think this approach to constructing the error message > meets our translatability guidelines. Better to just write out > "role name \"%s\" contains ..." or "database name \"%s\" contains > ...". We do use the other approach in some cases where it saves > dozens of repetitive messages, but when there are only ever going > to be two I'd rather err on the side of translatability. > Fixed. > 3. I do not like the tests added to 040_createuser.pl, as they > do not verify that the command fails for the expected reason. > Fixed. Added test case in .sql file to verify invalid names. > 4. There's no point in running check_database_role_names_in_old_cluser > against a v19 or later source server. Fixed. > > regards, tom lane Here, I am attaching updated patches for the review. -- Thanks and Regards Mahendra Singh Thalor EnterpriseDB: http://www.enterprisedb.com
v08_0001_don-t-allow-newline-or-carriage-return-in-db-user-role-names.patch
Description: Binary data
v08_0002-add-handling-to-pg_upgrade-to-report-alert-for-invalid-names.patch
Description: Binary data
