From: | Rémi Lapeyre <remi(dot)lapeyre(at)lenstra(dot)fr> |
---|---|
To: | Zhihong Yu <zyu(at)yugabyte(dot)com> |
Cc: | David Steele <david(at)pgmasters(dot)net>, Michael Paquier <michael(at)paquier(dot)xyz>, vignesh C <vignesh21(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Daniel Verite <daniel(at)manitou-mail(dot)org> |
Subject: | Re: Add header support to text format and matching feature |
Date: | 2021-04-11 11:01:22 |
Message-ID: | 683DD77E-38B3-4BC2-810E-4122C7A5A053@lenstra.fr |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
>
> 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 | Content-Type | Size |
---|---|---|
v9-0001-Add-header-support-to-COPY-TO-text-format.patch | application/octet-stream | 7.4 KB |
v9-0002-Add-header-matching-mode-to-COPY-FROM.patch | application/octet-stream | 14.6 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Andrew Dunstan | 2021-04-11 12:04:51 | Re: PL/R regression on windows, but not linux with master. |
Previous Message | Bharath Rupireddy | 2021-04-11 10:15:36 | Re: TRUNCATE on foreign table |