Re: tiny step toward threading: reduce dependence on setlocale()

On Wed, 2024-08-14 at 01:31 +0200, Andreas Karlsson wrote:
> 0001-Remove-lc_collate_is_c.patch
>
> Removes lc_collate_is_c().

This could use some performance testing, as the commit message says,
otherwise it looks good.

> 0002-Remove-lc_ctype_is_c.patch
>
> Removes lc_ctype_is_c() and POSIX_COLLATION_OID which is no longer
> necessary.

This overlaps a bit with what Peter already proposed here:

https://www.postgresql.org/message-id/4f562d84-87f4-44dc-8946-01d6c437936f%40eisentraut.org

right?

> 0003-Remove-dubious-check-against-default-locale.patch
>
> This patch removes a check against DEFAULT_COLLATION_OID which I
> thought
> looked really dubious. Shouldn't this just be a simple check for if
> the
> locale is deterministic? Since we know we have a valid locale that
> should be enough, right?

Looks good to me. The code was correct in the sense that the default
collation is always deterministic, but (a) Peter is working on non-
deterministic default collations; and (b) it was a redundant check.

> 0004-Do-not-check-both-for-collate_is_c-and-deterministic.patch
>
> It is redundant to check both for "collation_is_c && deterministic",
> right?

+1.

> 0005-Remove-pg_collate_deterministic-and-check-field-dire.patch
>
> Since after my patches we look a lot directly at the collation_is_c
> and
> ctype_is_c fields I think the thin wrapper around the deterministic
> field makes it seem like there is more to it so I suggest that we
> should
> just remove it.

+1. When I added that, there was also a NULL check, but that was
removed so we might as well just read the field.

> 0006-Slightly-refactor-varstr_sortsupport-to-improve-read.patch
>
> Small refactor to make a hard to read function a bit easier to read.

Looks good to me.

Regards,
Jeff Davis