Re: [HACKERS] ICU locales and text/char(n) SortSupport on Windows

2017-09-24 Thread Peter Geoghegan
On Sun, Sep 24, 2017 at 5:04 AM, Peter Eisentraut wrote: > On 9/22/17 12:25, Peter Geoghegan wrote: >> On Fri, Sep 22, 2017 at 7:25 AM, Peter Eisentraut >> wrote: >>> I agree. The attached patch should do it. >> >> I see one small issue here: You'll now need to set ssup->comparator >> back to NU

Re: [HACKERS] ICU locales and text/char(n) SortSupport on Windows

2017-09-24 Thread Peter Eisentraut
On 9/22/17 12:25, Peter Geoghegan wrote: > On Fri, Sep 22, 2017 at 7:25 AM, Peter Eisentraut > wrote: >> I agree. The attached patch should do it. > > I see one small issue here: You'll now need to set ssup->comparator > back to NULL before you return early in the Windows' libc case. That > way,

Re: [HACKERS] ICU locales and text/char(n) SortSupport on Windows

2017-09-22 Thread Peter Geoghegan
On Fri, Sep 22, 2017 at 7:25 AM, Peter Eisentraut wrote: > I agree. The attached patch should do it. I see one small issue here: You'll now need to set ssup->comparator back to NULL before you return early in the Windows' libc case. That way, a shim comparator (that goes through bttextcmp(), in

Re: [HACKERS] ICU locales and text/char(n) SortSupport on Windows

2017-09-22 Thread Peter Eisentraut
On 9/21/17 15:21, Peter Geoghegan wrote: > On Thu, Sep 21, 2017 at 12:12 PM, Peter Eisentraut > wrote: >>> Attached patch shows what I'm getting at. This is untested, since I >>> don't use Windows. Proceed with caution. >> >> Your analysis makes sense, but the patch doesn't work, because "locale"

Re: [HACKERS] ICU locales and text/char(n) SortSupport on Windows

2017-09-21 Thread Peter Geoghegan
On Wed, Sep 20, 2017 at 11:05 PM, Noah Misch wrote: > This is currently a v10 open item, but I think it doesn't qualify for that > treatment. It's merely an opportunity for optimization, albeit an > attractively-simple one. I have withdrawn this as an open item. I'm still hopeful that it can be

Re: [HACKERS] ICU locales and text/char(n) SortSupport on Windows

2017-09-21 Thread Peter Geoghegan
On Thu, Sep 21, 2017 at 12:12 PM, Peter Eisentraut wrote: >> Attached patch shows what I'm getting at. This is untested, since I >> don't use Windows. Proceed with caution. > > Your analysis makes sense, but the patch doesn't work, because "locale" > is never set before reading it. It was just fo

Re: [HACKERS] ICU locales and text/char(n) SortSupport on Windows

2017-09-21 Thread Peter Eisentraut
On 9/16/17 18:33, Peter Geoghegan wrote: > In summary, we're currently attaching the use of SortSupport to the > wrong thing. We're treating this UTF-16 business as something that > implies a broad OS/platform restriction, when in fact it should be > treated as implying a restriction for one partic

Re: [HACKERS] ICU locales and text/char(n) SortSupport on Windows

2017-09-21 Thread Peter Geoghegan
On Wed, Sep 20, 2017 at 11:05 PM, Noah Misch wrote: > This is currently a v10 open item, but I think it doesn't qualify for that > treatment. It's merely an opportunity for optimization, albeit an > attractively-simple one. Fair enough. This is clearly an omission that was made in 41839b7ab, th

Re: [HACKERS] ICU locales and text/char(n) SortSupport on Windows

2017-09-20 Thread Noah Misch
On Sat, Sep 16, 2017 at 03:33:53PM -0700, Peter Geoghegan wrote: > In summary, we're currently attaching the use of SortSupport to the > wrong thing. We're treating this UTF-16 business as something that > implies a broad OS/platform restriction, when in fact it should be > treated as implying a re

[HACKERS] ICU locales and text/char(n) SortSupport on Windows

2017-09-16 Thread Peter Geoghegan
varstr_sortsupport() only allows Windows to use SortSupport with a non-C-locale (when the server encoding happens to be UTF-8, which I assume is the common case). This is because we (quite reasonably) don't want to have to duplicate the ugly UTF-8 to UTF-16 conversion hack from varstr_cmp() for the