Re: Rework of collation code, extensibility
Peter Eisentraut <peter.eisentraut@enterprisedb.com>
From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
To: Jeff Davis <pgsql@j-davis.com>, pgsql-hackers@postgresql.org
Date: 2023-01-04T21:46:23Z
Lists: pgsql-hackers
On 22.12.22 06:40, Jeff Davis wrote: > On Sat, 2022-12-17 at 19:14 -0800, Jeff Davis 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. > > Here's version 5. There are a number of fixes, and better tests, and > it's passing in CI. > > The libc hook support is still experimental, but what's working is > passing in CI, even on windows. The challenges with libc hook support > are: > > * It obviously doesn't replace all of libc, so the separation is not > as clean and there are a number of callers throughout the code that > don't necessarily care about specific collations. > > * libc relies on setlocale() / uselocale(), which is global state and > not as easy to track. > > * More platform issues (obviously) and harder to test. I'm confused by this patch set. It combines some refactoring that was previously posted with partial support for multiple ICU libraries with partial support for some new hooks. Shouldn't those be three separate threads? I think the multiple ICU libraries already does have a separate thread; how does this relate to that work? I don't know what the hooks are supposed to be for? What other locale libraries are you thinking about using this way? How can we asses whether these interfaces are sufficient for that? The refactoring patches don't look convincing just by looking at the numbers: 3 files changed, 406 insertions(+), 247 deletions(-) 6 files changed, 481 insertions(+), 150 deletions(-) 12 files changed, 400 insertions(+), 323 deletions(-) My sense is this is trying to do too many things at once, and those things are each not fully developed yet.
Commits
-
Remove unnecessary #ifdef USE_ICU and branch.
- e0b3074e8944 16.0 landed
-
Refactor to introduce pg_locale_deterministic().
- 6974a8f7682b 16.0 landed
-
Refactor to add pg_strcoll(), pg_strxfrm(), and variants.
- d87d548cd030 16.0 landed
-
Fix MSVC build
- c971a5b27ac9 16.0 cited