Re: Rework of collation code, extensibility - Mailing list pgsql-hackers

From Ted Yu
Subject Re: Rework of collation code, extensibility
Date
Msg-id CALte62xnbakD-Qs+48GM3BF5P1rPwoERq2G0ctsp9Xzo+uC7-A@mail.gmail.com
Whole thread Raw
In response to Rework of collation code, extensibility  (Jeff Davis <pgsql@j-davis.com>)
Responses Re: Rework of collation code, extensibility  (John Naylor <john.naylor@enterprisedb.com>)
List pgsql-hackers


On Sat, Dec 17, 2022 at 7:14 PM Jeff Davis <pgsql@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.camel@j-davis.com
[2]
https://postgr.es/m/c4fda90ec6a7568a896f243a38eb273c3b5c3d93.camel@j-davis.com
[3]
https://postgr.es/m/CA+hUKGLEqMhnpZrgAcisoUeYFGz8W6EWdhtK2h-4QN0iOSFRqw@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

pgsql-hackers by date:

Previous
From: Jeff Davis
Date:
Subject: Rework of collation code, extensibility
Next
From: John Naylor
Date:
Subject: Re: Rework of collation code, extensibility