On Sat, Dec 17, 2022 at 7:14 PM Jeff Davis <pg...@j-davis.com> wrote:
> Attached is a new patch series. I think there are enough changes that > this has become more of a "rework" of the collation code rather than > just a refactoring. This is a continuation of some prior work[1][2] in > a new thread given its new scope. > > Benefits: > > 1. Clearer division of responsibilities. > 2. More consistent between libc and ICU providers. > 3. Hooks that allow extensions to replace collation provider libraries. > 4. New tests for the collation provider library hooks. > > There are a lot of changes, and still some loose ends, but I believe a > few of these patches are close to ready. > > This set of changes does not express an opinion on how we might want to > support multiple provider libraries in core; but whatever we choose, it > should be easier to accomplish. Right now, the hooks have limited > information on which to make the choice for a specific version of a > collation provider library, but that's because there's limited > information in the catalog. If the discussion here[3] concludes in > adding collation provider library or library version information to the > catalog, we can add additional parameters to the hooks. > > [1] > > https://postgr.es/m/99aa79cceefd1fe84fda23510494b8fbb7ad1e70.ca...@j-davis.com > [2] > > https://postgr.es/m/c4fda90ec6a7568a896f243a38eb273c3b5c3d93.ca...@j-davis.com > [3] > > https://postgr.es/m/ca+hukgleqmhnpzrgacisoueyfgz8w6ewdhtk2h-4qn0iosf...@mail.gmail.com > > > -- > Jeff Davis > PostgreSQL Contributor Team - AWS > > Hi, For pg_strxfrm_libc in v4-0002-Add-pg_strxfrm-and-pg_strxfrm_prefix.patch: +#ifdef HAVE_LOCALE_T + if (locale) + return strxfrm_l(dest, src, destsize, locale->info.lt); + else +#endif + return strxfrm(dest, src, destsize); It seems the `else` is not needed (since when the if branch is taken, we return from the func). + /* nul-terminate arguments */ nul-terminate -> null-terminate For pg_strnxfrm(), I think `result` can be removed - we directly return the result from pg_strnxfrm_libc or pg_strnxfrm_icu. Cheers