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

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?

Thanks,

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company