List:Commits« Previous MessageNext Message »
From:rsomla Date:February 22 2007 7:25pm
Subject:bk commit into 5.1 tree (rafal:1.2358)
View as plain text  
Below is the list of changes that have just been committed into a local
5.1 repository of rafal. When rafal does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2007-02-22 20:25:02+01:00, rafal@quant.(none) +6 -0
  Some cosmetic and symplifying changes. 

  sql/backup.cc@stripped, 2007-02-22 20:24:58+01:00, rafal@quant.(none) +10 -22
    Simplifying implementation of Table class. 

  sql/backup.h@stripped, 2007-02-22 20:24:58+01:00, rafal@quant.(none) +161 -138
    Rename Engine::Backup and Engine::Restore classes to Backup_engine and Restore_engine.
    
    Changed implementation of Db_ref and Table_ref classes. Make them concrete clases 
    (no virtual functions).
    
    Changed interface of Table_list class - now Table_ref class is used for storing table references.

  sql/backup0.h@stripped, 2007-02-22 20:24:58+01:00, rafal@quant.(none) +1 -1
    Remove unused NO value from enum_return_codes.

  sql/backup_private.h@stripped, 2007-02-22 20:24:58+01:00, rafal@quant.(none) +4 -5
    Rename Engine::Backup and Engine::Restore classes to Backup_engine and Restore_engine.

  sql/backup_prototype.cc@stripped, 2007-02-22 20:24:58+01:00, rafal@quant.(none) +1 -1
    Adapt to changes in Table_list interface.

  sql/backup_util.cc@stripped, 2007-02-22 20:24:58+01:00, rafal@quant.(none) +2 -2
    Adapt to changes in Table_list interface.

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	rafal
# Host:	quant.(none)
# Root:	/ext/mysql/bk/backup/prototype

--- 1.5/sql/backup.cc	2007-02-22 20:25:09 +01:00
+++ 1.6/sql/backup.cc	2007-02-22 20:25:09 +01:00
@@ -15,7 +15,7 @@
  * 
  *************************************************/
     
-
+// FIXME
 #define DBUG_ERROR(E) { my_error(ER_NO,MYF(0),"ala","ala"); \
                          DBUG_RETURN(1); }
 
@@ -89,26 +89,20 @@
 // Table_ref implementation which gets table identity from a filled record
 // of I_S.TABLES table.
 // field positions must be synchronized with defs in sql_show.cc
-// FIXME: use more direct access to a record (and its fields)
 
-struct Table: public Table_ref
+class Table: public Table_ref
 {
-  struct Db: public Db_ref
-  { 
-    String m_name;
-    const String &name() const { return m_name; };
-    Db(TABLE *t) { t->field[1]->val_str(&m_name); };
-  } m_db;
-  
-  String         m_name;
+  String  m_db_name;
+  String  m_name;
   const ::handlerton   *m_hton;
 
-  const String &name() const { return m_name; };
-  const Db_ref &db() const { return m_db; };
-  
-  Table(TABLE *t): m_db(t) 
+ public:
+ 
+  Table(TABLE *t): Table_ref(m_db_name, m_name) 
   { 
     String engine_name;
+    
+    t->field[1]->val_str(&m_db_name);
     t->field[2]->val_str(&m_name);
     t->field[4]->val_str(&engine_name);
     
@@ -119,13 +113,7 @@
     
     m_hton= ::ha_resolve_by_name(::current_thd,&name_lex);
     DBUG_ASSERT(m_hton);
-    
-    // FIXME: temporary debug hack -- add backup engine per force
-    // FIXME: the backup engine of ARCHIVE doesn't work
-    
-    //const_cast< ::handlerton* >(m_hton)->get_backup_engine= 
-    //  (prototype::get_trivial_engine);
-  };    
+  }    
   
   const ::handlerton *hton() const
   { return m_hton; }

