maria-developers team mailing list archive

[Alexander Barkov <bar@xxxxxxxxxxx>]

Hi Jerome,


On 05/22/2017 07:07 PM, jerome brauge wrote:
> Hello Alexander,
> Great news.
> I've done requested changes and re-record some tests (error number has changed).
> Pull request is submitted.

I merged it yesterday. Thank you very much!

> Thank!
>
> > -----Message d'origine-----
> > De : Alexander Barkov [mailto:bar@xxxxxxxxxxx]
> > Envoyé : lundi 22 mai 2017 14:13
> > À : jerome brauge; maria-developers; Monty
> > Objet : review for SP stack trace
> >
> > Hello Jerome,
> >
> > here's Monty's review on your stack trace patch.
> >
> > Thank you very much. We're sorry for delay.
> >
> >
> > > diff --git a/mysql-test/r/commit_1innodb.result
> > > b/mysql-test/r/commit_1innodb.result
> > > index 1adba7b..8e40ec0 100644
> > > <cut>
> > >
> > > --- a/mysql-test/r/get_diagnostics.result
> > > +++ b/mysql-test/r/get_diagnostics.result
> > > @@ -595,7 +595,7 @@ SELECT var, @var;
> > >   END|
> > >   CALL p1();
> > >   var	@var
> > > -1	1
> > > +2	2
> > >   DROP PROCEDURE p1;
> > >
> > > What is the reason for this going from 1 to 2 ?
> > > I would have understood if there would have been some warnings from
> > > before in the code, but there wasn't.
> > >
> > > Does this come from the error from 'call p1()' earlier that does now
> > > have a new 'note message'.
> > >
> > > If yes, it would be good to make this clear by adding a 'show
> > > warnings' just before the above create procedure p1() line.
>
> You are right. There is a new note message in the previous call of p1.
> +SHOW WARNINGS;
> +Level  Code    Message
> +Error  54321   MESSAGE_TEXT text
> +Note   4069    At line 16 in test.p1
>
>
> > >   # Setting TABLE_NAME is currently not implemented.
> > > diff --git a/mysql-test/r/signal.result b/mysql-test/r/signal.result
> > > index f05e357..a796cc6 100644
> > > --- a/mysql-test/r/signal_demo3.result
> > > +++ b/mysql-test/r/signal_demo3.result
> > > @@ -79,14 +79,23 @@ show warnings;
> > > @@ -95,6 +104,4 @@ call proc_1();
> > >   ERROR 45000: Oops in proc_1
> > >   show warnings;
> > >   Level	Code	Message
> > > -Error	1644	Oops in proc_5
> > > -Error	1644	Oops in proc_4
> > > -Error	1644	Oops in proc_3
> > > +Note	4068	At line 4 in demo.proc_3
> > >
> > > Why has 3 error disappered and why is there a line number note for
> > > demo.proc_3 but no error line?
> > >
> > > I assume this comes from the following line:
> > > SET @@session.max_error_count = 5
>
> Yes new notes pushed errors messages out of buffer.
>
> > > The question is if max_error_count's should really be counted for
> > > notes too, especially for "At line" notes.
> > > The documentation for the variable says "Specifies the maximum number
> > > of messages stored for display by"
> > >
> > > Still, the question is if we should include the 'At line' or not in counting.
> > > (Not important for this patch, but good to think about)
> > >
> > >
> > > --- a/sql/share/errmsg-utf8.txt
> > > +++ b/sql/share/errmsg-utf8.txt
> > > @@ -7484,3 +7484,5 @@ ER_UNKNOWN_SEQUENCES 42S02
> > >           eng "Unknown SEQUENCE: '%-.300s'"
> > >   ER_UNKNOWN_VIEW 42S02
> > >           eng "Unknown VIEW: '%-.300s'"
> > > +ER_SP_STACK_TRACE
> > > +        eng "At line %d in %s"
> > >
> > > Should be %u as lineno is uint
> > >
> > >
> > > Please add the "SHOW WARNINGS" and fix the error message and it's ok to
> > push.
> > > Thanks for a great addition to MariaDB!