List:Commits« Previous MessageNext Message »
From:Dao-Gang.Qu Date:December 17 2010 10:00am
Subject:bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3527) Bug#56662
View as plain text  
#At file:///home/daogang/bzrwork/bug56662/mysql-5.1-bugteam/ based on revid:luis.soares@stripped

 3527 Dao-Gang.Qu@stripped	2010-12-17
      Bug #56662  	Assertion failed: next_insert_id == 0, file .\handler.cc
      
      Normally, auto_increment value is generated for the column by
      inserting either NULL or 0 into it. NO_AUTO_VALUE_ON_ZERO
      suppresses this behavior for 0 so that only NULL generates
      the auto_increment value. This behavior is also followed by
      a slave, specifically by the SQL Thread, when applying events
      in the statement format from a master. However, when applying
      events in the row format, the flag was ignored thus causing
      an assertion failure:
      "Assertion failed: next_insert_id == 0, file .\handler.cc"
      
      In fact, we never need to generate a auto_increment value for
      the column when applying events in row format on slave. So we
      don't allow it to happen by using 'MODE_NO_AUTO_VALUE_ON_ZERO'.
      
      Refactoring: Get rid of all the sql_mode checks to rows_log_event
      when applying it for avoiding problems caused by the inconsistency
      of the sql_mode on slave and master as the sql_mode is not set for
      Rows_log_event.
     @ mysql-test/extra/rpl_tests/rpl_auto_increment.test
        Added test to verify if the assertion of "next_insert_id == 0"
        will fail in ha_external_lock() function.
     @ mysql-test/suite/rpl/r/rpl_auto_increment.result
        Test result for bug#56662.
     @ sql/log_event.cc
        Added code to not allow generation of auto_increment value when
        processing rows event by adding 'MODE_NO_AUTO_VALUE_ON_ZERO' to
        sql_mode.
     @ sql/rpl_record.cc
        Added code to get rid of the 'MODE_STRICT_TRANS_TABLES'  and
        MODE_STRICT_ALL_TABLES check to the table when appling the
        rows event and treat it as no strict.

    modified:
      mysql-test/extra/rpl_tests/rpl_auto_increment.test
      mysql-test/suite/rpl/r/rpl_auto_increment.result
      sql/log_event.cc
      sql/log_event.h
      sql/rpl_record.cc
      sql/rpl_record.h
=== modified file 'mysql-test/extra/rpl_tests/rpl_auto_increment.test'
--- a/mysql-test/extra/rpl_tests/rpl_auto_increment.test	2009-09-10 10:05:53 +0000
+++ b/mysql-test/extra/rpl_tests/rpl_auto_increment.test	2010-12-17 10:00:51 +0000
@@ -241,3 +241,29 @@ DROP TABLE t1;
 DROP TABLE t2;
 SET SQL_MODE='';
 sync_slave_with_master;
+
+#
+# BUG#56662
+# The test verifies if the assertion of "next_insert_id == 0"
+# will fail in ha_external_lock() function.
+#
+connection master;
+CREATE TABLE t1 (id SMALLINT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, data INT) ENGINE=innodb;
+
+BEGIN;
+--echo # Set sql_mode with NO_AUTO_VALUE_ON_ZERO for allowing
+--echo # zero to fill the auto_increment field.
+SET SQL_MODE=NO_AUTO_VALUE_ON_ZERO;
+INSERT INTO t1(id,data) VALUES(0,2);
+--echo # Resetting sql_mode without NO_AUTO_VALUE_ON_ZERO to
+--echo # affect the execution of the transaction on slave.
+SET SQL_MODE=0;
+COMMIT;
+SELECT * FROM t1;
+sync_slave_with_master;
+SELECT * FROM t1;
+
+connection master;
+DROP TABLE t1;
+sync_slave_with_master;
+

=== modified file 'mysql-test/suite/rpl/r/rpl_auto_increment.result'
--- a/mysql-test/suite/rpl/r/rpl_auto_increment.result	2009-09-10 10:05:53 +0000
+++ b/mysql-test/suite/rpl/r/rpl_auto_increment.result	2010-12-17 10:00:51 +0000
@@ -312,3 +312,20 @@ Comparing tables master:test.t2 and slav
 DROP TABLE t1;
 DROP TABLE t2;
 SET SQL_MODE='';
+CREATE TABLE t1 (id SMALLINT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, data INT) ENGINE=innodb;
+BEGIN;
+# Set sql_mode with NO_AUTO_VALUE_ON_ZERO for allowing
+# zero to fill the auto_increment field.
+SET SQL_MODE=NO_AUTO_VALUE_ON_ZERO;
+INSERT INTO t1(id,data) VALUES(0,2);
+# Resetting sql_mode without NO_AUTO_VALUE_ON_ZERO to
+# affect the execution of the transaction on slave.
+SET SQL_MODE=0;
+COMMIT;
+SELECT * FROM t1;
+id	data
+0	2
+SELECT * FROM t1;
+id	data
+0	2
+DROP TABLE t1;

