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

On Sat, 28 Jun 2025 00:33:45 +0900
Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:

>
>
> On 2025/06/27 13:09, Yugo Nagata wrote:
> > 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.
>
> 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)));

Fair point. There might not be any explicit rule.
However, I feel it somewhat confusing.

After looking into the code, it seems that ERRCODE_SYNTAX_ERROR tends to be used
for invalid keywords or parameter types, while ERRCODE_INVALID_PARAMETER_VALUE
is typically used for values that are out of the acceptable range.

So, the proposed change seems reasonable to me.

Regards,
Yugo Nagata

--
Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>