maria-developers team mailing list archive

[Alexey Botchkov <holyfoot@xxxxxxxxxxxx>]

> This looks too complex to me. Why would you extend the base
> Internal_error_handler class - no other error handler did that or needed
> that, fwiw.

No other handlers need it, but these methods are sometimes used
in places where we don't know the type of the handler.
So i think the code is nicer that way - otherwise we had to check
the type somehow manually.
And i don't think that adding three empty virtual functions complicates
the code that much.
 
> I'd simply push an error handler, store the first message there and
> access it in get_schema_tables_record() via THD::get_internal_handler().

That get_schema_tables_record is not the only place where these 
functions are used.
In the get_table_share_with_discover() for example we need the error number.
(the patch for the reference: 
http://lists.askmonty.org/pipermail/commits/2014-February/005931.html)

Also sql/sql_show.cc, make_table_name_list().

In these we can't be sure the handler is of 'our' type.
In some cases we have another handler above our so we have to implement the
THD::clear_any_error() that would be also not that nice without the
virtual methods.

Best regards.
HF


24.02.2014 23:59, Sergei Golubchik wrote:
> Hi, Holyfoot!
>
> On Feb 22, holyfoot@xxxxxxxxxxxx wrote:
> > revno: 4077
> > revision-id: holyfoot@xxxxxxxxxxxx-20140222173543-xkzlp5rpga41evur
> > parent: svoj@xxxxxxxxxxx-20140218065405-vw3bfhvhrfjsc6hh
> > committer: Alexey Botchkov <holyfoot@xxxxxxxxxxxx>
> > branch nick: mdev-4856
> > timestamp: Sat 2014-02-22 21:35:43 +0400
> > message:
> >    MDEV-4856 SQL_ERROR_LOG shows 1146 errors which didnt appear in mysql client.
> >        The fill_schema_table() function used to call get_table_share()
> >        for a table name in WHERE then clear the error list. That way
> >        plugins receive the superfluous error notification if it happens
> >        in it. Also the problem was that error handler didn't prevent
> >        the suppressed error message from logging anyway as the logging
> >        happens in THD::raise_condition before the handler call.
> >        Fixed by adding the Warnings_only_error_handler before the call.
> >        raise_condition() also fixed.  The Internal_error_handler class
> >        was exteded with virtual functions to handle storing the error
> >        message.
> This looks too complex to me. Why would you extend the base
> Internal_error_handler class - no other error handler did that or needed
> that, fwiw.
>
> I'd simply push an error handler, store the first message there and
> access it in get_schema_tables_record() via THD::get_internal_handler().