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@mariadb.org] 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!