Re: Review: Patch FORCE_NULL option for copy COPY in CSV mode - Mailing list pgsql-hackers

From Ian Lawrence Barwick
Subject Re: Review: Patch FORCE_NULL option for copy COPY in CSV mode
Date
Msg-id CAB8KJ=gGG6pZ=4i4QMLLbQ1Qy_zQrmA3sfhT+wXjHN0EWf-tNQ@mail.gmail.com
Whole thread Raw
In response to Re: Review: Patch FORCE_NULL option for copy COPY in CSV mode  (Andrew Dunstan <andrew@dunslane.net>)
Responses Re: Review: Patch FORCE_NULL option for copy COPY in CSV mode  (Andrew Dunstan <andrew@dunslane.net>)
List pgsql-hackers
2014-03-02 8:26 GMT+09:00 Andrew Dunstan <andrew@dunslane.net>:
>
> On 01/29/2014 10:59 AM, Ian Lawrence Barwick wrote:
>>
>> 2014/1/29 Ian Lawrence Barwick <barwick@gmail.com>:
>>>
>>> 2014-01-29 Andrew Dunstan <andrew@dunslane.net>:
>>>>
>>>> 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
>>>> contrib/file_fdw to accommodate this new option. I don't see why this
>>>> COPY
>>>> option should be different in that respect.
>>>
>>> Hmm, that idea seems to have escaped me completely. I'll get onto it
>>> forthwith.
>>
>> Striking while the keyboard is hot... version with contrib/file_fdw
>> modifications
>> attached.
>>
>>

> I have reviewed this. Generally it's good, but the author has made a
> significant error - the idea is not to force a quoted empty string to null,
> but to force a quoted null string to null, whatever the null string might
> be. The default case has these the same, but if you specify a non-empty null
> string they aren't.

The author slaps himself on the forehead while regretting he was temporally
constricted when dealing with the patch and never thought to look beyond
the immediate use case.

Thanks for the update, much appreciated.

> 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 and the docs and propose to apply the attached patch in
> the next day or two unless there are any objections.

Unless I'm overlooking something, output from "SELECT * FROM text_csv;"
in 'output/file_fdw.source' still needs updating?


Regards

Ian Barwick

Attachment

pgsql-hackers by date:

Previous
From: Kyotaro HORIGUCHI
Date:
Subject: Re: Hot standby doesn't come up on some situation.
Next
From: Kyotaro HORIGUCHI
Date:
Subject: Re: Hot standby doesn't come up on some situation.