On Sat, Sep 19, 2009 at 3:10 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Emmanuel Cecchet <manu@asterdata.com> writes:
>> [ latest patch version ]
>
> Do we have consensus on the syntax for this patch? In particular,
> what about the question of adding CSV_ to all the CSV-specific option
> names? Emmanuel argued that this is necessary to avoid confusion if
> we someday introduce other copy formats that have similar options.
> However, I think you could easily turn that argument around. Any one
> COPY command will surely use just one format, and it seems to me that
> forcing different formats to use different names for equivalent options
> won't simplify life for anybody. So I'm inclined to think we should
> not have the CSV_ prefixes. (I seem to recall that we had exactly
> this discussion when the options were introduced the first time, and
> settled on not using format-specific option names.)
Agreed. It doesn't seem inconceivable that some other format could
have a "header" or "quote" option. A related question is whether we
should replace the "CSV" option with a "FORMAT" option for which one
of the possible choices is "CSV" (much as we did with EXPLAIN).
> One other minor point is that the patch introduces an empty-list
> syntax for individual option values, but then treats it the same
> as specifying nothing:
>
>> + | '(' ')' { $$ = NULL; }
>> + | /* EMPTY */ { $$ = NULL; }
>
> I'm not convinced this is a a good idea, and in any case I don't see
> it documented. I'm inclined to omit the '(' ')' syntax.
It seemed like a good idea because if you can do force_quote (a, b, c)
and force_quote (a, b) you might think that you could also do
force_quote (). Particularly for the sake of people generating SQL
automatically by some means, it seems like this might simplify life.
But possibly it shouldn't evaluate to the same value, so that you
can't write OIDS () to mean the same thing as OIDS ON.
...Robert