Buggy handling of redundant options in COPY

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Cc: Rémi Lapeyre <remi(dot)lapeyre(at)lenstra(dot)fr>
Subject: Buggy handling of redundant options in COPY
Date: 2020-09-29 07:24:33
Message-ID: 20200929072433.GA15570@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
copy-opts-v1.patch text/x-diff 5.1 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fujii Masao 2020-09-29 07:32:44 Re: history file on replica and double switchover
Previous Message Heikki Linnakangas 2020-09-29 07:21:01 Re: [PATCH] audo-detect and use -moutline-atomics compilation flag for aarch64