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-22 06:09:23
Message-ID: CALDaNm2+a_NctqSncORCZR3RH39whttrrxoZmF7HNQeG6G4i_Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Jul 15, 2021 at 5:12 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Thu, Jul 15, 2021 at 1:40 PM Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> wrote:
> >
> > On Tue, 13 Jul 2021 at 15:30, vignesh C <vignesh21(at)gmail(dot)com> wrote:
> > >
> > > On Tue, Jul 13, 2021 at 4:25 PM Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> wrote:
> > > >
> > > > As it stands, the improvements from (3) seem quite worthwhile. Also,
> > > > the patch saves a couple of hundred lines of code, and for me an
> > > > optimised executable is around 30 kB smaller, which is more than I
> > > > expected.
> > >
> > > Agreed, it can be handled as part of the 2nd patch. The changes you
> > > made apply neatly and the test passes.
> >
> > Pushed.
> >
> > I noticed that it's actually safe to call parser_errposition() with a
> > null ParseState, so I simplified the ereport() code to just call it
> > unconditionally. Also, I decided to not bother using the new function
> > in cases with a null ParseState anyway since it doesn't provide any
> > meaningful benefit in those cases, and those are the cases most likely
> > to targeted next, so it didn't seem sensible to change that code, only
> > for it to be changed again later.
> >
> > Probably the thing to think about next is the few remaining cases that
> > throw this error directly and don't have any errdetail or errhint to
> > help the user identify the offending option. My preference remains to
> > leave the primary error text unchanged, but just add some suitable
> > errdetail. Also, it's probably not worth adding a new function for
> > those remaining errors, since there are only a handful of them.
>
> Thanks for pushing this patch. I have changed the commitfest status to
> "waiting for author" till 0002 patch is posted.

I'm marking this entry in commitfest as committed, I'm planning to
work on the other comments later once I finish my current project
works.

Regards,
Vignesh

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2021-07-22 06:20:28 Re: printf %s with NULL pointer (was Re: BUG #17098: Assert failed on composing an error message when adding a type to an extension being dropped)
Previous Message vignesh C 2021-07-22 06:06:47 Re: dynamic result sets support in extended query protocol