Re: Enhanced error message to include hint messages for redundant options error

From: vignesh C <vignesh21(at)gmail(dot)com>
To: Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
Cc: Daniel Gustafsson <daniel(at)yesql(dot)se>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>
Subject: Re: Enhanced error message to include hint messages for redundant options error
Date: 2021-07-10 17:08:53
Message-ID: CALDaNm33ybC9g=O2g1xS0fMcbNBwkDhP+iW8ym-mDLakw+=h2g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, Jul 10, 2021 at 4:31 PM Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> wrote:
>
> On Sat, 10 Jul 2021 at 11:44, Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fabien COELHO 2021-07-10 17:19:45 Re: [HACKERS] WIP aPatch: Pgbench Serialization and deadlock errors
Previous Message Dean Rasheed 2021-07-10 17:03:54 Re: pgsql: Fix numeric_mul() overflow due to too many digits after decimal