On Fri, 2024-07-26 at 19:38 +0200, Andreas Karlsson wrote:
> Nice refactoring!
>
> Two small comments about CheckMyDatabase().
>
> - Shouldn't we look at the default_locale.ctype_is_c when setting
> database_ctype_is_c instead of doing a strcmp()? or maybe we should
> even
> remove the global variable and always look at the default_locale?
database_ctype_is_c refers to the LC_CTYPE environment of the database
-- pg_database.datctype. default_locale.ctype_is_c is the ctype of the
database's default collation.
Confusing, I know, but it matters for a few things that still depend on
the LC_CTYPE, such as tsearch and maybe a few extensions. See
f413941f41.
> - I think that the lookup of Anum_pg_database_datlocale could be done
> later in the code since it is not needed when we use a libc locale.
> E.g.
> as below.
Done, thank you.
> Also is there any reaosn you do not squash th 4th and the 6th patch?
Done. I had to rearrange the patch ordering a bit because prior to the
cache refactoring patch, it's unsafe to call
pg_newlocale_from_collation() without checking lc_collate_is_c() or
lc_ctype_is_c() first.
Regards,
Jeff Davis