Thread: Extend COPY FROM with HEADER to skip multiple lines
Hi hackers, I'd like to propose a new feature for the COPY FROM command to allow skipping multiple header lines when loading data. This enhancement would enable files with multi-line headers to be loaded without any preprocessing, which would significantly improve usability. In real-world scenarios, it's common for data files to contain multiple header lines, such as file descriptions or column explanations. Currently, the COPY command cannot load these files directly, which requires users to preprocess them with tools like sed or tail. Although you can use "COPY t FROM PROGRAM 'tail -n +3 /path/to/file'", some environments do not have the tail command available. Additionally, this approach requires superuser privileges or membership in the pg_execute_server_program role. This feature also has precedent in other major RDBMS: - MySQL: LOAD DATA ... IGNORE N LINES [1] - SQL Server: BULK INSERT … WITH (FIRST ROW=N) [2] - Oracle SQL*Loader: sqlldr … SKIP=N [3] I have not yet created a patch, but I am willing to implement an extension for the HEADER option. I would like to discuss the specification first. The specification I have in mind is as follows: - Command: COPY FROM - Formats: text and csv - Option syntax: HEADER [ boolean | integer | MATCH] (Extend the HEADER option to accept an integer value in addition to the existing boolean and MATCH keywords.) - Behavior: Let N be the specified integer. - If N < 0, raise an error. - If N = 0 or 1, same behavior when boolean is specified. - If N > 1, skip the first N rows. Thoughts? [1] https://dev.mysql.com/doc/refman/8.4/en/load-data.html#load-data-field-line-handling [2] https://learn.microsoft.com/en-us/sql/t-sql/statements/bulk-insert-transact-sql?view=sql-server-ver17#firstrow--first_row [3] https://docs.oracle.com/en/database/oracle/oracle-database/23/sutil/oracle-sql-loader-commands.html#SUTIL-GUID-84244C46-6AFD-412D-9240-BEB0B5C2718B -- Best regards, Shinya Kato NTT OSS Center
On 2025/06/09 16:10, Shinya Kato wrote: > Hi hackers, > > I'd like to propose a new feature for the COPY FROM command to allow > skipping multiple header lines when loading data. This enhancement > would enable files with multi-line headers to be loaded without any > preprocessing, which would significantly improve usability. > > In real-world scenarios, it's common for data files to contain > multiple header lines, such as file descriptions or column > explanations. Currently, the COPY command cannot load these files > directly, which requires users to preprocess them with tools like sed > or tail. > > Although you can use "COPY t FROM PROGRAM 'tail -n +3 /path/to/file'", > some environments do not have the tail command available. > Additionally, this approach requires superuser privileges or > membership in the pg_execute_server_program role. > > This feature also has precedent in other major RDBMS: > - MySQL: LOAD DATA ... IGNORE N LINES [1] > - SQL Server: BULK INSERT … WITH (FIRST ROW=N) [2] > - Oracle SQL*Loader: sqlldr … SKIP=N [3] > > I have not yet created a patch, but I am willing to implement an > extension for the HEADER option. I would like to discuss the > specification first. > > The specification I have in mind is as follows: > - Command: COPY FROM > - Formats: text and csv > - Option syntax: HEADER [ boolean | integer | MATCH] (Extend the > HEADER option to accept an integer value in addition to the existing > boolean and MATCH keywords.) > - Behavior: Let N be the specified integer. > - If N < 0, raise an error. > - If N = 0 or 1, same behavior when boolean is specified. > - If N > 1, skip the first N rows. > > Thoughts? I generally like the idea. However, a similar proposal was made earlier [1], and seemingly some hackers weren't in favor of it. It's probably worth reading that thread to understand the previous concerns. Regards, [1] https://postgr.es/m/CALAY4q8nGSXp0P5uf56vn-mD7reWqZP5k6PS1CGUm26X4FsYJA@mail.gmail.com -- Fujii Masao NTT DATA Japan Corporation
2025年6月9日(月) 17:27 Fujii Masao <masao.fujii@oss.nttdata.com>: > I generally like the idea. Thanks. > However, a similar proposal was made earlier [1], and seemingly > some hackers weren't in favor of it. It's probably worth reading > that thread to understand the previous concerns. > > [1] https://postgr.es/m/CALAY4q8nGSXp0P5uf56vn-mD7reWqZP5k6PS1CGUm26X4FsYJA@mail.gmail.com Oh, I missed it. I will check it soon. -- Best regards, Shinya Kato NTT OSS Center
On 2025-06-09 Mo 4:27 AM, Fujii Masao wrote: > > > On 2025/06/09 16:10, Shinya Kato wrote: >> Hi hackers, >> >> I'd like to propose a new feature for the COPY FROM command to allow >> skipping multiple header lines when loading data. This enhancement >> would enable files with multi-line headers to be loaded without any >> preprocessing, which would significantly improve usability. >> >> In real-world scenarios, it's common for data files to contain >> multiple header lines, such as file descriptions or column >> explanations. Currently, the COPY command cannot load these files >> directly, which requires users to preprocess them with tools like sed >> or tail. >> >> Although you can use "COPY t FROM PROGRAM 'tail -n +3 /path/to/file'", >> some environments do not have the tail command available. >> Additionally, this approach requires superuser privileges or >> membership in the pg_execute_server_program role. >> >> This feature also has precedent in other major RDBMS: >> - MySQL: LOAD DATA ... IGNORE N LINES [1] >> - SQL Server: BULK INSERT … WITH (FIRST ROW=N) [2] >> - Oracle SQL*Loader: sqlldr … SKIP=N [3] >> >> I have not yet created a patch, but I am willing to implement an >> extension for the HEADER option. I would like to discuss the >> specification first. >> >> The specification I have in mind is as follows: >> - Command: COPY FROM >> - Formats: text and csv >> - Option syntax: HEADER [ boolean | integer | MATCH] (Extend the >> HEADER option to accept an integer value in addition to the existing >> boolean and MATCH keywords.) >> - Behavior: Let N be the specified integer. >> - If N < 0, raise an error. >> - If N = 0 or 1, same behavior when boolean is specified. >> - If N > 1, skip the first N rows. >> >> Thoughts? > > I generally like the idea. > > However, a similar proposal was made earlier [1], and seemingly > some hackers weren't in favor of it. It's probably worth reading > that thread to understand the previous concerns. > > Regards, > > > [1] > https://postgr.es/m/CALAY4q8nGSXp0P5uf56vn-mD7reWqZP5k6PS1CGUm26X4FsYJA@mail.gmail.com I think the earlier proposal went rather further than this one, which I suspect can be implemented fairly cheaply. I don't have terribly strong feelings about it, but matching a feature implemented elsewhere has some attraction if it can be done easily. OTOH I'm a bit curious to know what software produces multi-line CSV headers. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
> > However, a similar proposal was made earlier [1], and seemingly > > some hackers weren't in favor of it. It's probably worth reading > > that thread to understand the previous concerns. > > > > [1] https://postgr.es/m/CALAY4q8nGSXp0P5uf56vn-mD7reWqZP5k6PS1CGUm26X4FsYJA@mail.gmail.com > > Oh, I missed it. I will check it soon. I read it. There are clear differences from the earlier proposal. My sole motivation is to skip multiple headers, and I don't believe a feature for skipping footers is necessary. To be clear, I think it's best to simply extend the current HEADER option. Regarding the concern about adding ETL-like functionality, this feature is already implemented in other RDBMSs, which is why I believe it is also necessary for PostgreSQL. Honestly, I haven't implemented it yet, so I'm not sure about the performance. However, I don't expect it to be significantly different from the current HEADER option that skips a single line. > I think the earlier proposal went rather further than this one, which I > suspect can be implemented fairly cheaply. That's probably it, exactly. > I don't have terribly strong feelings about it, but matching a feature > implemented elsewhere has some attraction if it can be done easily. > > OTOH I'm a bit curious to know what software produces multi-line CSV > headers. Both Pandas and R can create CSV files with multi-line headers (although I don't personally think this is desirable). Furthermore, various systems sometimes generate reports as CSV files that unexpectedly contain multiple header lines. -- Best regards, Shinya Kato NTT OSS Center
On 2025/06/10 9:43, Shinya Kato wrote: >>> However, a similar proposal was made earlier [1], and seemingly >>> some hackers weren't in favor of it. It's probably worth reading >>> that thread to understand the previous concerns. >>> >>> [1] https://postgr.es/m/CALAY4q8nGSXp0P5uf56vn-mD7reWqZP5k6PS1CGUm26X4FsYJA@mail.gmail.com >> >> Oh, I missed it. I will check it soon. > > I read it. > > There are clear differences from the earlier proposal. My sole > motivation is to skip multiple headers, and I don't believe a feature > for skipping footers is necessary. To be clear, I think it's best to > simply extend the current HEADER option. Sounds ok to me. > Regarding the concern about adding ETL-like functionality, this > feature is already implemented in other RDBMSs, which is why I believe > it is also necessary for PostgreSQL. > > Honestly, I haven't implemented it yet, so I'm not sure about the > performance. However, I don't expect it to be significantly different > from the current HEADER option that skips a single line. So it seems better for you to implement the patch at first and then check the performance overhead etc if necessary. Regards, -- Fujii Masao NTT DATA Japan Corporation
On Tue, Jun 10, 2025 at 2:34 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote: > > There are clear differences from the earlier proposal. My sole > > motivation is to skip multiple headers, and I don't believe a feature > > for skipping footers is necessary. To be clear, I think it's best to > > simply extend the current HEADER option. > > Sounds ok to me. Thank you. > > Regarding the concern about adding ETL-like functionality, this > > feature is already implemented in other RDBMSs, which is why I believe > > it is also necessary for PostgreSQL. > > > > Honestly, I haven't implemented it yet, so I'm not sure about the > > performance. However, I don't expect it to be significantly different > > from the current HEADER option that skips a single line. > > So it seems better for you to implement the patch at first and then > check the performance overhead etc if necessary. Thank you for your advice. I will create a patch. -- Best regards, Shinya Kato NTT OSS Center On Tue, Jun 10, 2025 at 2:34 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote: > > > > On 2025/06/10 9:43, Shinya Kato wrote: > >>> However, a similar proposal was made earlier [1], and seemingly > >>> some hackers weren't in favor of it. It's probably worth reading > >>> that thread to understand the previous concerns. > >>> > >>> [1] https://postgr.es/m/CALAY4q8nGSXp0P5uf56vn-mD7reWqZP5k6PS1CGUm26X4FsYJA@mail.gmail.com > >> > >> Oh, I missed it. I will check it soon. > > > > I read it. > > > > There are clear differences from the earlier proposal. My sole > > motivation is to skip multiple headers, and I don't believe a feature > > for skipping footers is necessary. To be clear, I think it's best to > > simply extend the current HEADER option. > > Sounds ok to me. > > > > Regarding the concern about adding ETL-like functionality, this > > feature is already implemented in other RDBMSs, which is why I believe > > it is also necessary for PostgreSQL. > > > > Honestly, I haven't implemented it yet, so I'm not sure about the > > performance. However, I don't expect it to be significantly different > > from the current HEADER option that skips a single line. > > So it seems better for you to implement the patch at first and then > check the performance overhead etc if necessary. > > Regards, > > -- > Fujii Masao > NTT DATA Japan Corporation > -- Best regards, Shinya Kato NTT OSS Center
Andrew Dunstan <andrew@dunslane.net> writes: > OTOH I'm a bit curious to know what software produces multi-line CSV > headers. AWS CloudFront access logs are stored in S3 as TSV files (one per hour per CF node) with a two-line header comment where the first line is the version and the second lists the fields (but not in a form useful for HEADER MATCH). Although not useful for the above format, and not intended to derail or bloat the proposal in this thread, would it be useful to have a mode that combines skip and match? I.e. skip N lines, then check the fields in the one after that against the target columns. - ilmari
On 2025/06/26 14:35, Shinya Kato wrote:
>
> > > So it seems better for you to implement the patch at first and then
> > > check the performance overhead etc if necessary.
> >
> > Thank you for your advice. I will create a patch.
>
> I created a patch.
Thanks for the patch!
> As you can see from the patch, I believe the performance impact is negligible. The only changes were to modify how
theHEADER option is stored and to add a loop that skips the specified number of header lines when parsing the options.
>
> The design is such that if a HEADER value larger than the number of lines in the file is specified, the command will
completewith zero rows loaded and will not return an error. This is the same behavior as specifying HEADER true for a
CSVfile that has zero rows.
Sounds good to me.
> And I will add this patch for the next CF.
>
> Thoughts?
When I compiled with the patch applied, I got the following warning:
copyfromparse.c:834:20: error: ‘done’ may be used uninitialized [-Werror=maybe-uninitialized]
834 | if (done)
| ^
+ lines are discarded. An integer value greater than 1 is only valid for
+ <command>COPY FROM</command> commands.
This might be slightly misleading since 0 is also a valid value for COPY FROM.
+ for (int i = 0; i < cstate->opts.header.skip_lines; i++)
+ {
+ cstate->cur_lineno++;
+ done = CopyReadLine(cstate, is_csv);
+ }
If "done" is true, shouldn't we break out of the loop immediately?
Otherwise, with a large HEADER value, we could end up wasting a lot of
cycles unnecessarily.
+defGetCopyHeaderOption(DefElem *def, bool is_from)
{
+ CopyHeaderOption option;
To avoid repeating the same code like "option.match = false" in every case,
how about initializing the struct like "option = {false, 1}"?
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("HEADER must be non-negative integer")));
This message could be confusing since HEADER can also accept a Boolean or "match".
Maybe it's better to use the same message as "%s requires a Boolean value, integer value,
or \"match\"",? "integer value"? If so, it's also better to use "non-negative integer"
instead of just "integer".
Regards,
--
Fujii Masao
NTT DATA Japan Corporation
On Thu, Jun 26, 2025 at 4:36 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
On 2025/06/26 14:35, Shinya Kato wrote:
>
> > > So it seems better for you to implement the patch at first and then
> > > check the performance overhead etc if necessary.
> >
> > Thank you for your advice. I will create a patch.
>
> I created a patch.
Thanks for the patch!
Thank you for your review.
When I compiled with the patch applied, I got the following warning:
copyfromparse.c:834:20: error: ‘done’ may be used uninitialized [-Werror=maybe-uninitialized]
834 | if (done)
| ^
Oh, sorry. I missed it.
I fixed it to initialize done = false.
+ lines are discarded. An integer value greater than 1 is only valid for
+ <command>COPY FROM</command> commands.
This might be slightly misleading since 0 is also a valid value for COPY FROM.
That's certainly true. I fixed it below:
+ lines are discarded. An integer value greater than 1 is not allowed for
+ <command>COPY TO</command> commands.
+ <command>COPY TO</command> commands.
+ for (int i = 0; i < cstate->opts.header.skip_lines; i++)
+ {
+ cstate->cur_lineno++;
+ done = CopyReadLine(cstate, is_csv);
+ }
If "done" is true, shouldn't we break out of the loop immediately?
Otherwise, with a large HEADER value, we could end up wasting a lot of
cycles unnecessarily.
Exactly, fixed.
+defGetCopyHeaderOption(DefElem *def, bool is_from)
{
+ CopyHeaderOption option;
To avoid repeating the same code like "option.match = false" in every case,
how about initializing the struct like "option = {false, 1}"?
Exactly, fixed.
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("HEADER must be non-negative integer")));
This message could be confusing since HEADER can also accept a Boolean or "match".
Maybe it's better to use the same message as "%s requires a Boolean value, integer value,
or \"match\"",? "integer value"? If so, it's also better to use "non-negative integer"
instead of just "integer".
Yes, I fixed it to use the same message which replaced "integer" to "non-negative integer".
Original error message "%s requires a Boolean value, integer value, or \"match\"" should also be fixed to "non-negative integer"?
Best regards,
Shinya Kato
NTT OSS Center
Attachment
On Fri, Jun 27, 2025 at 12:03 AM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
On 2025/06/26 19:12, Shinya Kato wrote:
> On Thu, Jun 26, 2025 at 4:36 PM Fujii Masao <masao.fujii@oss.nttdata.com <mailto:masao.fujii@oss.nttdata.com>> wrote:
>
> On 2025/06/26 14:35, Shinya Kato wrote:
> >
> > > > So it seems better for you to implement the patch at first and then
> > > > check the performance overhead etc if necessary.
> > >
> > > Thank you for your advice. I will create a patch.
> >
> > I created a patch.
>
> Thanks for the patch!
>
>
> Thank you for your review.
>
> When I compiled with the patch applied, I got the following warning:
>
> copyfromparse.c:834:20: error: ‘done’ may be used uninitialized [-Werror=maybe-uninitialized]
> 834 | if (done)
> | ^
>
>
> Oh, sorry. I missed it.
> I fixed it to initialize done = false.
>
> + lines are discarded. An integer value greater than 1 is only valid for
> + <command>COPY FROM</command> commands.
>
>
> This might be slightly misleading since 0 is also a valid value for COPY FROM.
>
> That's certainly true. I fixed it below:
> + lines are discarded. An integer value greater than 1 is not allowed for
> + <command>COPY TO</command> commands.
Thanks for fixing that! However, it's odd that the description for COPY TO appears
under the section starting with "On input." Shouldn't it be under the "On output"
section instead?
Also, I think the entire HEADER option section could use a clearer structure.
How about revising it like this?
---------------------
<listitem>
<para>
- Specifies that the file contains a header line with the names of each
- column in the file. On output, the first line contains the column
- names from the table. On input, the first line is discarded when this
- option is set to <literal>true</literal> (or equivalent Boolean value).
- If this option is set to <literal>MATCH</literal>, the number and names
- of the columns in the header line must match the actual column names of
- the table, in order; otherwise an error is raised.
+ On output, if this option is set to <literal>true</literal>
+ (or an equivalent Boolean value), the first line of the output will
+ contain the column names from the table.
+ Integer values <literal>0</literal> and <literal>1</literal> are
+ accepted as Boolean values, but other integers are not allowed for
+ <command>COPY TO</command> commands.
+ </para>
+ <para>
+ On input, if this option is set to <literal>true</literal>
+ (or an equivalent Boolean value), the first line of the input is
+ discarded. If it is set to a non-negative integer, that number of
+ lines are discarded. If the option is set to <literal>MATCH</literal>,
+ the number and names of the columns in the header line must exactly
+ match those of the table, in order; otherwise an error is raised.
+ The <literal>MATCH</literal> value is only valid for
+ <command>COPY FROM</command> commands.
+ </para>
+ <para>
This option is not allowed when using <literal>binary</literal> format.
- The <literal>MATCH</literal> option is only valid for <command>COPY
- FROM</command> commands.
</para>
</listitem>
---------------------
Yes, your documentation is clearer than mine.
Also, similar to the existing "boolean" type explanation in the COPY docs,
we should add a corresponding entry for integer, now that it's accepted by
the HEADER option. The VACUUM docs has a good example of how to phrase this.
Exactly.
> Original error message "%s requires a Boolean value, integer value, or \"match\"" should also be fixed to "non-negative integer"?
Yes, I think that the message
"%s requires a Boolean value, a non-negative integer, or the string \"match\""
is clearer and more accurate.
Thank you, you're right.
-typedef enum CopyHeaderChoice
+typedef struct CopyHeaderOption
{
- COPY_HEADER_FALSE = 0,
- COPY_HEADER_TRUE,
- COPY_HEADER_MATCH,
-} CopyHeaderChoice;
+ bool match; /* header line must match actual names? */
+ int skip_lines; /* number of lines to skip before data */
+} CopyHeaderOption;
<snip>
- CopyHeaderChoice header_line; /* header line? */
+ CopyHeaderOption header; /* header line? */
Wouldn't it be simpler to just use an integer instead of introducing
CopyHeaderOption? We could use 0 for false, 1 for true, n > 1 for skipping lines,
and -1 for MATCH in that integer.
This would simplify the logic in defGetCopyHeaderOption().
I've attached a patch that shows this idea in action. Thought?
I thought about the approach, and had a minor concern about the following inconsistency between the option and its internal implementation:
- When the option is set to "MATCH", header_line becomes -1.
- When the option is set to -1, it's an error.
However, your patch is clear, and it seems we don't need to worry about this concern.
Your patch looks good to me.
Your patch looks good to me.
Best regards,
Shinya Kato
NTT OSS Center
On 2025/06/27 11:55, Shinya Kato wrote: > I thought about the approach, and had a minor concern about the following inconsistency between the option and its internalimplementation: > - When the option is set to "MATCH", header_line becomes -1. > - When the option is set to -1, it's an error. I think this inconsistency is acceptable, since users won't encounter it directly. Also the value -1 isn't used explicitly, i.e., it's always referenced through a macro for "match", which keeps the code readable and understandable. > However, your patch is clear, and it seems we don't need to worry about this concern. > Your patch looks good to me. Thanks for reviewing the patch! I've marked it as ready for committer. Regards, -- Fujii Masao NTT DATA Japan Corporation
On Fri, 27 Jun 2025 12:22:17 +0900
Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
> > However, your patch is clear, and it seems we don't need to worry about this concern.
> > Your patch looks good to me.
>
> Thanks for reviewing the patch! I've marked it as ready for committer.
I have a few minor comment on the patch.
+ if (ival < 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("%s requires a Boolean value, a non-negative "
+ "integer, or the string \"match\"",
+ def->defname)));
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("%s requires a Boolean value or \"match\"",
+ errmsg("%s requires a Boolean value, a non-negative integer, "
+ "or the string \"match\"",
def->defname)));
These two pieces of code raise the same error, but with different error codes:
one returns ERRCODE_INVALID_PARAMETER_VALUE, while the other returns ERRCODE_SYNTAX_ERROR.
I believe the former is more appropriate, although the existing code uses the
latter. In any case, the error codes should be consistent.
Regarding the documentation, how about explicitly stating that when MATCH is specified, only
the first line is skipped? While this may seem obvious, it’s worth clarifying, as the semantics
of the HEADER option have become a bit more complex with this change.
Regards,
Yugo Nagata
--
Yugo Nagata <nagata@sraoss.co.jp>
On 2025/06/27 13:09, Yugo Nagata wrote:
> On Fri, 27 Jun 2025 12:22:17 +0900
> Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>
>>> However, your patch is clear, and it seems we don't need to worry about this concern.
>>> Your patch looks good to me.
>>
>> Thanks for reviewing the patch! I've marked it as ready for committer.
>
> I have a few minor comment on the patch.
>
> + if (ival < 0)
> + ereport(ERROR,
> + (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> + errmsg("%s requires a Boolean value, a non-negative "
> + "integer, or the string \"match\"",
> + def->defname)));
>
> ereport(ERROR,
> (errcode(ERRCODE_SYNTAX_ERROR),
> - errmsg("%s requires a Boolean value or \"match\"",
> + errmsg("%s requires a Boolean value, a non-negative integer, "
> + "or the string \"match\"",
> def->defname)));
>
> These two pieces of code raise the same error, but with different error codes:
> one returns ERRCODE_INVALID_PARAMETER_VALUE, while the other returns ERRCODE_SYNTAX_ERROR.
>
> I believe the former is more appropriate, although the existing code uses the
> latter. In any case, the error codes should be consistent.
I'm not sure there's an actual rule like "the error code must match
if the error message is the same." But if using the same message with
a different error code is confusing, I'm fine with changing
the earlier message. For example:
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("%s requires a Boolean value, a non-negative "
- "integer, or the string \"match\"",
+ errmsg("a negative integer value cannot be "
+ "specified for %s",
def->defname)));
Regards,
--
Fujii Masao
NTT DATA Japan Corporation
On Wed, Jul 2, 2025 at 4:48 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote: > >> Regarding the documentation, how about explicitly stating that when MATCH is specified, only > >> the first line is skipped? While this may seem obvious, it’s worth clarifying, as the semantics > >> of the HEADER option have become a bit more complex with this change. > > > > Agreed. I have updated the documentation as follows: > > > > + lines are discarded. If the option is set to <literal>MATCH</literal>, > > + the number and names of the columns in the header line must exactly > > + match those of the table and, in order, after which the header line is > > + discarded; otherwise an error is raised. The <literal>MATCH</literal> > > How about making the wording a bit clearer? For example: > > If set to MATCH, the first line is discarded, and it must contain column names that > exactly match the table's columns, in both number and order; otherwise, an error is raised. Thank you for the review. I fixed it. > > Also, the phrase "if this option is set to..." is repeated three times in the current text. > For the second and third instances, we could simplify it to just "if set to...". Agreed. However, for the sake of symmetry between "On output" and "On input" and to maintain clarity between the paragraphs, I have omitted "this option is" from the "On input" paragraph only. -- Best regards, Shinya Kato NTT OSS Center
Attachment
On Thu, Jul 3, 2025 at 3:32 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote: > > > > On 2025/07/03 11:08, Shinya Kato wrote: > > On Wed, Jul 2, 2025 at 4:48 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote: > > > >>>> Regarding the documentation, how about explicitly stating that when MATCH is specified, only > >>>> the first line is skipped? While this may seem obvious, it’s worth clarifying, as the semantics > >>>> of the HEADER option have become a bit more complex with this change. > >>> > >>> Agreed. I have updated the documentation as follows: > >>> > >>> + lines are discarded. If the option is set to <literal>MATCH</literal>, > >>> + the number and names of the columns in the header line must exactly > >>> + match those of the table and, in order, after which the header line is > >>> + discarded; otherwise an error is raised. The <literal>MATCH</literal> > >> > >> How about making the wording a bit clearer? For example: > >> > >> If set to MATCH, the first line is discarded, and it must contain column names that > >> exactly match the table's columns, in both number and order; otherwise, an error is raised. > > > > Thank you for the review. I fixed it. > > Thanks for updating the patch! I've pushed the patch. > > > >> Also, the phrase "if this option is set to..." is repeated three times in the current text. > >> For the second and third instances, we could simplify it to just "if set to...". > > > > Agreed. However, for the sake of symmetry between "On output" and "On > > input" and to maintain clarity between the paragraphs, I have omitted > > "this option is" from the "On input" paragraph only. > > Yes, I agree that's better. > > Regards, > > -- > Fujii Masao > NTT DATA Japan Corporation > Thank you for pushing! -- Best regards, Shinya Kato NTT OSS Center