--- 1.5/sql/backup.h	2007-02-22 20:25:09 +01:00
+++ 1.6/sql/backup.h	2007-02-22 20:25:09 +01:00
@@ -16,6 +16,9 @@
 // Class Table_list is used to pass a list of tables to backup engine
 class Table_list;
 
+// Forward declarations
+class Backup_driver;
+class Restore_driver;
 
 /**
  * @class Engine
@@ -42,10 +45,6 @@
 {
   public:
     
-    class Backup;   ///< Backup driver class.
-    class Restore;  ///< Restore driver class.
-    
-    
     /// Return version of backup images created by this engine.
     virtual const version_t version() const =0;    
     
@@ -60,7 +59,7 @@
      * 
      * @return  Error code or <code>backup::OK</code> on success.  
      */ 
-    virtual result_t get_backup( const Table_list &tables, Backup* &eng ) =0;
+    virtual result_t get_backup(const Table_list &tables, Backup_driver* &drv) =0;
     
     /**
      * Create a restore driver.
@@ -74,8 +73,8 @@
      * 
      * @return  Error code or <code>backup::OK</code> on success.  
      */ 
-    virtual result_t get_restore( version_t version, 
-                                   const Table_list &tables, Restore* &eng ) =0;
+    virtual result_t get_restore(version_t version, 
+                                 const Table_list &tables, Restore_driver* &drv) =0;
     
     /**
      * Free any resources allocated by the backup engine.
@@ -92,6 +91,43 @@
 };
   
 
+/**
+ * @class Buffer
+ * 
+ * @brief Used for data transfers between backup kernel and backup/restore drivers.
+ * 
+ * Apart from allocated memory a Buffer object contains fields informing about its size 
+ * and holding other information.
+ * 
+ * As described in WL#3169 all blocks of backup image data are distributed among several
+ * streams corresponding to different tables being backed-up/restored. Field 
+ * <code>stream_no</code> contains the number of the stream to which the data belongs.
+ * 
+ * When backup driver fills buffers given by backup kernel with its backup image data it 
+ * sets <code>last</code> field to <code>TRUE</code> to indicate that this is the last
+ * block of data in a given stream. Backup kernel knows that all data has been sent if 
+ * it has received "last" blocks for all the streams forming backup image.
+ * 
+ * @note Backup driver indicates how much data it has put into the buffer by updating
+ * its <code>size</code> field.
+ */ 
+struct Buffer
+{
+  size_t  size;       ///< size of the buffer (of memory block pointed by data).
+  uint    stream_no;  ///< Number of the stream to which data in the buffer belongs.
+  bool    last;       ///< <code>TRUE</code> if this is last block of data in the stream. 
+  byte    *data;      ///< Pointer to data area.
+
+  Buffer(): data(NULL),size(0),stream_no(0),last(TRUE) 
+  {}
+  
+  void reset(size_t len)
+  { 
+    size=      len; 
+    stream_no= 0; 
+    last=      TRUE; 
+  }
+};
 
 
 /**
@@ -150,47 +186,9 @@
     const Table_list &m_tables;
 };
 
-/**
- * @class Buffer
- * 
- * @brief Used for data transfers between backup kernel and backup/restore drivers.
- * 
- * Apart from allocated memory a Buffer object contains fields informing about its size 
- * and holding other information.
- * 
- * As described in WL#3169 all blocks of backup image data are distributed among several
- * streams corresponding to different tables being backed-up/restored. Field 
- * <code>stream_no</code> contains the number of the stream to which the data belongs.
- * 
- * When backup driver fills buffers given by backup kernel with its backup image data it 
- * sets <code>last</code> field to <code>TRUE</code> to indicate that this is the last
- * block of data in a given stream. Backup kernel knows that all data has been sent if 
- * it has received "last" blocks for all the streams forming backup image.
- * 
- * @note Backup driver indicates how much data it has put into the buffer by updating
- * its <code>size</code> field.
- */ 
-struct Buffer
-{
-  size_t  size;       ///< size of the buffer (of memory block pointed by data).
-  uint    stream_no;  ///< Number of the stream to which data in the buffer belongs.
-  bool    last;       ///< <code>TRUE</code> if this is last block of data in the stream. 
-  byte    *data;      ///< Pointer to data area.
-
-  Buffer(): data(NULL),size(0),stream_no(0),last(TRUE) 
-  {}
-  
-  void reset(size_t len)
-  { 
-    size=      len; 
-    stream_no= 0; 
-    last=      TRUE; 
-  }
-};
-
 
 /**
- * @class Engine::Backup
+ * @class Backup_driver
  * 
  * @brief Represents backup driver for backing-up a given list of tables.
  * 
@@ -199,13 +197,13 @@
  * method is <code>get_data()</code> which is used by the kernel to poll the 
  * backup image data and at the same time learn about state of the backup process.
  */ 
