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:[email protected]] > 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! _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
