Re: bzr commit into mysql-6.0-backup-myisam branch (cbell:2618) Bug#36749, Bug#36778, Bug#36782

[Rafal Somla]

Chuck,

Chuck Bell wrote:
> Rafal,
> 
> I have moved the Tables definition per your request but it does not compile.
> The command (*snap_table_list)[i] returns a type table_ref which is not
> compatible what I need to do. It needs to be converted to a pointer of type
> obs::Obj in order to build a TABLE_LIST. I need access to the tables member,
> not the table_ref member.
>

Arrgh - you are completely right and I was wrong - sorry for misleading you :(
  	
> The only way I found around this was to do the materialization myself. But
> that means repeating operations that have already been done and therefore
> wasting cycles. This is the code without Tables exposed. It's messy.
> Exposing Tables is much easier, cleaner, and avoids duplication of work. I
> will leave it as I had it in my original patch.
>

I agree that using Tables class is much better. However, instead of exposing 
Image_info::Tables class (and polluting Image_info public interface) it is 
better to make it a protected member. Then it will become a private member of 
Restore_info class and restore_table_data() will have access to that class as 
its friend.

This is implemented by the following change. All the rest of your patch remains 
the same.

=== modified file 'sql/backup/image_info.h'
--- sql/backup/image_info.h     2008-05-17 16:08:00 +0000
+++ sql/backup/image_info.h     2008-06-24 11:24:20 +0000
@@ -129,10 +129,10 @@ public: // public interface
    uint m_table_count;
    MEM_ROOT  mem_root;    ///< Memory root for storage of catalogue items.

- private:
-
    class Tables; ///< Implementation of Table_list interface.

+ private:
+
    Map<uint, Db>   m_dbs; ///< Pointers to Db instances.
    Map<uint, Ts>   m_ts_map; ///< Pointers to Ts instances.
    String    m_binlog_file; ///< To store binlog file name at VP time.
@@ -257,6 +257,8 @@ class Snapshot_info

    virtual ~Snapshot_info();

+  Image_info::Tables *get_table_list() { return &m_tables; }
+
   protected:

    version_t m_version; ///< Stores version number of the snapshot's format.


If you agree with this the patch is ok to push. Once more I'm sorry for creating 
extra work for you by my untested suggestion.

Rafal