Re: Allow COPY's 'text' format to output a header
| От | Simon Muller |
|---|---|
| Тема | Re: Allow COPY's 'text' format to output a header |
| Дата | |
| Msg-id | CAF1-J-0p4wnexMzaV5gJHbtHuoX9vcn45X272ASXgu_o=XFt9w@mail.gmail.com обсуждение исходный текст |
| Ответ на | Re: Allow COPY's 'text' format to output a header (Cynthia Shang <cynthia.shang@crunchydata.com>) |
| Ответы |
Re: Allow COPY's 'text' format to output a header
|
| Список | pgsql-hackers |
On 1 August 2018 at 17:18, Cynthia Shang <cynthia.shang@crunchydata.com> wrote:
> On Aug 1, 2018, at 10:20 AM, Daniel Verite <daniel@manitou-mail.org> wrote:
>
> /* Check header */
> - if (!cstate->csv_mode && cstate->header_line)
> + if (cstate->binary && cstate->header_line)
> ereport(ERROR,
> - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> - errmsg("COPY HEADER available only in CSV mode")));
> + (errcode(ERRCODE_SYNTAX_ERROR),
> + errmsg("cannot specify HEADER in BINARY mode")));
>
> Why should ERRCODE_FEATURE_NOT_SUPPORTED become ERRCODE_SYNTAX_ERROR?
>
I agree; it should remain ERRCODE_FEATURE_NOT_SUPPORTED and I might also suggest the message read "COPY HEADER not available in BINARY mode", although I'm pretty agnostic on the latter.
Regards,
-Cynthia Shang
I changed the error type and message for consistency with other similar errors in that file. Whenever options are combined that are incompatible, it looks like the convention is for a ERRCODE_SYNTAX_ERROR to be thrown.
For instance, in case you both specify a specific DELIMITER but also declare the format as BINARY, then there is this code in that same file:
if (cstate->binary && cstate->delim)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("cannot specify DELIMITER in BINARY mode")));
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("cannot specify DELIMITER in BINARY mode")));
HEADER seems very similar to me since, like DELIMITER, it makes sense for the textual formats such as CSV and TEXT, but doesn't make sense with the BINARY format.
ERRCODE_FEATURE_NOT_SUPPORTED previously made sense since the only reason TEXT and HEADER weren't compatible options was because the feature was not yet implemented, but now ERRCODE_SYNTAX_ERROR seems to make sense to me since I can't foresee a use case where BINARY and HEADER would ever be compatible options.
--
Simon Muller
В списке pgsql-hackers по дате отправления: