Re: [BUG FIX] Fix validation of COPY options FORCE_NOT_NULL/FORCE_NULL

On Thu, Oct 17, 2024, at 01:50, Michael Paquier wrote:
> On Wed, Oct 16, 2024 at 02:50:53PM +0900, Michael Paquier wrote:
>> You are right. f6d4c9cf162b got that wrong. Will fix and backpatch
>> with the extra tests.
>
> And done down to 17 for 0002, down to 16 for 0001, with tweaks in 0001
> to limit the use of COPY TO in the queries where we want to force
> error patterns linked to the TO clause.
>
> The two tests for the all-column cases with force_quote were not
> strictly required for the bug, still are useful to have for coverage
> purposes.

Thanks for fixing.
I noticed a small mistake in the changes made in commit 03bf0d9:

In my v2-0001 patch, the correction was:

-COPY x to stdin (format TEXT, force_quote(a));
+COPY x to stdout (format TEXT, force_quote(a));

However, in commit 03bf0d9, the corresponding change was:

-COPY x to stdin (format TEXT, force_quote(a));
+COPY x from stdin (format TEXT, force_quote(a));

I believe the correction should be to use COPY TO stdout instead of
COPY FROM stdin, since FORCE_QUOTE is only applicable with COPY TO.
This way, the test correctly verifies the disallowed combination of FORCE_QUOTE
with a non-CSV format in COPY TO.

I see how this is easy to miss, since the tests for the three FORCE_* options
look very similar, but they are different in that it's only FORCE_QUOTE that
is allowed for COPY TO, whereas the two other FORCE_NOT_NULL
and FORCE_NULL are only allowed for COPY FROM.

/Joel