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

Thanks, I fixed both typos.

> -           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.
> 

I don’t remember why I used DefGetCopyHeader, should I change it?

> +           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.

I changed the error messages, they now are:
    ERROR:  incomplete header, expected 3 columns but got 2
    ERROR:  extra data after last expected header, expected 3 columns but got 4

> 
> + * 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 ?

This now reads "Represents whether the header must be absent, present or 
match.”.

Cheers,
Rémi

> 
> Cheers


Attachment: v9-0001-Add-header-support-to-COPY-TO-text-format.patch
Description: Binary data

Attachment: v9-0002-Add-header-matching-mode-to-COPY-FROM.patch
Description: Binary data

Reply via email to