List:Commits« Previous MessageNext Message »
From:He Zhenxing Date:September 20 2010 8:42am
Subject:Re: bzr commit into mysql-5.1-bugteam branch (Li-Bing.Song:3489)
Bug#56226
View as plain text  
Dmitry Lenev wrote:
> 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.
> 

Yes, you're right, prepared statement would fail if reuse the old
table_map_id.

> -- 
> Dmitry Lenev, Software Developer
> Oracle Development SPB/MySQL, www.mysql.com
> 
> Are you MySQL certified?  http://www.mysql.com/certification
> 


Thread
bzr commit into mysql-5.1-bugteam branch (Li-Bing.Song:3489) Bug#56226Li-Bing.Song16 Sep
  • Re: bzr commit into mysql-5.1-bugteam branch (Li-Bing.Song:3489)Bug#56226He Zhenxing17 Sep
  • Re: bzr commit into mysql-5.1-bugteam branch (Li-Bing.Song:3489)Bug#56226Dmitry Lenev20 Sep
Re: bzr commit into mysql-5.1-bugteam branch (Li-Bing.Song:3489)Bug#56226Libing Song20 Sep
  • Re: bzr commit into mysql-5.1-bugteam branch (Li-Bing.Song:3489)Bug#56226Dmitry Lenev20 Sep
    • Re: bzr commit into mysql-5.1-bugteam branch (Li-Bing.Song:3489)Bug#56226Libing Song20 Sep
Re: bzr commit into mysql-5.1-bugteam branch (Li-Bing.Song:3489)Bug#56226Dmitry Lenev20 Sep
  • Re: bzr commit into mysql-5.1-bugteam branch (Li-Bing.Song:3489)Bug#56226He Zhenxing20 Sep