From: | Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> |
---|---|
To: | Euler Taveira <euler(at)timbira(dot)com(dot)br>, Andres Freund <andres(at)anarazel(dot)de> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: move collation import to backend |
Date: | 2017-01-17 17:05:33 |
Message-ID: | 08ef05ba-d8c5-192f-9c59-17d6b1ec9b10@2ndquadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2017-01-17 17:26:58 | Re: Packages: Again |
Previous Message | Robert Haas | 2017-01-17 17:05:04 | Re: Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling) |