On 10/08/2013 12:30 PM, Robert Haas wrote:
> On Tue, Oct 8, 2013 at 8:33 AM, Andrew Dunstan <andrew@dunslane.net> wrote:
>> On 10/07/2013 11:34 PM, Amit Kapila wrote:
>>> On Tue, Oct 8, 2013 at 12:55 AM, Andrew Dunstan <andrew@dunslane.net>
>>> wrote:
>>>> On 10/07/2013 03:06 PM, Robert Haas wrote:
>>>>>> Also if your use case is to treat empty strings as NULL (as per above
>>>>>> documentation), can't it be handled with "WITH NULL AS" option.
>>>>>> For example, something like:
>>>>>>
>>>>>> postgres=# COPY testnull FROM stdin with CSV NULL AS E'';
>>>>>> Enter data to be copied followed by a newline.
>>>>>> End with a backslash and a period on a line by itself.
>>>>>>>> 50,
>>>>>>>> \.
>>>>>> postgres=# select * from testnull;
>>>>>> a | b
>>>>>> ----+------
>>>>>> 50 | NULL
>>>>>> (1 row)
>>>>> Good point. If this patch is just implementing something that can
>>>>> already be done with another syntax, we don't need it.
>>>>>
>>>> Isn't the point of this option to allow a *quoted* empty string to be
>>>> forced
>>>> to NULL? If so, this is not testing the same case - in fact the COPY
>>>> command
>>>> above just makes explicit the default CSV NULL setting anyway.
>>> I am really not sure if all the purpose of patch can be achieved by
>>> existing syntax, neither it is explained clearly.
>>> However the proposal hasn't discussed why it's not good idea to extend
>>> some similar syntax "COPY .. NULL" which is used to replace string
>>> with NULL's?
>>> Description of NULL says: "Specifies the string that represents a null
>>> value."
>>> Now why can't this syntax be extended to support quoted empty string
>>> if it's not supported currently?
>>> I have not checked completely, If it's difficult or not possible to
>>> support in existing syntax, then even it add's more value to introduce
>>> new syntax.
>>>
>>> By asking above question, I doesn't mean that we should not go for the
>>> new proposed syntax, rather it's to know and understand the benefit of
>>> new syntax, also it helps during CF review for reviewer's if the
>>> proposal involves new syntax and that's discussed previously.
>>>
>> Quite apart from any other consideration, this suggestion is inferior to
>> what's proposed in that it's an all or nothing deal, while the patch allows
>> you to specify the behaviour very explicitly on a per column basis. I can
>> well imagine wanting to be able to force a quoted empty string to null for
>> numeric fields but not for text.
>>
>> The basic principal of our CSV processing is that we don't ever turn a NULL
>> into something quoted and we don't ever turn something quoted into NULL.
>> That's what lets us round-trip test just about every combination of options.
>> I'm only going to be happy violating that, as this patch does, in a very
>> explicit and controlled way.
> Andrew, are you planning to review & commit this?
>
Yes.
cheers
andrew
