maria-developers team mailing list archive

[Alexander Barkov <bar@xxxxxxxxxxx>]

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!