maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxxx>]

Hi, Oleksandr!

On Aug 29, Oleksandr Byelkin wrote:
> >> === modified file 'sql/sql_update.cc'
> >> --- a/sql/sql_update.cc	2012-04-26 17:21:37 +0000
> >> +++ b/sql/sql_update.cc	2012-08-27 14:29:26 +0000
> >> @@ -872,7 +872,7 @@ int mysql_update(THD *thd,
> >>     id= thd->arg_of_last_insert_id_function ?
> >>       thd->first_successful_insert_id_in_prev_stmt : 0;
> >>   
> >> -  if (error < 0)
> >> +  if (error < 0 && (!thd->is_error()))
> >>     {
> >>       char buff[MYSQL_ERRMSG_SIZE];
> >>       my_snprintf(buff, sizeof(buff), ER(ER_UPDATE_INFO), (ulong) found, (ulong) updated,
> > Nope, this is not good.
> > The bug happens because the error from remove_eq_conds() is ignored.
> > You need to catch and process this error as soon as possible.
> > While in your patch you let the update work all the way till the end.
> >
> When "update works all the way" it catched inside update loops, but was 
> that if update never make any update it ignores errors.
> 
> So actually it should be my fix or both fixes, because errors 
> (especially such unexpected as "out of resources" could happened almost 
> anywhere, so issuing OK to the client without checking error state is a 
> mistake.

I know. Before sending a review I actually tried to get a crash by
generating an error in different places of the UPDATE. 
E.g. in the loop:

INSERT t1 VALUES (1,2); UPDATE ....

or in fill_record():

UPDATE t1 SET a=COLUMN_CREATE(...);

or with double my_error():

... WHERE COLUMN_CREATE(...) OR COLUMN_CREATE(...);

all these cases were correctly taken care of, no crash, no ok packet.

So, I think, it's enough to fix only that early failure that you have found.

Regards,
Sergei