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

From: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
To: vignesh C <vignesh21(at)gmail(dot)com>
Cc: "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, 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-05-09 12:39:38
Message-ID: CALj2ACXkk=xTYu466GmpnGb=Tm6Z+CsL=AcjpvaLrhm__5gP6w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, May 8, 2021 at 7:06 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Sat, May 8, 2021 at 2:20 PM Bharath Rupireddy
> <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> >
> > On Sat, May 8, 2021 at 12:01 PM houzj(dot)fnst(at)fujitsu(dot)com
> > <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> > >
> > > > > > Thanks! The v5 patch looks good to me. Let's see if all agree on the
> > > > > > goto duplicate_error approach which could reduce the LOC by ~80.
> > > > >
> > > > > I think the "goto duplicate_error" approach looks good, it avoids
> > > > > duplicating the same error code multiple times.
> > > >
> > > > Thanks. I will mark the v5 patch "ready for committer" if no one has comments.
> > >
> > > Hi,
> > >
> > > I looked into the patch and noticed a minor thing.
> > >
> > > + return; /* keep compiler quiet */
> > > }
> > >
> > > I think we do not need the comment here.
> > > The compiler seems not require "return" at the end of function
> > > when function's return type is VOID.
> > >
> > > In addition, it seems better to remove these "return;" like what
> > > commit "3974c4" did.
> >
> > It looks like that commit removed the plain return statements for a
> > void returning functions. I see in the code that there are return
> > statements that are there right after ereport(ERROR, just to keep the
> > compiler quiet. Here in this patch also, we have return; statements
> > after ereport(ERROR, for void returning functions. I'm not sure
> > removing them would cause some compiler warnings on some platforms
> > with some other compilers. If we're not sure, I'm okay to keep those
> > return; statements. Thoughts?
>
> I felt we could retain the return statement and remove the comments.
> If you are ok with that I will modify and post a patch for it.
> Thoughts?

I would like to keep it as is i.e. both return statement and /* keep
compiler quiet */ comment. Having said that, it's better to leave it
to the committer on whether to have the return statement at all.

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Yura Sokolov 2021-05-09 12:57:22 Re: plan with result cache is very slow when work_mem is not enough
Previous Message vignesh C 2021-05-09 12:37:45 Corrected documentation of data type for the logical replication message formats.