maria-developers team mailing list archive

[Oleksandr Byelkin <sanja@xxxxxxxxxxxxxxxx>]

Am 14.06.2018 um 14:19 schrieb Sergei Golubchik:
> Hi, Oleksandr!
>
> On Jun 14, Oleksandr Byelkin wrote:
> > Am 14.06.2018 um 11:44 schrieb Sergei Golubchik:
> > > On Jun 14, Oleksandr Byelkin wrote:
> > > > Am 13.06.2018 um 12:08 schrieb Sergei Golubchik:
> > > > > > diff --git a/sql/sql_table.cc b/sql/sql_table.cc
> > > > > > index 9e7973b745c..f3cb85f01d3 100644
> > > > > > --- a/sql/sql_table.cc
> > > > > > +++ b/sql/sql_table.cc
> > > > > > @@ -4894,7 +4894,13 @@ int create_table_impl(THD *thd,
> > > > > >         file= mysql_create_frm_image(thd, orig_db, orig_table_name, create_info,
> > > > > >                                      alter_info, create_table_mode, key_info,
> > > > > >                                      key_count, frm);
> > > > > > -    if (!file)
> > > > > > +    /*
> > > > > > +      We have to check thd->is_error() here because it can be set by
> > > > > > +      Item::val* for example, and before it will be cought accidentally by
> > > > > > +      Item_func::fix_fields() of the next call. Now we removed the check
> > > > > > +      from Item_func::fix_fields()
> > > > > > +    */
> > > > > and I still don't understand why do you need to check for thd->is_error()
> > > > > here
> > > > because it catch some other errors which never will be checked and
> > > > fixing this IMHO is other matter and in higher version.
> > > Some other errors like what?
> > like an error in val_* call due to some incompatibility of arguments
> > during  mysql_create_frm_image() as I remember. i.e. our tests will fail
> > without this check because the error left unchecked till it is too late.
> > I put TODO remove this tests, but without them it will not just work.
> Is there a test that fails without them?
yes, completely unrelated tests, before the error will be caught in 
fix_fields() which was produced for unrelated parts of the query (and 
only if we have Item_func there). We just have no other mechanisms to 
catch error during val_* than check thd->is_error().
> Wouldn't the original bug show up again? You're testing for
> thd->is_error() and it might be unrelated coming from some earlier
> error, right?

I think no, because it is on the upper level. I.e. we tried to generate 
.frm and checking if it succeed before starting preparing other parts of 
the query, and actually try to execute full query if .frm was not 
created  will lead eventually to crashes and other problems. in other 
words we stop execution when it is not too late because 1) it has no 
sens 2) it is impossible without success of the first part.
> Regards,
> Sergei
> Chief Architect MariaDB
> and security@xxxxxxxxxxx