COPY: validate option presence rather than option values - Mailing list pgsql-hackers

From Chao Li
Subject COPY: validate option presence rather than option values
Date
Msg-id C1D2509E-E5D1-46B0-932C-B57AA7B963A1@gmail.com
Whole thread
List pgsql-hackers
Hi,

While testing COPY TO (FORMAT json), I noticed that the doc says FORCE_ARRAY is only allowed with FORMAT json:
```
   <varlistentry id="sql-copy-params-force-array">
    <term><literal>FORCE_ARRAY</literal></term>
    <listitem>
     <para>
      Force output of square brackets as array decorations at the beginning
      and end of output, and commas between the rows. It is allowed only in
      <command>COPY TO</command>, and only when using
      <literal>json</literal> format. The default is
      <literal>false</literal>.
     </para>
    </listitem>
   </varlistentry>
```

However, this succeeds:
```
evantest=# copy t1 to stdout (format csv, force_array false);
1
```

So this is clearly validating the parsed value of force_array, rather than whether the option was specified at all.

Then I noticed a related issue with ESCAPE. That option is only allowed in CSV mode, but if I do:
```
evantest=# copy t1 to stdout (format json, escape);
ERROR:  escape requires a parameter
```

The error complains about the syntax of ESCAPE, rather than the fact that ESCAPE is not allowed for "FORMAT json".

That behavior feels misleading, because once the user fixes the syntax problem, they just hit the real failure next:
```
evantest=# copy t1 to stdout (format json, escape 1);
ERROR:  COPY ESCAPE requires CSV mode
```

When ESCAPE is specified for a non-CSV format, I think we should fail immediately, instead of parsing the option
argumentfirst. 

After that, I did a thorough pass over the other COPY options and found a few more cases with the same problem. So this
patchtightens option validation by checking option presence where appropriate, rather than the parsed value. 

The fix is straightforward, see the attached patch for details.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/


Attachment

pgsql-hackers by date:

Previous
From: Amit Kapila
Date:
Subject: Re: Include schema-qualified names in publication error messages.
Next
From: Chengpeng Yan
Date:
Subject: Re: [PATCH] ANALYZE: hash-accelerate MCV tracking for equality-only types