-class Engine::Backup: public Driver
+class Backup_driver: public Driver
 {
   public:
     
-    Backup(const Table_list &tables):Driver(tables) {};
+    Backup_driver(const Table_list &tables):Driver(tables) {};
     
-    virtual ~Backup() {}; // Each specific implementation will derive from this class.
+    virtual ~Backup_driver() {}; // Each specific implementation will derive from this class.
     
    /** 
     * @fn result_t get_data(Buffer &buf) 
@@ -364,7 +362,7 @@
 };
 
 /**
- * @class Engine::Restore
+ * @class Restore_driver
  * 
  * @brief Represents restore driver used for restoring a given list of tables.
  * 
@@ -373,12 +371,12 @@
  * method is <code>send_data()</code> which is used by the kernel to send  
  * backup image data to the driver. 
  */ 
-class Engine::Restore: public Driver
+class Restore_driver: public Driver
 {
   public:
     
-    Restore(const Table_list &tables):Driver(tables) {};
-    virtual ~Restore() {};    
+    Restore_driver(const Table_list &tables):Driver(tables) {};
+    virtual ~Restore_driver() {};    
     
    /** 
     * @fn result_t send_data(Buffer &buf) 
@@ -425,120 +423,145 @@
 
 
 //@{
-/// Functions used to implement BACKUP/RESTORE statements. They are called from sql_parse.cc
+/// Functions used to implement BACKUP/RESTORE SQL statements. 
+/// They are called from sql_parse.cc
 int do_backup(THD*);
 int do_restore(THD*);
 //@}
 
 
-/*
- *  This class is used to abstract away the way a table is identified inside mysql instance.
- *  Right now tables are identified using (table name, database name) pairs. If this changes
- *  in the future, implementation of Table_ref is the place to incorporate these changes
- *  into the backup code.
- */ 
-class Table_ref;
+//@{
 
-/**
- * @class Table_list
- * 
- * @brief Objects of this class store list of tables (just table references represented
- *        by Table_ref objects).
- * 
- * Contrary to the List class, this class allocates memory and stores elements of the
- * list, not just pointers to them.
- * 
- * Elements of the list can be accessed by index. E.g.
- * @code
- *  Table_list l;
- *  Table_ref  r = l[1];  // r refers to the second element of the list. 
- * @endcode
+/** 
+ * Classes Db_ref and Table_ref are used to identify databeses and tables 
+ * inside mysql server instance.
  * 
- * @note This is rather ad-hoc implementation and certainly can be improved/optimized.
+ * These classes abstract the way a table or database is identified inside mysqld, 
+ * so that when this changes (introduction of global db/table ids, introduction 
+ * of catalogs) it is easy to adapt backup code to the new identification schema.
+ * 
+ * Regardless of the internal representation, classes provide methods returning 
+ * db/table name as a String object. Also, each table belongs to some database 
+ * and a method returnig Db_ref object identifying this database is present. For 
+ * Db_ref objects there is catalog() method returning name of the catalog, but 
+ * currently it always returns null string.
+ * 
+ * Clases are implemented so that the memory for storing names can be allocated
+ * outside an instance. This is used in the Tables class implementing 
+ * Table_list interface to share space used for storing database names among 
+ * several Table_ref instances. 
+ * 
+ * Instances of Table_ref and Db_ref should be considered cheap to use, equivallent
+ * to using pointers or other base types. Currently, single instance of each class
+ * uses as much memory as a single pointer (+some external memory to store names 
+ * which can be shared among different instances). The methods are inlined to avoid
+ * function call costs.
  */
+
+class Db_ref
+{
+  const String &m_name;
   
-class Table_list
-{ 
-  public: 
+ public:
   
-    class Ref;
-   
-    /// Return reference to given list element. Elements are counted from 0.
-    virtual Ref operator[](uint pos) const =0;
-    
-    /// Return number of elements in the list.
-    virtual uint  count() const =0;
-};
-   
+  const String& name() const
+  { return m_name; }
+  
+  const String& catalog() const 
+  { return my_null_string; }
 
-/*
-class Opt_val
-{
  protected:
+ 
+  // Constructors are made protected as clients of this class are
+  // not supposed to create instances (see comment inside Table_ref)
+ 
+  Db_ref(const String &name): m_name(name)
+  {}
   
-  bool valid;
+  Db_ref(const Db_ref &db): m_name(db.m_name) 
+  {}
   
- public:
- 
-  Opt_val(bool not_null=FALSE): valid(not_null) {}; 
-  virtual operator bool() const { return valid; };
+  friend class Table_ref;
 };
-*/
-
-//@{
 
-/** 
- * Objects of type Db_ref and Table_ref are used to identify databeses and tables, 
- * respectively, inside mysql server instance.
- * 
- * Current, ad-hoc implementation, uses table name together with a database reference
- * to identify a table. Database is identified by its name (catalog is empty but can
- * be introduced in future versions).
- */  
-
-
-struct Db_ref
-{
-  virtual const String &name()    const =0;
-  virtual const String &catalog() const { return my_null_string; };
-};
 
-struct Table_ref
+class Table_ref
 {
-  virtual const Db_ref &db()   const =0;
-  virtual const String &name() const =0;
-};
-//@}
-
-
-// Table_ref object which doesn't store strings.
-
-class Table_list::Ref: public Table_ref
-{
-  struct Db: public Db_ref
-  { 
-    const String &m_name;
-    const String &name() const { return m_name; };
-    Db(const String &s): m_name(s) {};
-  } m_db;
-  
+  const Db_ref  m_db;
+  const String  &m_name;
+ 
  public: 
+ 
+  const Db_ref& db() const 
+  { return m_db; }
   
-  const String &m_name;
+  const String& name() const
+  { return m_name; }
+  
+  // Produce string identifying the table (e.g. for error reporting)
+  //operator String() const;
+  
+ protected:
 
-  const String &name() const { return m_name; };
-  const Db_ref &db()   const { return m_db; };
+  /*
+    Constructor is made protected as it should not be used by
+    clients of this class -- they obtain already constructed
+    instances from the backup kernel via Table_list object passed
+    when creating backup/restore driver.
+  */
+ 
+  Table_ref(const String &db, const String &name): 
+    m_db(db), m_name(name)
+  {}  
   
-  Ref(const String &db, const String &nm): m_db(db), m_name(nm) {};
+  friend class Tables; // internal implementation of Table_list interface
 };
 
 
+//@}
 
 
+/**
+ * @class Table_list
+ * 
+ * @brief This abstract class defines interface used to access a list of
+ * tables (e.g. when such a list is passed to a backup/restore driver).
+ * 
+ * Elements of the list can be accessed by index, counting from 0. E.g.
+ * @code
+ *  Table_list &tables;
+ *  Table_ref  t2 = tables[1];  // t2 refers to the second element of the list. 
+ * @endcode
+ * 
+ * Interface is made abstract, so that different implementations can be
+ * used in the backup code. For example it is possible to create a class which 
+ * adds this interface to a list of tables represented by a linked list of
+ * TABLE_LIST structures as used elsewhere in the code. On the other hand, much 
+ * more space efficient implementations are possible, as for each table we need 
+ * to store only table's identity (db/table name). In any case, the interface 
+ * to the list reamins the same, as defined by this class.
+ * 
+ * TODO: add iterators.
+ */
+  
+class Table_list
+{ 
+  public: 
+  
+    /// Return reference to given list element. Elements are counted from 0.
+    virtual Table_ref operator[](uint pos) const =0;
+    
+    /// Return number of elements in the list.
+    virtual uint  count() const =0;
+};
+   
+   
 } // backup namespace
 
-typedef backup::Engine::Backup   Backup_driver;
-typedef backup::Engine::Restore  Restore_driver;
+// export Backup/Restore_driver classes to global namespace
+
+using backup::Backup_driver;
+using backup::Restore_driver;
 
    
 #endif

--- 1.2/sql/backup0.h	2007-02-22 20:25:09 +01:00
+++ 1.3/sql/backup0.h	2007-02-22 20:25:09 +01:00
@@ -15,7 +15,7 @@
 typedef uint version_t; 
 typedef int  result_t; 
   
-enum enum_return_codes { OK=0, ERROR, NO, WAIT, NOT_NOW, INIT_DONE, READY, END_BLOCK };  
+enum enum_return_codes { OK=0, ERROR, WAIT, NOT_NOW, INIT_DONE, READY, END_BLOCK };  
 
 class Engine;
 

--- 1.1/sql/backup_private.h	2007-02-22 20:25:09 +01:00
+++ 1.2/sql/backup_private.h	2007-02-22 20:25:09 +01:00
@@ -74,11 +74,11 @@
    const version_t version() const 
    { return 1; }    
   
-   result_t get_backup( const Table_list &tables, Backup* &eng )
+   result_t get_backup( const Table_list &tables, Backup_driver* &drv )
    { return ERROR; }
 
    result_t get_restore( version_t version, 
-                         const Table_list &tables, Restore* &eng )
+                         const Table_list &tables, Restore_driver* &drv )
    { return ERROR; }
    
    static Trivial_engine instance;
@@ -119,12 +119,11 @@
     int  add(const StringPool::Key &db, const String &name); 
     
     /// Return reference to given list element. Elements are counted from 0.
-    Ref operator[](uint pos) const;
+    Table_ref operator[](uint pos) const;
     
     /// Return number of elements in the list.
     uint  count() const 
     { return m_count; }
-
    
   private:
    
@@ -143,7 +142,7 @@
     uint m_count;
 
     StringPool  dbnames;
-      
+
     friend stream_result::value operator<<(OStream &str, const Tables &tl);
     friend stream_result::value operator>>(IStream &str, Tables &tl);
 };

--- 1.1/sql/backup_prototype.cc	2007-02-22 20:25:09 +01:00
+++ 1.2/sql/backup_prototype.cc	2007-02-22 20:25:09 +01:00
@@ -327,7 +327,7 @@
     DBUG_ASSERT(ptr);
     bzero(ptr,sizeof(TABLE_LIST));
 
-    Tables::Ref tbl= tables[tno];
+    Table_ref tbl= tables[tno];
 
     ptr->alias= ptr->table_name= const_cast<char*>(tbl.name().ptr());
     ptr->db= const_cast<char*>(tbl.db().name().ptr());

--- 1.1/sql/backup_util.cc	2007-02-22 20:25:10 +01:00
+++ 1.2/sql/backup_util.cc	2007-02-22 20:25:10 +01:00
@@ -104,7 +104,7 @@
 } 
 
 
-Table_list::Ref Tables::operator[](uint pos) const
+Table_ref Tables::operator[](uint pos) const
 {
   DBUG_ASSERT(pos < m_count);
   
@@ -122,7 +122,7 @@
                         dbnames[ptr->db].ptr(), ptr->name.ptr() ));
   */
   
-  return Ref(dbnames[ptr->db],ptr->name);
+  return Table_ref(dbnames[ptr->db],ptr->name);
 }
 
 
Thread
bk commit into 5.1 tree (rafal:1.2358)rsomla22 Feb