From: Date: October 24 2008 9:46am
Subject: Re: bzr commit into mysql-6.0-backup branch (oystein.grovlen:2712)
 Bug#38810
List-Archive: http://lists.mysql.com/commits/56967
Message-Id: <49017D38.2000708@sun.com>
MIME-Version: 1.0
Content-Type: text/plain; format=flowed; charset=UTF-8
Content-Transfer-Encoding: 7BIT

Good to push.

Oystein.Grovlen@stripped wrote:
> #At file:///home/oysteing/mysql/mysql-6.0-backup-38810/
> 
>  2712 oystein.grovlen@stripped	2008-10-21
>       BUG#38810 Backup: crash if trigger and event with same name
>       Issue: Server crashes when restoring a backup image of a database
>              with trigger and event with the same names.
>       Cause: Dependency table is indexed by name.  Hence, trigger is not added when
>              event with same name has already been inserted.
>       Fix: Add type id to the key for the dependency table.
> modified:
>   mysql-test/suite/backup/r/backup_triggers_and_events.result
>   mysql-test/suite/backup/t/backup_triggers_and_events.test
>   sql/backup/backup_info.cc
>   sql/backup/backup_info.h
> 
> per-file messages:
>   mysql-test/suite/backup/r/backup_triggers_and_events.result
>     Rename one trigger to test name collisions with events.
>   mysql-test/suite/backup/t/backup_triggers_and_events.test
>     Rename one trigger to test name collisions with events.
>   sql/backup/backup_info.cc
>     Add type info to keys for the dependency table.
>   sql/backup/backup_info.h
>     Add type parameter to get_dep_node so it so that type info can be added to the
>     Dep_node keys.
> === modified file 'mysql-test/suite/backup/r/backup_triggers_and_events.result'
> --- a/mysql-test/suite/backup/r/backup_triggers_and_events.result	2008-10-07 17:15:44 +0000
> +++ b/mysql-test/suite/backup/r/backup_triggers_and_events.result	2008-10-21 12:40:22 +0000
> @@ -34,7 +34,7 @@ CALL trg_msg(NEW.a);
>  CREATE TRIGGER before_upd BEFORE UPDATE ON t1 FOR EACH ROW 
>  CALL trg_msg(NEW.a);
>  ||
> -CREATE TRIGGER before_del BEFORE DELETE ON t1 FOR EACH ROW 
> +CREATE TRIGGER ev BEFORE DELETE ON t1 FOR EACH ROW 
>  CALL trg_msg(OLD.a);
>  ||
>  USE test||
> @@ -134,7 +134,7 @@ Definer	root@localhost
>  character_set_client	#
>  collation_connection	latin1_swedish_ci
>  Database Collation	latin1_swedish_ci
> -Trigger	before_del
> +Trigger	ev
>  Event	DELETE
>  Table	t1
>  Statement	CALL trg_msg(OLD.a)
> 
> === modified file 'mysql-test/suite/backup/t/backup_triggers_and_events.test'
> --- a/mysql-test/suite/backup/t/backup_triggers_and_events.test	2008-10-07 17:15:44 +0000
> +++ b/mysql-test/suite/backup/t/backup_triggers_and_events.test	2008-10-21 12:40:22 +0000
> @@ -79,7 +79,9 @@ CREATE TRIGGER before_upd BEFORE UPDATE 
>  CALL trg_msg(NEW.a);
>  ||
>  
> -CREATE TRIGGER before_del BEFORE DELETE ON t1 FOR EACH ROW 
> +# Create a trigger with the same name as an event to test that backup is
> +# able to handle that.  (Bug#38810)
> +CREATE TRIGGER ev BEFORE DELETE ON t1 FOR EACH ROW 
>  CALL trg_msg(OLD.a);
>  ||
>  
> 
> === modified file 'sql/backup/backup_info.cc'
> --- a/sql/backup/backup_info.cc	2008-10-14 12:08:56 +0000
> +++ b/sql/backup/backup_info.cc	2008-10-21 12:40:22 +0000
> @@ -240,7 +240,7 @@ struct Backup_info::Dep_node: public Sql
>    Dbobj *obj;
>    String key;
>  
> -  Dep_node(const ::String &db_name, const ::String &name);
> +  Dep_node(const ::String &db_name, const ::String &name, const obj_type type);
>    Dep_node(const Dep_node&);
>  
>    static uchar* get_key(const uchar *record, size_t *key_length, my_bool);
> @@ -250,14 +250,18 @@ struct Backup_info::Dep_node: public Sql
>  /**
>    Create an empty dependency list node for a given per-database object.
>  
> -  The object is identified by its name and the name of the database to which
> -  it belongs. 
> +  The object is identified by its name, the name of the database to
> +  which it belongs, and its type.
>   */ 
>  inline
> -Backup_info::Dep_node::Dep_node(const ::String &db_name, const ::String &name)
> +Backup_info::Dep_node::Dep_node(const ::String &db_name, const ::String &name,
> +                                const obj_type type)
>    :next(NULL), obj(NULL)
>  {
>    key.length(0);
> +  // Add type to make sure keys are unique even between different namespaces
> +  key.set_int(type, TRUE, &my_charset_bin);
> +  key.append("|");
>    key.append(db_name);
>    key.append(".");
>    key.append(name);
> @@ -952,13 +956,13 @@ int Backup_info::add_view_deps(obs::Obj 
>      Dep_node *n= NULL;
>      const ::String *name= bv->get_name();
>      const ::String *db_name= bv->get_db_name();
> -  
> +
>      DBUG_ASSERT(name); 
>      DBUG_ASSERT(db_name); 
>  
>      // Locate or create a dependency list node for the base view.
>  
> -    int res= get_dep_node(*db_name, *name, n);
> +    int res= get_dep_node(*db_name, *name, BSTREAM_IT_VIEW, n);
>  
>      if (res == get_dep_node_res::ERROR)
>        goto error;
> @@ -1074,7 +1078,7 @@ Backup_info::add_db_object(Db &db, const
>  
>    // Get a dep. list node for the object.  
>  
> -  int res= get_dep_node(db.name(), *name, n);
> +  int res= get_dep_node(db.name(), *name, type, n);
>    
>    if (res == get_dep_node_res::ERROR)
>    {
> @@ -1159,9 +1163,10 @@ Backup_info::add_db_object(Db &db, const
>   */ 
>  int Backup_info::get_dep_node(const ::String &db_name, 
>                                const ::String &name, 
> +                              const obj_type type,
>                                Dep_node* &node)
>  {
> -  Dep_node n(db_name, name);
> +  Dep_node n(db_name, name, type);
>    size_t klen;
>    uchar  *key= Dep_node::get_key((const uchar*)&n, &klen, TRUE);
>  
> 
> === modified file 'sql/backup/backup_info.h'
> --- a/sql/backup/backup_info.h	2008-08-20 13:23:10 +0000
> +++ b/sql/backup/backup_info.h	2008-10-21 12:40:22 +0000
> @@ -69,7 +69,8 @@ class Backup_info: public backup::Image_
>  
>    struct Dep_node;
>  
> -  int get_dep_node(const ::String&, const ::String&, Dep_node*&);
> +  int get_dep_node(const ::String&, const ::String&, const obj_type, 
> +                   Dep_node*&);
>    int add_to_dep_list(const obj_type, Dep_node*);
>  
>    struct get_dep_node_res 
> 
>