Horton

Thread

Commits

  1. Fix handling of redundant options with COPY for "freeze" and "header"

  1. Buggy handling of redundant options in COPY

    Michael Paquier <michael@paquier.xyz> — 2020-09-29T07:24:33Z

    Hi all,

While diving into the CF, I have noticed the following message from
Remy (in CC):
https://www.postgresql.org/message-id/0B55BD07-83E4-439F-AACC-FA2D7CF50532@lenstra.fr

The following two cases should fail the same way, but the second does
not because we check directly the flag value extracted from the
DefElem to see if the option is repeated or not:
=# copy (select 1) to '/tmp/data.txt' (header on, header off);
ERROR:  42601: conflicting or redundant options
=# copy (select 1) to '/tmp/data.txt' (header off, header on);
ERROR:  0A000: COPY HEADER available only in CSV mode

Looking quickly at the usages of defGetBoolean() across the code, it
seems that we are rather consistent on a command-basis to handle such
cases (EXPLAIN does not care, subscriptions do, etc.), while COPY is 
a mixed bag that clearly aims at checking for redundant options
correctly.  So, attached is a patch to do that, with tests for the
various options while on it.  This is not something worth a
back-patch in my opinion.

Any thoughts?
--
Michael

  2. Re: Buggy handling of redundant options in COPY

    Pavel Stehule <pavel.stehule@gmail.com> — 2020-09-29T07:35:39Z

    út 29. 9. 2020 v 9:24 odesílatel Michael Paquier <michael@paquier.xyz>
napsal:

> Hi all,
>
> While diving into the CF, I have noticed the following message from
> Remy (in CC):
>
> https://www.postgresql.org/message-id/0B55BD07-83E4-439F-AACC-FA2D7CF50532@lenstra.fr
>
> The following two cases should fail the same way, but the second does
> not because we check directly the flag value extracted from the
> DefElem to see if the option is repeated or not:
> =# copy (select 1) to '/tmp/data.txt' (header on, header off);
> ERROR:  42601: conflicting or redundant options
> =# copy (select 1) to '/tmp/data.txt' (header off, header on);
> ERROR:  0A000: COPY HEADER available only in CSV mode
>
> Looking quickly at the usages of defGetBoolean() across the code, it
> seems that we are rather consistent on a command-basis to handle such
> cases (EXPLAIN does not care, subscriptions do, etc.), while COPY is
> a mixed bag that clearly aims at checking for redundant options
> correctly.  So, attached is a patch to do that, with tests for the
> various options while on it.  This is not something worth a
> back-patch in my opinion.
>
> Any thoughts?
>

+1

Pavel

--
> Michael
>

  3. Re: Buggy handling of redundant options in COPY

    Michael Paquier <michael@paquier.xyz> — 2020-10-05T00:48:21Z

    On Tue, Sep 29, 2020 at 09:35:39AM +0200, Pavel Stehule wrote:
> +1

Thanks.  I have applied this one.  We may rework the handling of
redundant options for all commands to be more consistent in the
future, though it does not prevent fixing this issue until it
happens.
--
Michael