Re: [HACKERS] ICU integration

On Thu, Mar 23, 2017 at 6:31 PM, Craig Ringer wrote: > Congratulations on getting this done. It's great work, and it'll make > a whole class of potential bugs and platform portability warts go away > if widely adopted. +1 I would like to see us rigorously define a

Re: [HACKERS] ICU integration

On 24 March 2017 at 04:54, Peter Eisentraut wrote: > Fixed. Congratulations on getting this done. It's great work, and it'll make a whole class of potential bugs and platform portability warts go away if widely adopted. -- Craig Ringer

Re: [HACKERS] ICU integration

On 3/23/17 16:45, Thomas Munro wrote: > On Fri, Mar 24, 2017 at 8:34 AM, Peter Eisentraut > wrote: >> Committed. > > On a macOS system using MacPorts, I have "icu" installed. MacPorts is > a package manager that installs stuff under /opt/local. I have >

Re: [HACKERS] ICU integration

On Fri, Mar 24, 2017 at 8:34 AM, Peter Eisentraut wrote: > Committed. On a macOS system using MacPorts, I have "icu" installed. MacPorts is a package manager that installs stuff under /opt/local. I have /opt/local/bin in $PATH so that pkg-config can be found.

Re: [HACKERS] ICU integration

On Thu, Mar 23, 2017 at 12:34 PM, Peter Eisentraut < peter.eisentr...@2ndquadrant.com> wrote: > On 3/23/17 05:34, Andreas Karlsson wrote: > > I am fine with this version of the patch. The issues I have with it, > > which I mentioned earlier in this thread, seem to be issues with ICU > > rather

Re: [HACKERS] ICU integration

On 3/23/17 05:34, Andreas Karlsson wrote: > I am fine with this version of the patch. The issues I have with it, > which I mentioned earlier in this thread, seem to be issues with ICU > rather than with this patch. For example there seems to be no way for > ICU to validate the syntax of the BCP

Re: [HACKERS] ICU integration

I am fine with this version of the patch. The issues I have with it, which I mentioned earlier in this thread, seem to be issues with ICU rather than with this patch. For example there seems to be no way for ICU to validate the syntax of the BCP 47 locales (or ICU's old format). But I think we

Re: [HACKERS] ICU integration

On 03/15/2017 05:33 PM, Peter Eisentraut wrote: Updated patch attached. Ok, I applied to patch again and ran the tests. I get a test failure on make check-world in the pg_dump tests but it can be fixed with the below. diff --git a/src/bin/pg_dump/t/002_pg_dump.pl

Re: [HACKERS] ICU integration

On 03/09/2017 10:13 PM, Peter Eisentraut wrote: - Naming of collations: Are we happy with the "de%icu" naming? I might have come up with that while reviewing the IPv6 zone index patch. ;-) An alternative might be "de$icu" for more Oracle vibe and avoiding the need for double quotes in some

Re: [HACKERS] ICU integration

On Mon, Feb 20, 2017 at 3:51 PM, Bruce Momjian wrote: > So we don't have any other cases where we warn about possible corruption > except this? I'm not sure that I understand the distinction you're making. > Also, I will go back to my previous concern, that while I like the

Re: [HACKERS] ICU integration

On Mon, Feb 20, 2017 at 03:29:07PM -0800, Peter Geoghegan wrote: > Marking all indexes as invalid would be an enormous overkill. We don't > even know for sure that the values the user has indexed happens to be > affected. In order for there to have been a bug in ICU in the first > place, the

Re: [HACKERS] ICU integration

On Mon, Feb 20, 2017 at 3:19 PM, Bruce Momjian wrote: > Well, the release notes are a clear-enough communication for a need to > reindex. I don't see a LOG message as similar. Don't we have other > cases where we have to warn administrators? We can mark the indexes as >

Re: [HACKERS] ICU integration

On Mon, Feb 20, 2017 at 02:54:22PM -0800, Peter Geoghegan wrote: > On Mon, Feb 20, 2017 at 2:38 PM, Bruce Momjian wrote: > > I can't think of any cases where we warn of possible corruption only in > > the server logs. > > It's just like any case where there was a bug in

Re: [HACKERS] ICU integration

