maria-developers team mailing list archive

[jerome brauge <j.brauge@xxxxxxxxxxx>]

Hello Alexander,
Great news.
I've done requested changes and re-record some tests (error number has changed).
Pull request is submitted.

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!