Re: Add header support to text format and matching feature

On Sat, Apr 10, 2021 at 4:17 PM Rémi Lapeyre <remi(dot)lapeyre(at)lenstra(dot)fr>
wrote:

>
> >
> > Michael, since the issue of duplicated options has been fixed do either
> of these patches look like they are ready for commit?
> >
>
> Here’s a rebased version of the patch.
>
>
> Cheers,
> Rémi
>
>
> > Regards,
> > --
> > -David
> > david(at)pgmasters(dot)net
>
>
> Hi,
>> sure it matches what is expected and exit immediatly if it does not.

Typo: immediately

+CREATE FOREIGN TABLE header_dont_match (a int, foo text) SERVER file_server

nit: since header is singular, you can name the table header_doesnt_match

+ from the one expected, or the name or case do not match, the copy
will

For 'the name or case do not match', either use plural for the subjects or
change 'do' to doesn't

- opts_out->header_line = defGetBoolean(defel);
+ opts_out->header_line = DefGetCopyHeader(defel);

Existing method starts with lower case d, I wonder why the new method
starts with upper case D.

+ if (fldct < list_length(cstate->attnumlist))
+ ereport(ERROR,
+ (errcode(ERRCODE_BAD_COPY_FILE_FORMAT),
+ errmsg("missing header")));

The message seems to be inaccurate: the header may be there - it just
misses some fields.

+ * Represents whether the header must be absent, present or present and
match.

present and match: it seems present is redundant - if header is absent, how
can it match ?

Cheers