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

Dilip Kumar <dilipbalaut@gmail.com>

From: Dilip Kumar <dilipbalaut@gmail.com>
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, vignesh C <vignesh21@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2021-05-01T05:17:25Z
Lists: pgsql-hackers
On Sat, May 1, 2021 at 10:43 AM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> On Fri, Apr 30, 2021 at 2:49 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > Looking into this again, why not as shown below?  IMHO, this way the
> > code will be logically the same as it was before the patch, basically
> > why to process an extra statement ( *volatility_item = defel;) if we
> > have already decided to error.
>
> I changed my mind given the concerns raised on removing the goto
> statements. We could just do as below:

Okay, that make sense.

> diff --git a/src/backend/commands/functioncmds.c
> b/src/backend/commands/functioncmds.c
> index 9548287217..1f1c74c379 100644
> --- a/src/backend/commands/functioncmds.c
> +++ b/src/backend/commands/functioncmds.c
> @@ -575,7 +575,7 @@ compute_common_attribute(ParseState *pstate,
>  duplicate_error:
>      ereport(ERROR,
>              (errcode(ERRCODE_SYNTAX_ERROR),
> -             errmsg("conflicting or redundant options"),
> +             errmsg("option \"%s\" specified more than once", defel->defname),
>               parser_errposition(pstate, defel->location)));
>      return false;                /* keep compiler quiet */
>
> I'm not attaching above one line change as a patch, maybe Vignesh can
> merge this into the main patch.

+1

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com



Commits

  1. Improve reporting of "conflicting or redundant options" errors.

  2. Refactor function parse_subscription_options.