On Sat, 10 Jul 2021 at 11:44, Dean Rasheed <dean.a.rash...@gmail.com> wrote: > > I'm inclined to think that it isn't worth the effort trying to > distinguish between conflicting options, options specified more than > once and faked-up options that weren't really specified. If we just > make errorConflictingDefElem() report "conflicting or redundant > options", then it's much easier to update calling code without making > mistakes. The benefit then comes from the reduced code size and the > fact that the patch includes pstate in more places, so the > parser_errposition() indicator helps the user identify the problem. > > In file_fdw_validator(), where there is no pstate, it's already using > "specified more than once" as a hint to clarify the "conflicting or > redundant options" error, so I think we should leave that alone. >
Another possibility would be to pass the option list to errorConflictingDefElem() and it could work out whether or not to include the "option \%s\" specified more than once" hint, since that hint probably is useful, and working out whether to include it is probably less error-prone if it's done there. Regards, Dean
