On Sat, Jul 10, 2021 at 4:31 PM Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
>
> On Sat, 10 Jul 2021 at 11:44, Dean Rasheed <dean.a.rasheed@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.
I'm planning to handle conflicting errors separately after this
current work is done, once the patch is changed to have just the valid
scenarios(removing the scenarios you have pointed out) existing
function can work as is without any changes. Thoughts?
Regards,
Vignesh