On 2025-Apr-06, Andrew Dunstan wrote:
> On 2025-03-28 Fr 10:43 AM, Nathan Bossart wrote:
> > Taking a step back, are we sure that 1) this is the right place to do these
> > checks and 2) we shouldn't apply the same restrictions to all names? I'm
> > wondering if it would be better to add these checks to the grammar instead
> > of trying to patch up all the various places they are used in the tree.
>
> Maybe. I don't think there is time for that for v18, so we'd have to defer
> this for now. I can live with that - it's been like this for a long time.
Grumble. I'd rather introduce a partial restriction now only for names
that affect the tools failing outright (pg_dumpall in particular), than
do nothing for this release. If we feel the need to extend the
restriction later, that's easy to do and bothers nobody. We've wanted
these characters to be forbidden on database names for a long time, but
nobody has cared as much for their use on other types of names. I'm not
even sure we'd support the idea of forbidding them on all names (though
the SQL standard doesn't allow control chars in identifiers.)
Another point is that we can easily have pg_upgrade check for invalid
database and role names, but checking *all* names would be more onerous.
I don't like the present implementation though, on translability
grounds. I think the error message should appear at each callsite
rather than be hardcoded in the new function, to avoid string building.
I think it'd be cleaner if the new function (maybe "name_contains_crlf"
or "is_identifier_awful") just returned a boolean based on strpbrk(),
and the callsite throws the error.
I wonder why does the patch restrict both database and role names. Does
a user with a newline also cause pg_upgrade to fail? I mean, this
thread started with a consideration for database names only, and the
usage on role names seemed to have appeared out of nowhere in [1].
Cheers
[1] https://postgr.es/m/CAKYtNAoVKQL5rLD4P4hZXZSnThwO-j4q3Y1vTDHQGjzwC-kUJg@mail.gmail.com
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Aprender sin pensar es inútil; pensar sin aprender, peligroso" (Confucio)
