Re: Add header support to text format and matching feature

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?)