← Back to team overview

maria-developers team mailing list archive

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.

 # 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

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!


Follow ups