Re: Extend COPY FROM with HEADER <integer> to skip multiple lines

On Fri, 27 Jun 2025 12:22:17 +0900
Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)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(at)sraoss(dot)co(dot)jp>