On 31.01.22 07:54, Peter Eisentraut wrote:
> On 30.01.22 23:56, Rémi Lapeyre wrote:
>>> I notice in the 0002 patch that there is no test case for the error
>>> "wrong header for column \"%s\": got \"%s\"", which I think is really
>>> the core functionality of this patch. So please add that.
>>>
>>
>> I added a test for it in this new version of the patch.
>
> The file_fdw.sql tests contain this
>
> +CREATE FOREIGN TABLE header_doesnt_match (a int, foo text) SERVER
> file_server
> +OPTIONS (format 'csv', filename :'filename', delimiter ',', header
> 'match'); -- ERROR
>
> but no actual error is generated. Please review the additions on the
> file_fdw tests to see that they make sense.
A few more comments on your latest patch:
- The DefGetCopyHeader() function seems very bulky and might not be
necessary. I think you can just check for the string "match" first and
then use defGetBoolean() as before if it didn't match.
- If you define COPY_HEADER_ABSENT = 0 in the enum, then most of the
existing truth checks don't need to be changed.
- In NextCopyFromRawFields(), it looks like you have code that replaces
the null_print value if the supplied column name is null. I don't think
we should allow null column values. Someone, this should be an error.
(Do we use null_print on output and make the column name null if it
matches?)
