Re: Extend COPY FROM with HEADER to skip multiple lines - Mailing list pgsql-hackers

From Yugo Nagata
Subject Re: Extend COPY FROM with HEADER to skip multiple lines
Date
Msg-id 20250627130942.f48ab7f5032e176f6af44775@sraoss.co.jp
Whole thread Raw
In response to Re: Extend COPY FROM with HEADER to skip multiple lines  (Fujii Masao <masao.fujii@oss.nttdata.com>)
List pgsql-hackers
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>



pgsql-hackers by date:

Previous
From: jian he
Date:
Subject: pg_restore --no-policies should not restore policies' comment
Next
From: Dilip Kumar
Date:
Subject: Re: Slot's restart_lsn may point to removed WAL segment after hard restart unexpectedly