Bug#19995 (Extreneous table maps generated for statements that do not generate rows):

Switched to writing out table maps for tables that are locked when
the first row in a statement is seen.


mysql-test/include/master-slave.inc:
  Moved code to reset master and slave into separate file.
mysql-test/r/binlog_row_blackhole.result:
  Result change
mysql-test/r/binlog_row_mix_innodb_myisam.result:
  Result change
mysql-test/r/ndb_binlog_ignore_db.result:
  Result change
mysql-test/r/rpl_ndb_charset.result:
  Result change
mysql-test/r/rpl_row_basic_11bugs.result:
  Result change
mysql-test/r/rpl_row_charset.result:
  Result change
mysql-test/r/rpl_row_create_table.result:
  Result change
mysql-test/t/rpl_row_basic_11bugs.test:
  Added test to check that no events are generated when no rows are changed.
mysql-test/t/rpl_row_create_table.test:
  Master log position changed
sql/handler.cc:
  Adding function write_locked_table_maps() that will write table maps for all
  tables locked for write.
  Using "table->in_use" instead of "current_thd" since tables are now locked
  when the function is called.
  Removing old code to write table map.
sql/log_event.cc:
  Added assertion
sql/sql_class.cc:
  Removing code to write "dummy termination event".
sql/sql_class.h:
  Adding getter for binlog_table_maps.
sql/sql_insert.cc:
  Setting thd->lock before calling write_record for the execution of
  CREATE-SELECT and INSERT-SELECT since they keep multiple locks in the
  air at the same time.
mysql-test/include/master-slave-reset.inc:
  New BitKeeper file ``mysql-test/include/master-slave-reset.inc''

5 files changed, 109 insertions, 74 deletions

diff --git a/sql/handler.cc b/sql/handler.cc
index b9ef05a33c2..dfa3789f6a0 100644
--- a/sql/handler.cc
+++ b/sql/handler.cc
@@ -3199,6 +3199,58 @@ namespace {
   }
 }
 
+/**
+   Write table maps for all (manually or automatically) locked tables
+   to the binary log.
+
+   This function will generate and write table maps for all tables
+   that are locked by the thread 'thd'.  Either manually locked
+   (stored in THD::locked_tables) and automatically locked (stored in
+   THD::lock) are considered.
+   
+   See THD::lock and THD::locked_tables for more information.
+ */
+static int
+write_locked_table_maps(THD *thd)
+{
+  DBUG_ENTER("write_locked_table_maps");
+  DBUG_PRINT("enter", ("thd=%p, thd->lock=%p, thd->locked_tables=%p",
+                       thd, thd->lock, thd->locked_tables));
+
+  if (thd->get_binlog_table_maps() == 0)
+  {
+    /*
+      Exactly one table has to be locked, otherwise this code is not
+      guaranteed to work.
+    */
+    DBUG_ASSERT((thd->lock != NULL) + (thd->locked_tables != NULL) == 1);
+
+    MYSQL_LOCK *lock= thd->lock ? thd->lock : thd->locked_tables;
+    DBUG_ASSERT(lock->table_count > 0);
+    TABLE **const end_ptr= lock->table + lock->table_count;
+    for (TABLE **table_ptr= lock->table ; 
+         table_ptr != end_ptr ;
+         ++table_ptr)
+    {
+      TABLE *const table= *table_ptr;
+      DBUG_PRINT("info", ("Checking table %s", table->s->table_name));
+      if (table->current_lock == F_WRLCK &&
+          check_table_binlog_row_based(thd, table))
+      {
+        int const has_trans= table->file->has_transactions();
+        int const error= thd->binlog_write_table_map(table, has_trans);
+        /*
+          If an error occurs, it is the responsibility of the caller to
+          roll back the transaction.
+        */
+        if (unlikely(error))
+          DBUG_RETURN(1);
+      }
+    }
+  }
+  DBUG_RETURN(0);
+}
+
 template<class RowsEventT> int binlog_log_row(TABLE* table,
                                               const byte *before_record,
                                               const byte *after_record)