On Mon, Feb 20, 2017 at 2:38 PM, Bruce Momjian wrote: > I can't think of any cases where we warn of possible corruption only in > the server logs. It's just like any case where there was a bug in Postgres that could have subtly broken index builds. We don't make the next point

Re: [HACKERS] ICU integration

On Wed, Feb 15, 2017 at 10:35:34PM -0800, Peter Geoghegan wrote: > On Wed, Feb 15, 2017 at 9:17 PM, Peter Eisentraut > wrote: > > Stable operating systems shouldn't do major library upgrades, so I feel > > pretty confident about this. > > There is one case where

Re: [HACKERS] ICU integration

On 2/16/17 01:13, Peter Geoghegan wrote: > On Wed, Feb 15, 2017 at 9:17 PM, Peter Eisentraut > wrote: >> I have changed it to use ucol_nextSortKeyPart() when appropriate. > > I think it would be fine if the second last argument was > "sizeof(Datum)", not

Re: [HACKERS] ICU integration

Peter Eisentraut wrote: > - Naming of collations: Are we happy with the "de%icu" naming? I might > have come up with that while reviewing the IPv6 zone index patch. ;-) > An alternative might be "de$icu" for more Oracle vibe and avoiding the > need for double quotes in some cases. (But we have

Re: [HACKERS] ICU integration

On Wed, Feb 15, 2017 at 9:17 PM, Peter Eisentraut wrote: >> Clearly when you upgrade your system from (say) Debian 8 to Debian 9 >> and the ICU major version changes we expect to have to REINDEX, but >> does anyone know if such data changes are ever pulled into

Re: [HACKERS] ICU integration

On Wed, Feb 15, 2017 at 9:17 PM, Peter Eisentraut wrote: > I have changed it to use ucol_nextSortKeyPart() when appropriate. I think it would be fine if the second last argument was "sizeof(Datum)", not "Min(sizeof(Datum), sss->buflen2)" here: > +

Re: [HACKERS] ICU integration

On 1/31/17 16:59, Thomas Munro wrote: > I assume (but haven't checked) that ucol_nextSortKeyPart accesses only > the start of the string via the UCharIterator passed in, unless you > have the rare reverse-accent-sort feature enabled (maybe used only in > fr_CA, it looks like it is required to scan

Re: [HACKERS] ICU integration

On Thu, Feb 9, 2017 at 7:58 AM, Peter Eisentraut wrote: > I think I have this sorted out. What I don't understand, however, is > why varstr_abbrev_convert() makes a point of looping until the result of > strxfrm() fits into the output buffer (buf2), when we only

Re: [HACKERS] ICU integration

On 1/24/17 12:45 PM, Peter Eisentraut wrote: > On 1/9/17 3:45 PM, Peter Geoghegan wrote: >> * I think it's worth looking into ucol_nextSortKeyPart(), and using >> that as an alternative to ucol_getSortKey(). It doesn't seem any >> harder, and when I tested it it was clearly faster. (I think that

Re: [HACKERS] ICU integration

I wrote: > Evidently collateral damage from 2f5c9d9c9. But I'd suggest waiting > to fix it until you can also do s/simple_heap_delete/CatalogTupleDelete/ > as I proposed in > https://www.postgresql.org/message-id/462.1485902...@sss.pgh.pa.us > I'll go make that happen right now, now that I

Re: [HACKERS] ICU integration

Pavel Stehule writes: > This patch is not possible to compile on today master > commands/collationcmds.o: In function `AlterCollation': > /home/pavel/src/postgresql/src/backend/commands/collationcmds.c:297: > undefined reference to `CatalogUpdateIndexes' Evidently

Re: [HACKERS] ICU integration

Hi 2017-01-24 18:44 GMT+01:00 Peter Eisentraut < peter.eisentr...@2ndquadrant.com>: > On 1/15/17 5:53 AM, Pavel Stehule wrote: > > the regress test fails > > > > Program received signal SIGSEGV, Segmentation fault. > > 0x007bbc2b in pattern_char_isalpha (locale_is_c=0 '\000', > >

Re: [HACKERS] ICU integration

On Wed, Jan 25, 2017 at 2:44 AM, Peter Eisentraut wrote: > On 1/15/17 5:53 AM, Pavel Stehule wrote: >> the regress test fails >> >> Program received signal SIGSEGV, Segmentation fault. >> 0x007bbc2b in pattern_char_isalpha (locale_is_c=0 '\000', >>

Re: [HACKERS] ICU integration

On Tue, Jan 10, 2017 at 9:45 AM, Peter Geoghegan wrote: > * I think it's worth looking into ucol_nextSortKeyPart(), and using > that as an alternative to ucol_getSortKey(). It doesn't seem any > harder, and when I tested it it was clearly faster. (I think that >

Re: [HACKERS] ICU integration

On 1/9/17 3:45 PM, Peter Geoghegan wrote: > * I think it's worth looking into ucol_nextSortKeyPart(), and using > that as an alternative to ucol_getSortKey(). It doesn't seem any > harder, and when I tested it it was clearly faster. (I think that > ucol_nextSortKeyPart() is more or less intended

Re: [HACKERS] ICU integration

Hi 2016-12-28 3:50 GMT+01:00 Peter Eisentraut : > Updated patch attached. > > The previous round of reviews showed that there was general agreement on > the approach. So I have focused on filling in the gaps, added ICU > support to all the locale-using places,

Re: [HACKERS] ICU integration

On Tue, Dec 27, 2016 at 6:50 PM, Peter Eisentraut wrote: > Updated patch attached. Some more things I noticed following another quick read over the patch: * I think it's worth looking into ucol_nextSortKeyPart(), and using that as an alternative to

Re: [HACKERS] ICU integration

On Mon, Jan 9, 2017 at 12:25 PM, Peter Eisentraut wrote: > On 1/7/17 10:01 PM, Peter Geoghegan wrote: >> It occurs to me that the comparison caching stuff added by commit >> 0e57b4d8b needs to be considered here, too. > That might be worth looking into, but it

Re: [HACKERS] ICU integration

On 1/7/17 10:01 PM, Peter Geoghegan wrote: > It occurs to me that the comparison caching stuff added by commit > 0e57b4d8b needs to be considered here, too. When we had to copy the > string to a temp buffer anyway, in order to add the terminating NUL > byte expected by strcoll(), there was an

Re: [HACKERS] ICU integration

On Tue, Dec 27, 2016 at 6:50 PM, Peter Eisentraut wrote: > I don't have much experience with the abbreviated key stuff. I have > filled in what I think should work, but it needs detailed review. Thanks. It occurs to me that the comparison caching stuff added

Re: [HACKERS] ICU integration

On 9/30/16 4:32 PM, Thomas Munro wrote: >> Hmm, yeah, that will need more work. To be completely correct, I think, >> > we'd also need to record the version in each expression node, so that >> > check constraints of the form CHECK (x > 'abc') can be handled. > Hmm. That is quite a rabbit hole.

Re: [HACKERS] ICU integration

On Sat, Oct 1, 2016 at 3:30 AM, Peter Eisentraut wrote: > On 9/23/16 2:27 AM, Thomas Munro wrote: >> The warning persists even after I reindex all affected tables (and >> start a new backend), because you're only recording the collation at >> pg_collation level

Re: [HACKERS] ICU integration

On 9/23/16 2:27 AM, Thomas Munro wrote: > This patch adds pkg-config support to our configure script, in order > to produce the build options for ICU. That's cool, and I'm a fan of > pkg-config, but it's an extra dependency that I just wanted to > highlight. For example MacOSX appears to ship

Re: [HACKERS] ICU integration

On Fri, Sep 23, 2016 at 6:27 PM, Thomas Munro wrote: > On Wed, Aug 31, 2016 at 2:46 PM, Peter Eisentraut > wrote: >> Here is a patch I've been working on to allow the use of ICU for sorting >> and other locale things. > > This is

Re: [HACKERS] ICU integration

On Sat, Sep 24, 2016 at 10:13 PM, Peter Geoghegan wrote: > On Fri, Sep 23, 2016 at 7:27 AM, Thomas Munro > wrote: >> It looks like varstr_abbrev_convert calls strxfrm unconditionally >> (assuming TRUST_STRXFRM is defined). This needs to >> use

Re: [HACKERS] ICU integration

On Fri, Sep 23, 2016 at 7:27 AM, Thomas Munro wrote: > A couple of thoughts about abbreviated keys: > > #ifndef TRUST_STRXFRM > if (!collate_c) > abbreviate = false; > #endif > > I think this macro should affect only strxfrm, and we should trust >

Re: [HACKERS] ICU integration

On Wed, Aug 31, 2016 at 2:46 PM, Peter Eisentraut wrote: > Here is a patch I've been working on to allow the use of ICU for sorting > and other locale things. This is very interesting work, and it's great to see some development in this area. I've been

Re: [HACKERS] ICU integration

Alvaro Herrera writes: > Petr Jelinek wrote: >>> I'm not sure how well it will work to replace all the bits of LIKE and >>> regular expressions with ICU API calls. One problem is that ICU likes >>> to do case folding as a whole string, not by character. I need to do

Re: [HACKERS] ICU integration

Petr Jelinek wrote: > > I'm not sure how well it will work to replace all the bits of LIKE and > > regular expressions with ICU API calls. One problem is that ICU likes > > to do case folding as a whole string, not by character. I need to do > > more research about that. > > Can't we use the

Re: [HACKERS] ICU integration

On 31/08/16 04:46, Peter Eisentraut wrote: > Here is a patch I've been working on to allow the use of ICU for sorting > and other locale things. > Hi, first pass review of the patch (somewhat high level at this point). First, there was some discussion about ICU versioning and collation

Re: [HACKERS] ICU integration

On Thu, Sep 8, 2016 at 12:19:39PM -0400, Peter Eisentraut wrote: > On 9/8/16 11:16 AM, Tom Lane wrote: > > This is a problem, if ICU won't guarantee cross-version compatibility, > > because it destroys the argument that moving to ICU would offer us > > collation behavior stability. > > It would

Re: [HACKERS] ICU integration

On Fri, Sep 9, 2016 at 6:39 AM, Dave Page wrote: > Looking back at my old emails, apparently ICU 5.0 and later include > ucol_strcollUTF8() which avoids the need to convert UTF-8 characters > to 16 bit before sorting. RHEL 6 has the older 4.2 version of ICU. At the risk of

Re: [HACKERS] ICU integration

On Fri, Sep 9, 2016 at 2:27 PM, Tom Lane wrote: > Dave Page writes: >> We needed a specific version that was newer than that shipped with >> RHEL 6 (or in EPEL) iirc. > > Sure hope that's not true of the currently-proposed patch :-( Looking back at my old

Re: [HACKERS] ICU integration

Dave Page writes: > We needed a specific version that was newer than that shipped with > RHEL 6 (or in EPEL) iirc. Sure hope that's not true of the currently-proposed patch :-( regards, tom lane -- Sent via pgsql-hackers mailing list

Re: [HACKERS] ICU integration

On Fri, Sep 9, 2016 at 9:02 AM, Devrim Gündüz wrote: > > Hi, > > On Fri, 2016-09-09 at 09:48 +0800, Craig Ringer wrote: >> Personally I would be pretty reluctant to package libicu when it's >> already in RH/Fedora. If it were bundled in Pg's source tree and a >> private copy

Re: [HACKERS] ICU integration

On 9/8/16 8:07 PM, Peter Geoghegan wrote: > Not exactly. Peter E. didn't seem to be aware that there is an ICU > collator versioning concept (perhaps I misunderstood, though). You appear to have missed the part about the collversion column that my patch adds. That's exactly the collator version

Re: [HACKERS] ICU integration

On 9/8/16 11:08 PM, Peter Geoghegan wrote: > In principle, and assuming I haven't gotten something wrong, it ought > to be possible to unambiguously identify a collation based on a > matching UCA version (i.e. DUCET table), plus the collation tailorings > matching exactly, even across ICU versions

Re: [HACKERS] ICU integration

On 9 September 2016 at 16:21, Magnus Hagander wrote: > On Thu, Sep 8, 2016 at 6:19 PM, Peter Eisentraut > wrote: >> >> On 9/8/16 11:16 AM, Tom Lane wrote: >> > This is a problem, if ICU won't guarantee cross-version compatibility, >> >

Re: [HACKERS] ICU integration

On Thu, Sep 8, 2016 at 6:19 PM, Peter Eisentraut < peter.eisentr...@2ndquadrant.com> wrote: > On 9/8/16 11:16 AM, Tom Lane wrote: > > This is a problem, if ICU won't guarantee cross-version compatibility, > > because it destroys the argument that moving to ICU would offer us > > collation

Re: [HACKERS] ICU integration

Hi, On Fri, 2016-09-09 at 09:48 +0800, Craig Ringer wrote: > Personally I would be pretty reluctant to package libicu when it's > already in RH/Fedora. If it were bundled in Pg's source tree and a > private copy was built/installed by the build system so it was part of > the main

Re: [HACKERS] ICU integration

On Thu, Sep 8, 2016 at 6:48 PM, Craig Ringer wrote: > Pity ICU doesn't offer versioned collations within a single install. > Though I can understand why they don't. There are two separate issues with collator versioning. ICU can probably be used in a way that clearly

Re: [HACKERS] ICU integration

On 9 September 2016 at 08:51, Tatsuo Ishii wrote: >> On 9 September 2016 at 00:19, Peter Eisentraut >> wrote: >>> On 9/8/16 11:16 AM, Tom Lane wrote: This is a problem, if ICU won't guarantee cross-version compatibility, because it

Re: [HACKERS] ICU integration

> On 9 September 2016 at 00:19, Peter Eisentraut > wrote: >> On 9/8/16 11:16 AM, Tom Lane wrote: >>> This is a problem, if ICU won't guarantee cross-version compatibility, >>> because it destroys the argument that moving to ICU would offer us >>> collation

Re: [HACKERS] ICU integration

On 9 September 2016 at 00:19, Peter Eisentraut wrote: > On 9/8/16 11:16 AM, Tom Lane wrote: >> This is a problem, if ICU won't guarantee cross-version compatibility, >> because it destroys the argument that moving to ICU would offer us >> collation behavior

Re: [HACKERS] ICU integration

On Thu, Sep 8, 2016 at 8:16 AM, Tom Lane wrote: >> I understand that in principle, but I don't see operating system >> providers shipping a bunch of ICU versions to facilitate that. They >> will usually ship one. > > I agree with that estimate, and I would further venture

Re: [HACKERS] ICU integration

On 9/8/16 11:16 AM, Tom Lane wrote: > This is a problem, if ICU won't guarantee cross-version compatibility, > because it destroys the argument that moving to ICU would offer us > collation behavior stability. It would offer a significant upgrade over the current situation. First, it offers

Re: [HACKERS] ICU integration

Peter Eisentraut writes: > On 9/6/16 1:40 PM, Doug Doole wrote: >> We carried the ICU version numbers around on our collation and locale >> IDs (such as fr_FR%icu36) . The database would load multiple versions of >> the ICU library so that something created with

Re: [HACKERS] ICU integration

> - I can't remember the specific language but they had the collation rule > that "CH" was treated as a distinct entity between C and D. This gave the > order C < CG < CI < CZ < CH < D. Then they removed CH as special which gave > C < CG < CH < CI < CZ < D. Suppose there was the constraint CHECK

Re: [HACKERS] ICU integration

> > This isn't a problem for Postgres, or at least wouldn't be right now, > because we don't have case insensitive collations. I was wondering if Postgres might be that way. It does avoid the RI constraint problem, but there are still troubles with range based predicates. (My previous project

Re: [HACKERS] ICU integration

> I understand that in principle, but I don't see operating system > providers shipping a bunch of ICU versions to facilitate that. They > will usually ship one. Yep. If you want the protection I've described, you can't depend on the OS copy of ICU. You need to have multiple ICU libraries that

Re: [HACKERS] ICU integration

On 9/6/16 1:40 PM, Doug Doole wrote: > We carried the ICU version numbers around on our collation and locale > IDs (such as fr_FR%icu36) . The database would load multiple versions of > the ICU library so that something created with ICU 3.6 would always be > processed with ICU 3.6. This avoided

Re: [HACKERS] ICU integration

On Tue, Sep 6, 2016 at 10:40 AM, Doug Doole wrote: > - Suppose in ICU X.X, AA = Å but in ICU Y.Y AA != Å. Further suppose there > was an RI constraint where the primary key used AA and the foreign key used > Å. If ICU was updated, the RI constraint between the rows would

Re: [HACKERS] ICU integration

On Tue, Sep 6, 2016 at 10:40 AM, Doug Doole wrote: >> The ICU ABI (not API) is also versioned. The way that this is done is >> that all functions are actually macros to a versioned symbol. So >> ucol_open() is actually a macro that expands to, say, ucol_open_57() in >>

Re: [HACKERS] ICU integration

> The ICU ABI (not API) is also versioned. The way that this is done is > that all functions are actually macros to a versioned symbol. So > ucol_open() is actually a macro that expands to, say, ucol_open_57() in > ICU version 57. (They also got rid of a dot in their versions a while > ago.)

Re: [HACKERS] ICU integration

On 8/31/16 1:32 PM, Peter Geoghegan wrote: > ICU is happy to have multiple versions > of a collation at a time, and you'll probably retain the old collation > version in ICU. > > Even if your old collation version isn't available in a new ICU > release (which I think is unlikely in practice) I

Re: [HACKERS] ICU integration

On 8/31/16 4:24 PM, Doug Doole wrote: > ICU explicitly does not provide stability in their locales and collations. We > pushed them hard to provide this, but between changes to the CLDR data and > changes to the ICU code it just wasn’t feasible for them to provide version > to version

Re: [HACKERS] ICU integration

On 8/31/16 12:32 AM, Michael Paquier wrote: > On Wed, Aug 31, 2016 at 1:12 PM, Peter Eisentraut > wrote: >> On 8/30/16 11:27 PM, Craig Ringer wrote: >>> Speaking of which, have you had a chance to try it on Windows yet? >> >> nope > > +SELECT a, b FROM

Re: [HACKERS] ICU integration

Hi all. I’m new to the PostgreSQL code and the mailing list, but I’ve had a lot of experience with using ICU in a different database product. So while I’m not up to speed on the code yet, I can offer some insights on using ICU. > On Aug 30, 2016, at 9:12 PM, Peter Eisentraut >

Re: [HACKERS] ICU integration

On Tue, Aug 30, 2016 at 7:46 PM, Peter Eisentraut wrote: > In initdb, I initialize the default collation set as before from the > `locale -a` output, but also add all available ICU locales with a "%icu" > appended (so "fr_FR%icu"). I suppose one could create a

Re: [HACKERS] ICU integration

On Tue, Aug 30, 2016 at 7:46 PM, Peter Eisentraut wrote: > Here is a patch I've been working on to allow the use of ICU for sorting > and other locale things. I'm really happy that you're working on this. This is more important than is widely appreciated, and

Re: [HACKERS] ICU integration

On Wed, Aug 31, 2016 at 1:12 PM, Peter Eisentraut wrote: > On 8/30/16 11:27 PM, Craig Ringer wrote: >> Speaking of which, have you had a chance to try it on Windows yet? > > nope +SELECT a, b FROM collate_test2 ORDER BY b; + a | b +---+- + 1 | abc + 4 | ABC

Re: [HACKERS] ICU integration

On 8/30/16 11:27 PM, Craig Ringer wrote: > Speaking of which, have you had a chance to try it on Windows yet? nope > How stable are the UCU locales? Most importantly, does ICU offer any > way to "pin" a locale version, so we can say "we want de_DE as it was > in ICU 4.6" and get consistent

Re: [HACKERS] ICU integration

On 31 August 2016 at 10:46, Peter Eisentraut wrote: > Here is a patch I've been working on to allow the use of ICU for sorting > and other locale things. Great to see you working on this. We've got some icky head-in-the-sand issues in this area when it comes to

[HACKERS] ICU integration

Here is a patch I've been working on to allow the use of ICU for sorting and other locale things. This is mostly complementary to the existing FreeBSD ICU patch, most recently discussed in [0]. While that patch removes the POSIX locale use and replaces it with ICU, my interest was on allowing