Hello!
* Dmitry Lenev <Dmitry.Lenev@stripped> [10/09/20 10:53]:
> Here are my comment about your patch:
>
> * Li-Bing.Song@stripped <Li-Bing.Song@stripped> [10/09/16 13:40]:
> > #At file:///home/anders/work/bzrwork1/wt2/mysql-5.1-bugteam/ based on
> revid:mattias.jonsson@stripped
> >
> > 3489 Li-Bing.Song@stripped 2010-09-16
> > Bug#56226 Table map set to 0 after altering MyISAM table
> >
> > A MyISAM table's table_map_id will always be reset to 0 after altered
> > only its metadata. 0 is a valid value of table_map_id. But the problem
> will
> > cause that more than one tables have the same table_map_id 0. If more
> than
> > one tables which's table_map_id are 0 are updated in one statement, the
> update
> > of these different tables will be write into only one rows event. It will
> > cause slave server to crash.
> >
> > After this patch, table_map_id always be refreshed as a correct value
> after
> > altered only its metadata.
>
> ...
>
> > === modified file 'sql/sql_base.cc'
> > --- a/sql/sql_base.cc 2010-07-20 18:07:36 +0000
> > +++ b/sql/sql_base.cc 2010-09-16 09:35:41 +0000
> > @@ -662,6 +662,7 @@ void close_handle_and_leave_table_as_loc
> > share->set_table_cache_key(key_buff, old_share->table_cache_key.str,
> > old_share->table_cache_key.length);
> > share->tmp_table= INTERNAL_TMP_TABLE; // for intern_close_table()
> > + share->table_map_id= ~0UL;
> > }
> >
> > /*
> > @@ -3082,7 +3083,8 @@ bool reopen_table(TABLE *table)
> > tmp.maybe_null= table->maybe_null;
> > tmp.status= table->status;
> >
> > - tmp.s->table_map_id= table->s->table_map_id;
> > + if (table->s->table_map_id != ~0UL)
> > + tmp.s->table_map_id= table->s->table_map_id;
> >
>
> I don't think that there is a situation when it makes sense to
> inherit table_map_id from old version of share here.
> Especially since, AFAIU, shares for TABLE objects for which
> reopen_table() is called are always dummy and not a real shares.
>
> So I think it makes sense simply to remove this line and keep
> close_handle_and_leave_table_as_lock() as is.
After reading comment from He Zhenxing I've decided that my comment
needs clarification:
Actually, I think that inheriting table_map_id in reopen_table() from
old share is ALWAYS WRONG. We are opening a new share which may
correspond to new version of structure and therefore new table_map_id
is needed (otherwise e.g. invalidation of prepared statements may break,
I also suspect that this might cause problems for replication).
Because of this I think that He Zhenxing's proposal to preserve
table_map_id value from old share in close_handle_and_leave_table_as_lock()
is a bad idea.
--
Dmitry Lenev, Software Developer
Oracle Development SPB/MySQL, www.mysql.com
Are you MySQL certified? http://www.mysql.com/certification
