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
0001-Correct-negative-tests-for-the-COPY-option-FORCE_QUO.patch
Description: Binary data
