List:Commits« Previous MessageNext Message »
From:Konstantin Osipov Date:August 12 2010 4:40pm
Subject:Re: bzr commit into mysql-5.5-bugfixing branch (jon.hauglid:3101)
Bug#54105
View as plain text  
* Jon Olav Hauglid <jon.hauglid@stripped> [10/08/12 20:32]:
>  3101 Jon Olav Hauglid	2010-08-12
>       Bug #54105 assert in MDL_context::release_locks_stored_before
>       
>       The problem was that SHOW CREATE EVENT released all metadata locks
>       held by the current transaction. This made any exisiting savepoints
>       invalid, triggering the assert when ROLLBACK TO SAVEPOINT later
>       was executed.
>       
>       This patch fixes the problem by making sure SHOW CREATE EVENT only
>       releases metadata locks acquired by the statement itself.
>       
>       Test case added to event_trans.test.

The patch is OK to push, see below.

> === modified file 'sql/event_db_repository.cc'
> --- a/sql/event_db_repository.cc	2010-07-27 10:25:53 +0000
> +++ b/sql/event_db_repository.cc	2010-08-12 15:47:00 +0000
> @@ -996,24 +996,27 @@ Event_db_repository::load_named_event(TH
>                                        LEX_STRING name, Event_basic *etn)
>  {
>    bool ret;
> -  TABLE *table= NULL;
>    ulong saved_mode= thd->variables.sql_mode;
> +  Open_tables_backup open_tables_backup;
> +  TABLE_LIST event_table;
>  
>    DBUG_ENTER("Event_db_repository::load_named_event");
>    DBUG_PRINT("enter",("thd: 0x%lx  name: %*s", (long) thd,
>                        (int) name.length, name.str));
>  
> +  event_table.init_one_table("mysql", 5, "event", 5, "event", TL_READ);
> +
>    /* Reset sql_mode during data dictionary operations. */
>    thd->variables.sql_mode= 0;
>  
> -  if (!(ret= open_event_table(thd, TL_READ, &table)))
> +  if (!(ret= open_system_tables_for_read(thd, &event_table,
> &open_tables_backup)))
>    {
> -    if ((ret= find_named_event(dbname, name, table)))
> +    if ((ret= find_named_event(dbname, name, event_table.table)))
>        my_error(ER_EVENT_DOES_NOT_EXIST, MYF(0), name.str);
> -    else if ((ret= etn->load_from_row(thd, table)))
> +    else if ((ret= etn->load_from_row(thd, event_table.table)))
>        my_error(ER_CANNOT_LOAD_FROM_TABLE, MYF(0), "event");
>  
> -    close_mysql_tables(thd);
> +    close_system_tables(thd, &open_tables_backup);

The reason load_named_event() uses close_mysql_tables() is that it
is used, in most cases, in contexts when there are no open tables
and no transactions. Only for SHOW CREATE EVENT we use it 
in a transactional context.

Thus your change is a bit of an overkill, we don't always need to
backup open tables.

But let it be, only please add a comment for
open_system_tables_for_read(): 

/*
  We don't use open_event_table() here to make sure that SHOW
  CREATE EVENT works properly in transactional context, and
  does not release transactional metadata locks when the
  event table is closed.
*/

-- 
Thread
bzr commit into mysql-5.5-bugfixing branch (jon.hauglid:3101) Bug#54105Jon Olav Hauglid12 Aug
  • Re: bzr commit into mysql-5.5-bugfixing branch (jon.hauglid:3101)Bug#54105Konstantin Osipov12 Aug