On Tue, Jan 17, 2017 at 9:05 AM, Peter Eisentraut < peter.eisentr...@2ndquadrant.com> wrote:
> On 1/9/17 10:04 PM, Euler Taveira wrote: > > On 18-12-2016 18:30, Peter Eisentraut wrote: > >> Updated patch with that fix. > >> > > Peter, I reviewed and improved your patch. > > > > * I document the new function. Since collation is a database object, I > > chose "Database Object Management Functions" section. > > OK > > > * I've added a check to any-encoding database because I got 'FATAL: > > collation "C" already exists' on Debian 8, although, I didn't get on > > CentOS 7. The problem is that Debian has two locales for C (C and > > C.UTF-8) and CentOS has just one (C). > > OK > > > * I've added OidIsValid to test the new collation row. > > OK > > > * I've changed the parameter order. Schema seems more important than > > if_not_exists. Also, we generally leave those boolean parameters for the > > end of list. I don't turn if_not_exists optional but IMO it would be a > > good idea (default = true). > > I put them that way because in an SQL command the "IF NOT EXISTS" comes > before the schema, but I can see how it is weird that way in a function. > > > * You removed some #if and #ifdef while moving things around. I put it > back. > > * You didn't pgident some lines of code but I'm sure you didn't for a > > small patch footprint. > > I had left the #if in initdb, but I think your changes are better. > > > I'm attaching the complete and also a patch at the top of your last > patch. > > Thanks. If there are no more comments, I will proceed with that. > > With this commit, I'm getting 'make check' fail at initdb with the error: 2017-01-18 07:47:50.565 PST [43691] FATAL: collation "aa_ER@saaho" for encoding "UTF8" already exists 2017-01-18 07:47:50.565 PST [43691] STATEMENT: SELECT pg_import_system_collations(if_not_exists => false, schema => 'pg_catalog'); My system: CentOS release 6.8 (Final) gcc version 4.4.7 20120313 (Red Hat 4.4.7-17) (GCC) ./configure > /dev/null # no options $ locale -a|fgrep aa_ER aa_ER aa_ER.utf8 aa_ER.utf8@saaho aa_ER@saaho I have no idea what @ means in a locale, I'm just relaying the information. Cheers, Jeff
