← Back to team overview

maria-developers team mailing list archive

Re: bb-10.2-compatibility

 

Hello Alexander,
I will start by doing a separate patch for corrected yylineno.
Thanks you very much !

Jérôme.

> -----Message d'origine-----
> De : Alexander Barkov [mailto:bar@xxxxxxxxxxx]
> Envoyé : lundi 27 mars 2017 09:16
> À : jerome brauge
> Cc : MariaDB Developers (maria-developers@xxxxxxxxxxxxxxxxxxx)
> Objet : Re: bb-10.2-compatibility
> 
> Hello Jérôme,
> 
> 
> On 03/24/2017 09:00 PM, jerome brauge wrote:
> > Hello Alexander,
> >
> > On oracle when an error occurred inside a stored procedure, the error
> > message contains the stack trace  of calling procedures.
> >
> > It's very useful when you have tons of procedures.
> >
> > I wrote this little patch to do something like that by adding the
> > stack trace as "notes".
> >
> 
> Thank you! I like the patch.
> 
> 
> Minor suggestions on coding style:
> 
> when "if" goes inside another "if" or "for", please use parentheses:
> 
> 
> Please change:
> 
> + for (; state_map[(uchar) c] == MY_LEX_SKIP ; c= lip->yyGet())
> +   if (c == '\n') lip->yylineno++;
> 
> to:
> 
> + for (; state_map[(uchar) c] == MY_LEX_SKIP ; c= lip->yyGet()) {
> +   if (c == '\n')
> +     lip->yylineno++;
> + }
> 
> 
> 
> The same here:
> 
> +      if (i != NULL)
> +        if (m_qname.str != NULL)
> +          push_warning_printf(thd, Sql_condition::WARN_LEVEL_NOTE,
> +                              ER_SP_STACK_TRACE,
> +                              ER_THD(thd, ER_SP_STACK_TRACE),
> +                              i->m_lineno, m_qname.str);
> +        else
> +          push_warning_printf(thd, Sql_condition::WARN_LEVEL_NOTE,
> +                              ER_SP_STACK_TRACE,
> +                              ER_THD(thd, ER_SP_STACK_TRACE),
> +                              i->m_lineno, "anonymous block");
> 
> 
> 
> But I'd better do like this:
> 
> 
>  if (i != NULL)
>    push_warning_printf(thd, Sql_condition::WARN_LEVEL_NOTE,
>                        ER_SP_STACK_TRACE,
>                        ER_THD(thd, ER_SP_STACK_TRACE),
>                        i->m_lineno,
>                        m_qname.str != NULL ? m_qname.str :
>                                              "anonymous block");
> 
> 
> 
> >
> >
> > To show the correct lines number (according to result of show create
> > procedure) , I had to correct yylineno in sql_les.cc.
> 
> Looks like a bug, specific to some non-default sql_mode values that include
> INGORE_SPACE. sql_mode=ORACLE is affected.
> Thanks for fixing this. Perhaps it should go in a separate patch.
> 
> >
> > But now, some tests failed because the line number of errors have
> changed !
> 
> I run all test suites like this:
> 
> ./mtr --mem --reorder --force --max-test-fail=0 --parallel=6
> 
> 
> Only a few tests failed. Most of them should be recorded.
> 
> But in some tests something goes unexpectedly:
> 
> - rpl.rpl_slave_grp_exec  does not work at all
> 
> It fails with:
> 
> 2017-03-27 11:08:42 140352080532224 [Warning] Slave: At line 3 in
> test.tr2 Error_code: 4059
> 
> where 4059 is exactly ER_SP_STACK_TRACE in my build.
> So it seems some replication code gets confused with these new notes.
> Can you please check?
> 
> 
> - main.signal_demo3 removes "Error"s  and adds "Note"s instead:
> 
>  show warnings;
>  Level	Code	Message
> -Error	1644	Oops in proc_7
> +Note	4059	At line 4 in demo.proc_7
> 
>  ...
> 
>  show warnings;
>  Level	Code	Message
> -Error	1644	Oops in proc_9
> -Error	1644	Oops in proc_8
>  Error	1644	Oops in proc_7
> +Note	4059	At line 4 in demo.proc_7
>  Error	1644	Oops in proc_6
> +Note	4059	At line 4 in demo.proc_6
> 
> Looks wrong.
> 
> 
> 
> >
> >
> > What is your opinion on this patch ?
> >
> >
> >
> > Regards,
> >
> > Jérôme.
> >


References