> > 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
>> This now reads "Represents whether the header must be absent, present or match.”.
Since match shouldn't be preceded with be, I think we can say:
Represents whether the header must match, be absent or be present.