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.
cheers
andrew
