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

From: Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
To: vignesh C <vignesh21(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-15 08:10:19
Message-ID: CAEZATCXHWa9OoSAetiZiGQy1eM2raa9q-b3K4ZYDwtcARypCcA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

Regards,
Dean

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Magnus Hagander 2021-07-15 08:33:11 Git revision in tarballs
Previous Message gkokolatos 2021-07-15 07:48:08 Re: Introduce pg_receivewal gzip compression tests