Re: [PATCH] Add error hints for invalid COPY options

On Thu, Dec 4, 2025 at 4:35 PM Kirill Reshke <reshkekirill(at)gmail(dot)com> wrote:

> On Thu, 4 Dec 2025 at 03:56, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
> wrote:
>
> > The proposed patch requires us to create one function per option. I'm
> > concerned that it could cause bloating functions and be overkill just
> > to save changing a separate list. I would suggest starting with
> > putting a validation check for options at the top of foreach() loop.
> > When adding a new COPY option in the future, it wouldn't work if we
> > miss either changing the valid-options list or handling the option,
> > which seems a good prevention.
> >
>
> Yep, Im +1 on that. "bloating functions" - that's precise wording, I
> did not know how to explain the same concern.
>
>
> --
> Best regards,
> Kirill Reshke
>

Thanks everyone for reviewing my proposal.

> The proposed patch requires us to create one function per option. I'm
> concerned that it could cause bloating functions and be overkill just
> to save changing a separate list. I would suggest starting with
> putting a validation check for options at the top of foreach() loop.
> When adding a new COPY option in the future, it wouldn't work if we
> miss either changing the valid-options list or handling the option,
> which seems a good prevention.

I completely agree with your feedback. I will proceed with a smaller and
simpler revision of the changes. The v3 patch was beneficial in that it
removed duplicated option names for COPY options in the code, but it
introduced too much refactoring for such a small improvement.

Attached is the new patch. One possible discussion point is that I choose
FATAL
over ERROR at src/backend/commands/copy.c#L816, since reaching that point
would
indicate an implementation problem. Please let me know if there is a better
option.

Regards,