From: | Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> |
---|---|
To: | Jeff Davis <pgsql(at)j-davis(dot)com>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Refactor to introduce pg_strcoll(). |
Date: | 2022-11-01 12:36:23 |
Message-ID: | 850cd82e-2077-9c5e-8ae4-0584d143450f@enterprisedb.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 15.10.22 01:00, Jeff Davis wrote:
> On Thu, 2022-10-13 at 10:57 +0200, Peter Eisentraut wrote:
>> It's a bit confusing that arguments must be NUL-terminated, but the
>> length is still specified. Maybe another sentence to explain that
>> would
>> be helpful.
>
> Added a comment. It was a little frustrating to get a perfectly clean
> API, because the callers do some buffer manipulation and optimizations
> of their own. I think this is an improvement, but suggestions welcome.
>
> If win32 is used with UTF-8 and wcscoll, it ends up allocating some
> extra stack space for the temporary buffers, whereas previously it used
> the buffers on the stack of varstr_cmp(). I'm not sure if that's a
> problem or not.
Refactoring the common code from varstr_cmp() and varstrfastcmp_locale()
looks like a clear win.
But I think putting the Windows UTF-8 code (win32_utf8_wcscoll()) from
varstr_cmp() into pg_strcoll() might create future complications.
Normally, it would be up to varstr_sortsupport() to set up a
Windows/UTF-8 specific comparator function, but it says there it's not
implemented. But someone who wanted to implement that would have to
lift it back out of pg_strcoll, or rearrange the code in some other way.
It's not a clear win, I think. Perhaps you have some follow-up work
planned, in which case it might be better to consider this in more
context. Otherwise, I'd be tempted to leave it where it was in
varstr_cmp(). (It could still be a separate function, to keep
varstr_cmp() itself a bit smaller.)
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2022-11-01 12:37:39 | Re: heavily contended lwlocks with long wait queues scale badly |
Previous Message | thomas | 2022-11-01 12:30:06 | Re: [PATCH] Add `verify-system` sslmode to use system CA pool for server cert |