On Thu, Dec 4, 2025 at 4:35 PM Kirill Reshke <[email protected]> wrote:
> On Thu, 4 Dec 2025 at 03:56, Masahiko Sawada <[email protected]> > 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,
v4-0001-Add-option-name-validation-and-hints-for-COPY-comman.patch
Description: Binary data