=== modified file 'sql/log_event.cc'
--- a/sql/log_event.cc	2010-12-14 16:43:25 +0000
+++ b/sql/log_event.cc	2010-12-17 10:00:51 +0000
@@ -7570,6 +7570,14 @@ int Rows_log_event::do_apply_event(Relay
     // Do event specific preparations 
     error= do_before_row_operations(rli);
 
+    /*
+      Bug#56662 Assertion failed: next_insert_id == 0, file handler.cc
+      Don't allow generation of auto_increment value when processing
+      rows event by setting 'MODE_NO_AUTO_VALUE_ON_ZERO'.
+    */
+    ulong saved_sql_mode= thd->variables.sql_mode;
+    thd->variables.sql_mode= MODE_NO_AUTO_VALUE_ON_ZERO;
+
     // row processing loop
 
     while (error == 0 && m_curr_row < m_rows_end)
@@ -7632,6 +7640,11 @@ int Rows_log_event::do_apply_event(Relay
           thd->transaction.stmt.modified_non_trans_table= TRUE;
     } // row processing loop
 
+    /*
+      Restore the sql_mode after the rows event is processed.
+    */
+    thd->variables.sql_mode= saved_sql_mode;
+
     DBUG_EXECUTE_IF("STOP_SLAVE_after_first_Rows_event",
                     const_cast<Relay_log_info*>(rli)->abort_slave= 1;);
 
@@ -8601,16 +8614,11 @@ Rows_log_event::write_row(const Relay_lo
   int UNINIT_VAR(keynum);
   auto_afree_ptr<char> key(NULL);
 
-  /* fill table->record[0] with default values */
-  bool abort_on_warnings= (rli->sql_thd->variables.sql_mode &
-                           (MODE_STRICT_TRANS_TABLES | MODE_STRICT_ALL_TABLES));
-  if ((error= prepare_record(table, m_width,
-                             table->file->ht->db_type != DB_TYPE_NDBCLUSTER,
-                             abort_on_warnings, m_curr_row == m_rows_buf)))
-    DBUG_RETURN(error);
-  
+  prepare_record(table, m_width,
+                 table->file->ht->db_type != DB_TYPE_NDBCLUSTER);
+
   /* unpack row into table->record[0] */
-  if ((error= unpack_current_row(rli, abort_on_warnings)))
+  if ((error= unpack_current_row(rli)))
     DBUG_RETURN(error);
 
   if (m_curr_row == m_rows_buf)
@@ -9454,11 +9462,9 @@ Update_rows_log_event::do_exec_row(const
 
   store_record(m_table,record[1]);
 
-  bool abort_on_warnings= (rli->sql_thd->variables.sql_mode &
-                           (MODE_STRICT_TRANS_TABLES | MODE_STRICT_ALL_TABLES));
   m_curr_row= m_curr_row_end;
   /* this also updates m_curr_row_end */
-  if ((error= unpack_current_row(rli, abort_on_warnings)))
+  if ((error= unpack_current_row(rli)))
     return error;
 
   /*

=== modified file 'sql/log_event.h'
--- a/sql/log_event.h	2010-10-19 22:36:59 +0000
+++ b/sql/log_event.h	2010-12-17 10:00:51 +0000
@@ -3583,16 +3583,13 @@ protected:
   int write_row(const Relay_log_info *const, const bool);
 
   // Unpack the current row into m_table->record[0]
-  int unpack_current_row(const Relay_log_info *const rli,
-                         const bool abort_on_warning= TRUE)
-  { 
+  int unpack_current_row(const Relay_log_info *const rli)
+  {
     DBUG_ASSERT(m_table);
 
-    bool first_row= (m_curr_row == m_rows_buf);
     ASSERT_OR_RETURN_ERROR(m_curr_row < m_rows_end, HA_ERR_CORRUPT_EVENT);
     int const result= ::unpack_row(rli, m_table, m_width, m_curr_row, &m_cols,
-                                   &m_curr_row_end, &m_master_reclength,
-                                   abort_on_warning, first_row);
+                                   &m_curr_row_end, &m_master_reclength);
     if (m_curr_row_end > m_rows_end)
       my_error(ER_SLAVE_CORRUPT_EVENT, MYF(0));
     ASSERT_OR_RETURN_ERROR(m_curr_row_end <= m_rows_end, HA_ERR_CORRUPT_EVENT);

=== modified file 'sql/rpl_record.cc'
--- a/sql/rpl_record.cc	2010-01-28 21:51:40 +0000
+++ b/sql/rpl_record.cc	2010-12-17 10:00:51 +0000
@@ -180,8 +180,7 @@ int
 unpack_row(Relay_log_info const *rli,
            TABLE *table, uint const colcnt,
            uchar const *const row_data, MY_BITMAP const *cols,
-           uchar const **const row_end, ulong *const master_reclength,
-           const bool abort_on_warning, const bool first_row)
+           uchar const **const row_end, ulong *const master_reclength)
 {
   DBUG_ENTER("unpack_row");
   DBUG_ASSERT(row_data);
@@ -251,22 +250,9 @@ unpack_row(Relay_log_info const *rli,
         }
         else
         {
-          MYSQL_ERROR::enum_warning_level error_type=
-            MYSQL_ERROR::WARN_LEVEL_NOTE;
-          if (abort_on_warning && (table->file->has_transactions() ||
-                                   first_row))
-          {
-            error = HA_ERR_ROWS_EVENT_APPLY;
-            error_type= MYSQL_ERROR::WARN_LEVEL_ERROR;
-          }
-          else
-          {
-            f->set_default();
-            error_type= MYSQL_ERROR::WARN_LEVEL_WARN;
-          }
-          push_warning_printf(current_thd, error_type,
-                              ER_BAD_NULL_ERROR,
-                              ER(ER_BAD_NULL_ERROR),
+          f->set_default();
+          push_warning_printf(current_thd, MYSQL_ERROR::WARN_LEVEL_WARN,
+                              ER_BAD_NULL_ERROR, ER(ER_BAD_NULL_ERROR),
                               f->field_name);
         }
       }
@@ -350,20 +336,13 @@ unpack_row(Relay_log_info const *rli,
   @param skip   Number of columns for which default/nullable check 
                 should be skipped.
   @param check  Specifies if lack of default error needs checking.
-  @param abort_on_warning
-                Controls how to react on lack of a field's default.
-                The parameter mimics the master side one for
-                @c check_that_all_fields_are_given_values.
-                
+
   @returns 0 on success or a handler level error code
  */ 
-int prepare_record(TABLE *const table, 
-                   const uint skip, const bool check,
-                   const bool abort_on_warning, const bool first_row)
+int prepare_record(TABLE *const table, const uint skip, const bool check)
 {
   DBUG_ENTER("prepare_record");
 
-  int error= 0;
   restore_record(table, s->default_values);
 
   /*
@@ -386,28 +365,16 @@ int prepare_record(TABLE *const table,
     if ((f->flags &  NO_DEFAULT_VALUE_FLAG) &&
         (f->real_type() != MYSQL_TYPE_ENUM))
     {
-
-      MYSQL_ERROR::enum_warning_level error_type=
-        MYSQL_ERROR::WARN_LEVEL_NOTE;
-      if (abort_on_warning && (table->file->has_transactions() ||
-                               first_row))
-      {
-        error= HA_ERR_ROWS_EVENT_APPLY;
-        error_type= MYSQL_ERROR::WARN_LEVEL_ERROR;
-      }
-      else
-      {
-        f->set_default();
-        error_type= MYSQL_ERROR::WARN_LEVEL_WARN;
-      }
-      push_warning_printf(current_thd, error_type,
+      f->set_default();
+      push_warning_printf(current_thd,
+                          MYSQL_ERROR::WARN_LEVEL_WARN,
                           ER_NO_DEFAULT_FOR_FIELD,
                           ER(ER_NO_DEFAULT_FOR_FIELD),
                           f->field_name);
     }
   }
 
-  DBUG_RETURN(error);
+  DBUG_RETURN(0);
 }
 
 #endif // HAVE_REPLICATION

=== modified file 'sql/rpl_record.h'
--- a/sql/rpl_record.h	2009-10-22 00:15:45 +0000
+++ b/sql/rpl_record.h	2010-12-17 10:00:51 +0000
@@ -27,13 +27,10 @@ size_t pack_row(TABLE* table, MY_BITMAP
 int unpack_row(Relay_log_info const *rli,
                TABLE *table, uint const colcnt,
                uchar const *const row_data, MY_BITMAP const *cols,
-               uchar const **const row_end, ulong *const master_reclength,
-               const bool abort_on_warning= TRUE, const bool first_row= TRUE);
+               uchar const **const row_end, ulong *const master_reclength);
 
 // Fill table's record[0] with default values.
-int prepare_record(TABLE *const table, const uint skip, const bool check,
-                   const bool abort_on_warning= TRUE,
-                   const bool first_row= TRUE);
+int prepare_record(TABLE *const table, const uint skip, const bool check);
 #endif
 
 #endif


Attachment: [text/bzr-bundle] bzr/dao-gang.qu@sun.com-20101217100051-5aa62kqfblmdm9ce.bundle
Thread
bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3527) Bug#56662Dao-Gang.Qu17 Dec