Thanks Álvaro, Tom and Nathan for the review.
On Mon, 2 Feb 2026 at 16:46, Álvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
>
> > +void
> > +reject_newline_in_name(const char *objname, const char *objtype)
> > +{
> > + /* Report error if name has \n or \r character. */
> > + if (strpbrk(objname, "\n\r"))
> > + ereport(ERROR,
> > + (errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
> > + errmsg("\"%s\" name \"%s\" contains a newline or carriage return character",objtype,
objname));
> > +}
>
> I think this error message doesn't work very well. Having the
> "database" word be an untranslatable piece of the message makes no sense
> to me. I would rather have the ereport() in each place where this is
> needed so that we can have a complete phrase to translate, and avoid
> this wrinkle. Alternatively you could pass the error message from the
> caller (to abstract away the strpbrk() call) but I'm not sure that's
> really all that useful.
Fixed.
>
> BTW, looking at generate_db in src/bin/pg_upgrade/t/002_pg_upgrade.pl I
> wonder why don't we reject BEL here also. (Or actually, maybe not, but
> I think commit 322becb6085c was wrong to lose the part of the comment
> that explained the reason.)
>
> > --- a/src/bin/scripts/t/020_createdb.pl
> > +++ b/src/bin/scripts/t/020_createdb.pl
> > @@ -241,6 +241,18 @@ $node->command_fails(
> > ],
> > 'fails for invalid locale provider');
> >
> > +$node->command_fails_like(
> > + [ 'createdb', "invalid \n dbname" ],
> > + qr(contains a newline or carriage return character),
> > + 'fails if database name containing newline character in name'
> > +);
> > +
> > +$node->command_fails_like(
> > + [ 'createdb', "invalid \r dbname" ],
> > + qr(contains a newline or carriage return character),,
> > + 'fails if database name containing carriage return character in name'
> > +);
>
> Note there are two commas the the qr() line in the second stanza. Seems
> to be innocuous, because the test log shows
Fixed.
>
> # Running: createdb invalid
> dbname
> [11:57:00.942](0.012s) ok 34 - fails if database name containing newline character in name: exit code not 0
> [11:57:00.942](0.000s) ok 35 - fails if database name containing newline character in name: matches
> # Running: createdb invalid ^M dbname
> [11:57:00.953](0.011s) ok 36 - fails if database name containing carriage return character in name: exit code not 0
> [11:57:00.954](0.000s) ok 37 - fails if database name containing carriage return character in name: matches
>
> but it'd look nicer without those commas. Also, the "fails if ...
> containing" test names sound ungrammatical to me.
Fixed.
>
> --
> Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
> "Always assume the user will do much worse than the stupidest thing
> you can imagine." (Julien PUYDT)
Based on suggestions, I removed the new added function and changed the
error message and added a check for tablespace also.
Here, I am attaching updated patches.
--
Thanks and Regards
Mahendra Singh Thalor
EnterpriseDB: http://www.enterprisedb.com
