Re: proposal: enable new error fields in plpgsql (9.4)

2013/6/25 Rushabh Lathia <rushabh(dot)lathia(at)gmail(dot)com>:
>
>
>
> On Tue, Jun 25, 2013 at 2:41 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> wrote:
>>
>> 2013/6/25 Rushabh Lathia <rushabh(dot)lathia(at)gmail(dot)com>:
>> > Hi Pavel,
>> >
>> > I gone through the discussion over here and found that with this patch
>> > we
>> > enable the new error fields in plpgsql. Its a simple patch to expose the
>> > new
>> > error fields in plpgsql.
>> >
>> > Patch gets applied cleanly. make and make install too went smooth. make
>> > check
>> > was smooth too. Patch also include test coverage
>> >
>> > I tested the functionality with manual test and its woking as expected.
>> >
>> > BTW in the patch I found one additional new like in
>> > read_raise_options():
>> >
>> > @@ -3631,7 +3661,23 @@ read_raise_options(void)
>> > else if (tok_is_keyword(tok, &yylval,
>> > K_HINT,
>> > "hint"))
>> > opt->opt_type = PLPGSQL_RAISEOPTION_HINT;
>> > + else if (tok_is_keyword(tok, &yylval,
>> > +
>> > K_COLUMN_NAME, "column_name"))
>> > + opt->opt_type = PLPGSQL_RAISEOPTION_COLUMN_NAME;
>> > + else if (tok_is_keyword(tok, &yylval,
>> > +
>> > K_CONSTRAINT_NAME, "constraint_name"))
>> > + opt->opt_type =
>> > PLPGSQL_RAISEOPTION_CONSTRAINT_NAME;
>> > + else if (tok_is_keyword(tok, &yylval,
>> > +
>> > K_DATATYPE_NAME, "datatype_name"))
>> > + opt->opt_type =
>> > PLPGSQL_RAISEOPTION_DATATYPE_NAME;
>> > + else if (tok_is_keyword(tok, &yylval,
>> > +
>> > K_TABLE_NAME, "table_name"))
>> > + opt->opt_type = PLPGSQL_RAISEOPTION_TABLE_NAME;
>> > + else if (tok_is_keyword(tok, &yylval,
>> > +
>> > K_SCHEMA_NAME, "schema_name"))
>> > + opt->opt_type = PLPGSQL_RAISEOPTION_SCHEMA_NAME;
>> > else
>> > +
>> > yyerror("unrecognized RAISE statement option");
>> >
>> > can you please remove that.
>>
>> No, these fields are there as was proposed - it enhance possibilities
>> to PLpgSQL developers - they can use these fields for custom
>> exceptions. It is same like possibility to specify SQLCODE, MESSAGE,
>> HINT in current RAISE statement implementation.
>>
>> Why you dislike it?
>
>
> Seems like some confusion.
>
> I noticed additional new line which has been added into your patch in
> function
> read_raise_options()::pl_gram.y @ line number 3680. And in the earlier mail
> thats what I asked to remove.
>
I am sorry

I remove these new lines

Regards

Pavel
>
>
>>
>> Regards
>>
>> Pavel
>>
>> >
>> > Apart from that patch looks good to me.
>> >
>> > Thanks,
>> > Rushabh
>> >
>> >
>> > On Fri, Feb 1, 2013 at 7:29 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
>> > wrote:
>> >>
>> >> 2013/2/1 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>:
>> >> > 2013/2/1 Peter Eisentraut <peter_e(at)gmx(dot)net>:
>> >> >> On 2/1/13 8:00 AM, Pavel Stehule wrote:
>> >> >>> 2013/2/1 Marko Tiikkaja <pgmail(at)joh(dot)to>:
>> >> >>>> On 2/1/13 1:47 PM, Pavel Stehule wrote:
>> >> >>>>>
>> >> >>>>> now a most "hard" work is done and I would to enable access to
>> >> >>>>> new
>> >> >>>>> error fields from plpgsql.
>> >> >>>>
>> >> >>>>
>> >> >>>> Is there a compelling reason why we wouldn't provide these already
>> >> >>>> in
>> >> >>>> 9.3?
>> >> >>>
>> >> >>> a time for assign to last commitfest is out.
>> >> >>>
>> >> >>> this patch is relative simple and really close to enhanced error
>> >> >>> fields feature - but depends if some from commiters will have a
>> >> >>> time
>> >> >>> for commit to 9.3 - so I am expecting primary target 9.4, but I am
>> >> >>> not
>> >> >>> be angry if it will be commited early.
>> >> >>
>> >> >> If we don't have access to those fields on PL/pgSQL, what was the
>> >> >> point
>> >> >> of the patch to begin with? Surely, accessing them from C wasn't
>> >> >> the
>> >> >> main use case?
>> >> >>
>> >> >
>> >> > These fields are available for application developers now. But is a
>> >> > true, so without this patch, GET STACKED DIAGNOSTICS statement will
>> >> > not be fully consistent, because some fields are accessible and
>> >> > others
>> >> > not
>> >>
>> >> there is one stronger argument for commit this patch now. With this
>> >> patch, we are able to wrote regression tests for new fields via
>> >> plpgsql.
>> >>
>> >> Regards
>> >>
>> >> Pavel
>> >>
>> >> >
>> >> > Pavel
>> >>
>> >>
>> >> --
>> >> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
>> >> To make changes to your subscription:
>> >> http://www.postgresql.org/mailpref/pgsql-hackers
>> >
>> >
>> >
>> >
>> > --
>> > Rushabh Lathia
>
>
>
>
> --
> Rushabh Lathia