@@ -3206,7 +3258,7 @@ template<class RowsEventT> int binlog_log_row(TABLE* table,
   if (table->file->is_injective())
     return 0;
   bool error= 0;
-  THD *const thd= current_thd;
+  THD *const thd= table->in_use;
 
   if (check_table_binlog_row_based(thd, table))
   {
@@ -3215,17 +3267,26 @@ template<class RowsEventT> int binlog_log_row(TABLE* table,
     uint32 bitbuf[BITMAP_STACKBUF_SIZE/sizeof(uint32)];
     uint n_fields= table->s->fields;
     my_bool use_bitbuf= n_fields <= sizeof(bitbuf)*8;
+
+    /*
+      If there are no table maps written to the binary log, this is
+      the first row handled in this statement. In that case, we need
+      to write table maps for all locked tables to the binary log.
+    */
     if (likely(!(error= bitmap_init(&cols,
                                     use_bitbuf ? bitbuf : NULL,
                                     (n_fields + 7) & ~7UL,
                                     false))))
     {
       bitmap_set_all(&cols);
-      error=
-        RowsEventT::binlog_row_logging_function(thd, table,
-                                                table->file->has_transactions(),
-                                                &cols, table->s->fields,
-                                                before_record, after_record);
+      if (likely(!(error= write_locked_table_maps(thd))))
+      {
+        error=
+          RowsEventT::binlog_row_logging_function(thd, table,
+                                                  table->file->has_transactions(),
+                                                  &cols, table->s->fields,
+                                                  before_record, after_record);
+      }
       if (!use_bitbuf)
         bitmap_free(&cols);
     }
@@ -3251,41 +3312,6 @@ int handler::ha_external_lock(THD *thd, int lock_type)
   int error;
   if (unlikely(error= external_lock(thd, lock_type)))
     DBUG_RETURN(error);
-#ifdef HAVE_ROW_BASED_REPLICATION
-  if (table->file->is_injective())
-    DBUG_RETURN(0);
-
-  /*
-    There is a number of statements that are logged statement-based
-    but call external lock. For these, we do not need to generate a
-    table map.
-
-    TODO: The need for this switch is an indication that the model for
-    locking combined with row-based replication needs to be looked
-    over. Ideally, no such special handling should be needed.
-   */
-  switch (thd->lex->sql_command) {
-  case SQLCOM_TRUNCATE:
-  case SQLCOM_ALTER_TABLE:
-  case SQLCOM_OPTIMIZE:
-  case SQLCOM_REPAIR:
-    DBUG_RETURN(0);
-  default:
-    break;
-  }
-
-  /*
-    If we are locking a table for writing, we generate a table map.
-    For all other kinds of locks, we don't do anything.
-   */
-  if (lock_type == F_WRLCK && check_table_binlog_row_based(thd, table))
-  {
-    int const has_trans= table->file->has_transactions();
-    error= thd->binlog_write_table_map(table, has_trans);
-    if (unlikely(error))
-      DBUG_RETURN(error);
-  }
-#endif
   DBUG_RETURN(0);
 }
 
diff --git a/sql/log_event.cc b/sql/log_event.cc
index ab9fa2975a1..e78a6fc5865 100644
--- a/sql/log_event.cc
+++ b/sql/log_event.cc
@@ -5602,6 +5602,7 @@ int Rows_log_event::exec_event(st_relay_log_info *rli)
 bool Rows_log_event::write_data_header(IO_CACHE *file)
 {
   byte buf[ROWS_HEADER_LEN];	// No need to init the buffer
+  DBUG_ASSERT(m_table_id != ~0UL);
   DBUG_EXECUTE_IF("old_row_based_repl_4_byte_map_id_master",
                   {
                     int4store(buf + 0, m_table_id);
diff --git a/sql/sql_class.cc b/sql/sql_class.cc
index 916bcd94e0e..b0f399b3c12 100644
--- a/sql/sql_class.cc
+++ b/sql/sql_class.cc
@@ -2624,31 +2624,6 @@ int THD::binlog_flush_pending_rows_event(bool stmt_end)
 
     error= mysql_bin_log.flush_and_set_pending_rows_event(this, 0);
   }
-  else if (stmt_end && binlog_table_maps > 0)
-  {                      /* there is no pending event at this point */
-    /*
-      If pending is null and we are going to end the statement, we
-      have to write an extra, empty, binrow event so that the slave
-      knows to discard the tables it has received.  Otherwise, the
-      table maps written this far will be included in the table maps
-      for the following statement.
-
-      TODO: Remove the need for a dummy event altogether.  It can be
-      fixed if we can write table maps to a memory buffer before
-      writing the first binrow event.  We can then flush and clear the
-      memory buffer with table map events before writing the first
-      binrow event.  In the event of a crash, nothing is lost since
-      the table maps are only needed if there are binrow events.
-    */
-
-    Rows_log_event *ev=
-      new Write_rows_log_event(this, 0, ~0UL, 0, FALSE);
-    ev->set_flags(Rows_log_event::STMT_END_F);
-    binlog_set_pending_rows_event(ev);
-
-    error= mysql_bin_log.flush_and_set_pending_rows_event(this, 0);
-    binlog_table_maps= 0;
-  }
 
   DBUG_RETURN(error);
 }
diff --git a/sql/sql_class.h b/sql/sql_class.h
index 0135763b3b2..11225bb85fa 100644
--- a/sql/sql_class.h
+++ b/sql/sql_class.h
@@ -941,16 +941,18 @@ public:
   int binlog_flush_pending_rows_event(bool stmt_end);
   void binlog_delete_pending_rows_event();
 
-private:
 #ifdef HAVE_ROW_BASED_REPLICATION
+private:
   uint binlog_table_maps; // Number of table maps currently in the binlog
-#endif /* HAVE_ROW_BASED_REPLICATION */
-
 public:
-
+  uint get_binlog_table_maps() const {
+    return binlog_table_maps;
+  }
+#endif /* HAVE_ROW_BASED_REPLICATION */
 #endif /* MYSQL_CLIENT */
 
 #ifndef MYSQL_CLIENT
+public:
   enum enum_binlog_query_type {
       /*
         The query can be logged row-based or statement-based
@@ -1572,6 +1574,9 @@ class select_insert :public select_result_interceptor {
   bool send_eof();
   /* not implemented: select_insert is never re-used in prepared statements */
   void cleanup();
+
+protected:
+  MYSQL_LOCK *lock;
 };
 
 
@@ -1581,7 +1586,6 @@ class select_create: public select_insert {
   List<create_field> *extra_fields;
   List<Key> *keys;
   HA_CREATE_INFO *create_info;
-  MYSQL_LOCK *lock;
   Field **field;
 public:
   select_create (TABLE_LIST *table,
@@ -1590,8 +1594,7 @@ public:
 		 List<Key> &keys_par,
 		 List<Item> &select_fields,enum_duplicates duplic, bool ignore)
     :select_insert (NULL, NULL, &select_fields, 0, 0, duplic, ignore), create_table(table),
-    extra_fields(&fields_par),keys(&keys_par), create_info(create_info_par),
-    lock(0)
+    extra_fields(&fields_par),keys(&keys_par), create_info(create_info_par)
     {}
   int prepare(List<Item> &list, SELECT_LEX_UNIT *u);
   
diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc
index aed8b12d2c1..8dee3b054b3 100644
--- a/sql/sql_insert.cc
+++ b/sql/sql_insert.cc
@@ -2152,6 +2152,7 @@ select_insert::select_insert(TABLE_LIST *table_list_par, TABLE *table_par,
                              bool ignore_check_option_errors)
   :table_list(table_list_par), table(table_par), fields(fields_par),
    last_insert_id(0),
+   lock(0),
    insert_into_view(table_list_par && table_list_par->view != 0)
 {
   bzero((char*) &info,sizeof(info));
@@ -2348,7 +2349,36 @@ bool select_insert::send_data(List<Item> &values)
       DBUG_RETURN(1);
     }
   }
-  if (!(error= write_record(thd, table, &info)))
+
+  /*
+    The thd->lock lock contain the locks for the select part of the
+    statement and the 'lock' variable contain the write lock for the
+    currently locked table that is being created or inserted
+    into. However, the row-based replication will investigate the
+    thd->lock to decide what table maps are to be written, so this one
+    has to contain the tables locked for writing.  To be able to write
+    table map for the table being created, we temporarily set
+    THD::lock to select_insert::lock while writing the record to the
+    storage engine. We cannot set this elsewhere, since the execution
+    of a stored function inside the select expression might cause the
+    lock structures to be NULL.
+   */
+  
+  {
+    MYSQL_LOCK *saved_lock= NULL;
+    if (lock)
+    {
+      saved_lock= thd->lock;
+      thd->lock= lock;
+    }
+
+    error= write_record(thd, table, &info);
+    
+    if (lock)
+      thd->lock= saved_lock;
+  }
+
+  if (!error)
   {
     if (table->triggers || info.handle_duplicates == DUP_UPDATE)
     {