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
