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

On Fri, Apr 30, 2021 at 8:16 AM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> On Thu, Apr 29, 2021 at 10:44 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
> >
> > On 2021-Apr-29, vignesh C wrote:
> >
> > > Thanks for the comments, please find the attached v3 patch which has
> > > the change for the first part.
> >
> > Looks good to me. I would only add parser_errposition() to the few
> > error sites missing that.
>
> Yes, we need to add parser_errposition as agreed in [1].
>
> I think we will have to make changes in compute_common_attribute as
> well because the error in the duplicate_error goto statement is
> actually for the duplicate option specified more than once, we can do
> something like the attached. If it seems okay, it can be merged with
> the main patch.

+ DefElem *duplicate_item = NULL;
+
if (strcmp(defel->defname, "volatility") == 0)
{
if (is_procedure)
goto procedure_error;
if (*volatility_item)
- goto duplicate_error;
+ duplicate_item = defel;

In this function, we already have the "defel" variable then I do not
understand why you are using one extra variable and assigning defel to
that?
If the goal is to just improve the error message then you can simply
use defel->defname?

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