Re: [HACKERS] Review: Patch FORCE_NULL option for copy COPY in CSV mode

2014-04-22 Thread Michael Paquier
On Wed, Apr 23, 2014 at 5:07 AM, Bruce Momjian wrote: > On Fri, Mar 7, 2014 at 05:08:54PM +0900, Michael Paquier wrote: > > On Thu, Mar 6, 2014 at 12:09 AM, Tom Lane wrote: > > > Andrew Dunstan writes: > > >> On 03/05/2014 09:11 AM, Michael Paquier wrote: > > >>> After testing this feature, I

Re: [HACKERS] Review: Patch FORCE_NULL option for copy COPY in CSV mode

2014-04-22 Thread Bruce Momjian
On Fri, Mar 7, 2014 at 05:08:54PM +0900, Michael Paquier wrote: > On Thu, Mar 6, 2014 at 12:09 AM, Tom Lane wrote: > > Andrew Dunstan writes: > >> On 03/05/2014 09:11 AM, Michael Paquier wrote: > >>> After testing this feature, I noticed that FORCE_NULL and > >>> FORCE_NOT_NULL can both be speci

Re: [HACKERS] Review: Patch FORCE_NULL option for copy COPY in CSV mode

2014-04-22 Thread Bruce Momjian
On Wed, Mar 5, 2014 at 09:49:30PM +0900, Michael Paquier wrote: > On Wed, Mar 5, 2014 at 7:44 AM, Andrew Dunstan wrote: > > I have picked this up and committed the patch. Thanks to all. > Sorry for coming after the battle, but while looking at what has been > committed I noticed that copy2.sql is

Re: [HACKERS] Review: Patch FORCE_NULL option for copy COPY in CSV mode

2014-03-07 Thread Michael Paquier
On Thu, Mar 6, 2014 at 12:09 AM, Tom Lane wrote: > Andrew Dunstan writes: >> On 03/05/2014 09:11 AM, Michael Paquier wrote: >>> After testing this feature, I noticed that FORCE_NULL and >>> FORCE_NOT_NULL can both be specified with COPY on the same column. > >> Strictly they are not actually cont

Re: [HACKERS] Review: Patch FORCE_NULL option for copy COPY in CSV mode

2014-03-05 Thread Tom Lane
Andrew Dunstan writes: > On 03/05/2014 09:11 AM, Michael Paquier wrote: >> After testing this feature, I noticed that FORCE_NULL and >> FORCE_NOT_NULL can both be specified with COPY on the same column. > Strictly they are not actually contradictory, since FORCE NULL relates > to quoted null str

Re: [HACKERS] Review: Patch FORCE_NULL option for copy COPY in CSV mode

2014-03-05 Thread Michael Paquier
On Wed, Mar 5, 2014 at 11:58 PM, Michael Paquier wrote: > So if we specify both this produces the exact opposite of the default, > default being an empty string inserted for a quoted empty string and > NULL inserted for a non-quoted empty string. So yes I'm fine with a > note on the docs about tha

Re: [HACKERS] Review: Patch FORCE_NULL option for copy COPY in CSV mode

2014-03-05 Thread Michael Paquier
On Wed, Mar 5, 2014 at 11:37 PM, Ian Lawrence Barwick wrote: > 2014-03-05 23:27 GMT+09:00 Andrew Dunstan : >> >> On 03/05/2014 09:11 AM, Michael Paquier wrote: >>> >>> After testing this feature, I noticed that FORCE_NULL and >>> FORCE_NOT_NULL can both be specified with COPY on the same column. >

Re: [HACKERS] Review: Patch FORCE_NULL option for copy COPY in CSV mode

2014-03-05 Thread Ian Lawrence Barwick
2014-03-05 23:27 GMT+09:00 Andrew Dunstan : > > On 03/05/2014 09:11 AM, Michael Paquier wrote: >> >> After testing this feature, I noticed that FORCE_NULL and >> FORCE_NOT_NULL can both be specified with COPY on the same column. >> This does not seem correct. The attached patch adds some more error

Re: [HACKERS] Review: Patch FORCE_NULL option for copy COPY in CSV mode

2014-03-05 Thread Andrew Dunstan
On 03/05/2014 09:11 AM, Michael Paquier wrote: After testing this feature, I noticed that FORCE_NULL and FORCE_NOT_NULL can both be specified with COPY on the same column. This does not seem correct. The attached patch adds some more error handling, and a regression test case for that. Stric

Re: [HACKERS] Review: Patch FORCE_NULL option for copy COPY in CSV mode

2014-03-05 Thread Michael Paquier
After testing this feature, I noticed that FORCE_NULL and FORCE_NOT_NULL can both be specified with COPY on the same column. This does not seem correct. The attached patch adds some more error handling, and a regression test case for that. Regards, -- Michael diff --git a/src/backend/commands/copy

