RE: Enhanced error message to include hint messages for redundant options error
Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>
From: "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, vignesh C <vignesh21@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Dilip Kumar <dilipbalaut@gmail.com>
Date: 2021-05-10T00:30:09Z
Lists: pgsql-hackers
> > > > > > > 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. Yes, it's better to leave it to the committer on whether to have the "return;". But, I think at least removing "return;" which is at the *end* of the function will not cause any warning. Such as: + return; /* keep compiler quiet */ } So, I'd vote for at least removing the comment " keep the compiler quiet ". Best regards, houzj
Commits
-
Improve reporting of "conflicting or redundant options" errors.
- 2bfb50b3df11 15.0 landed
-
Refactor function parse_subscription_options.
- 8aafb0261675 15.0 cited