Re: [HACKERS] Review: Patch FORCE_NULL option for copy COPY in CSV mode

2014-03-05 Thread Michael Paquier
On Wed, Mar 5, 2014 at 7:44 AM, Andrew Dunstan wrote: > I have picked this up and committed the patch. Thanks to all. Sorry for coming after the battle, but while looking at what has been committed I noticed that copy2.sql is actually doing twice in a row the same test: COPY forcetest (a, b, c) FR

Re: [HACKERS] Review: Patch FORCE_NULL option for copy COPY in CSV mode

2014-03-04 Thread Andrew Dunstan
On 03/03/2014 06:48 AM, Michael Paquier wrote: On Mon, Mar 3, 2014 at 1:13 PM, Andrew Dunstan wrote: That difference actually made the file_fdw regression results plain wrong, in my view, in that they expected a quoted empty string to be turned to null even when the null string was something e

Re: [HACKERS] Review: Patch FORCE_NULL option for copy COPY in CSV mode

2014-03-03 Thread Michael Paquier
On Mon, Mar 3, 2014 at 1:13 PM, Andrew Dunstan wrote: >>> That difference actually made the file_fdw regression results plain >>> wrong, >>> in my view, in that they expected a quoted empty string to be turned to >>> null >>> even when the null string was something else. >>> >>> I've adjusted this

Re: [HACKERS] Review: Patch FORCE_NULL option for copy COPY in CSV mode

2014-03-02 Thread Andrew Dunstan
On 03/02/2014 10:06 PM, Ian Lawrence Barwick wrote: 2014-03-02 8:26 GMT+09:00 Andrew Dunstan : On 01/29/2014 10:59 AM, Ian Lawrence Barwick wrote: 2014/1/29 Ian Lawrence Barwick : 2014-01-29 Andrew Dunstan : On 01/28/2014 05:55 AM, Ian Lawrence Barwick wrote: Hi Payal Many thanks for the

Re: [HACKERS] Review: Patch FORCE_NULL option for copy COPY in CSV mode

2014-03-02 Thread Ian Lawrence Barwick
2014-03-02 8:26 GMT+09:00 Andrew Dunstan : > > On 01/29/2014 10:59 AM, Ian Lawrence Barwick wrote: >> >> 2014/1/29 Ian Lawrence Barwick : >>> >>> 2014-01-29 Andrew Dunstan : On 01/28/2014 05:55 AM, Ian Lawrence Barwick wrote: > > > Hi Payal > > Many thanks for the revi

Re: [HACKERS] Review: Patch FORCE_NULL option for copy COPY in CSV mode

2014-03-01 Thread Andrew Dunstan
On 01/29/2014 10:59 AM, Ian Lawrence Barwick wrote: 2014/1/29 Ian Lawrence Barwick : 2014-01-29 Andrew Dunstan : On 01/28/2014 05:55 AM, Ian Lawrence Barwick wrote: Hi Payal Many thanks for the review, and my apologies for not getting back to you earlier. Updated version of the patch attac

Re: [HACKERS] Review: Patch FORCE_NULL option for copy COPY in CSV mode

2014-01-29 Thread Ian Lawrence Barwick
2014/1/29 Ian Lawrence Barwick : > 2014-01-29 Andrew Dunstan : >> >> On 01/28/2014 05:55 AM, Ian Lawrence Barwick wrote: >>> >>> >>> Hi Payal >>> >>> Many thanks for the review, and my apologies for not getting back to >>> you earlier. >>> >>> Updated version of the patch attached with suggested co

Re: [HACKERS] Review: Patch FORCE_NULL option for copy COPY in CSV mode

2014-01-28 Thread Ian Lawrence Barwick
2014-01-29 Andrew Dunstan : > > On 01/28/2014 05:55 AM, Ian Lawrence Barwick wrote: >> >> >> Hi Payal >> >> Many thanks for the review, and my apologies for not getting back to >> you earlier. >> >> Updated version of the patch attached with suggested corrections. > > On a very quick glance, I see

Re: [HACKERS] Review: Patch FORCE_NULL option for copy COPY in CSV mode

2014-01-28 Thread Andrew Dunstan
On 01/28/2014 05:55 AM, Ian Lawrence Barwick wrote: Hi Payal Many thanks for the review, and my apologies for not getting back to you earlier. Updated version of the patch attached with suggested corrections. On a very quick glance, I see that you have still not made adjustments to contri

Re: [HACKERS] Review: Patch FORCE_NULL option for copy COPY in CSV mode

2014-01-28 Thread Ian Lawrence Barwick
2013-11-01 Payal Singh : > The post was made before I subscribed to the mailing list, so posting my > review separately. The link to the original patch mail is > http://www.postgresql.org/message-id/CAB8KJ=jS-Um4TGwenS5wLUfJK6K4rNOm_V6GRUj+tcKekL2=g...@mail.gmail.com > >> >> Hi, >> >> This patch im