BUG#48091 valgrind errors when slave has double not null and master has double null

      
Backporting BUG#43789 to mysql-5.1-bugteam
                              
The replication was generating corrupted data, warning messages on Valgrind
and aborting on debug mode while replicating a "null" to "not null" field.
Specifically the unpack_row routine, was considering the slave's table
definition and trying to retrieve a field value, where there was nothing to be
retrieved, ignoring the fact that the value was defined as "null" by the master.
                              
To fix the problem, we proceed as follows:
                              
1 - If it is not STRICT sql_mode, implicit default values are used, regardless
if it is multi-row or single-row statement.
                              
2 - However, if it is STRICT mode, then a we do what follows:
                              
2.1 If it is a transactional engine, we do a rollback on the first NULL that is
to be set into a NOT NULL column and return an error.
                              
2.2 If it is a non-transactional engine and it is the first row to be inserted
with multi-row, we also return the error. Otherwise, we proceed with the
execution, use implicit default values and print out warning messages.
                        
Unfortunately, the current patch cannot mimic the behavior showed by the master
for updates on multi-tables and multi-row inserts. This happens because such
statements are unfolded in different row events. For instance, considering the
following updates and strict mode:
                        
(master)
create table t1 (a int);
create table t2 (a int not null);
insert into t1 values (1);
insert into t2 values (2);
update t1, t2 SET t1.a=10, t2.a=NULL;
                        
t1 would have (10) and t2 would have (0) as this would be handled as a
multi-row update. On the other hand, if we had the following updates:
                        
(master)
create table t1 (a int);
create table t2 (a int);
                        
(slave)
create table t1 (a int);
create table t2 (a int not null);
                        
(master)
insert into t1 values (1);
insert into t2 values (2);
update t1, t2 SET t1.a=10, t2.a=NULL;
                        
On the master t1 would have (10) and t2 would have (NULL). On
the slave, t1 would have (10) but the update on t1 would fail.

4 files changed, 68 insertions, 19 deletions

diff --git a/sql/log_event.cc b/sql/log_event.cc
index 935dfa1a536..ddedf07a70e 100644
--- a/sql/log_event.cc
+++ b/sql/log_event.cc
@@ -8450,16 +8450,17 @@ Rows_log_event::write_row(const Relay_log_info *const rli,
   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,
-                             (rli->sql_thd->variables.sql_mode &
-                               (MODE_STRICT_TRANS_TABLES |
-                                MODE_STRICT_ALL_TABLES)))))
+                             abort_on_warnings, m_curr_row == m_rows_buf)))
     DBUG_RETURN(error);
   
   /* unpack row into table->record[0] */
-  error= unpack_current_row(rli); // TODO: how to handle errors?
+  if ((error= unpack_current_row(rli, abort_on_warnings)))
+    DBUG_RETURN(error);
+
   if (m_curr_row == m_rows_buf)
   {
     /* this is the first row to be inserted, we estimate the rows with
@@ -9256,8 +9257,12 @@ Update_rows_log_event::do_exec_row(const Relay_log_info *const rli)
 
   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;
-  error= unpack_current_row(rli); // this also updates m_curr_row_end
+  /* this also updates m_curr_row_end */
+  if ((error= unpack_current_row(rli, abort_on_warnings)))
+    return error;
 
   /*
     Now we have the right row to update.  The old row (the one we're
diff --git a/sql/log_event.h b/sql/log_event.h
index 31d4a7480c2..0b4c63a73af 100644
--- a/sql/log_event.h
+++ b/sql/log_event.h
@@ -3541,12 +3541,16 @@ 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)
+  int unpack_current_row(const Relay_log_info *const rli,
+                         const bool abort_on_warning= TRUE)
   { 
     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);
+                                   &m_curr_row_end, &m_master_reclength,
+                                   abort_on_warning, first_row);
     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);
diff --git a/sql/rpl_record.cc b/sql/rpl_record.cc
index f4768e2456a..8e80620dd2c 100644
--- a/sql/rpl_record.cc
+++ b/sql/rpl_record.cc
@@ -180,7 +180,8 @@ 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)
+           uchar const **const row_end, ulong *const master_reclength,
+           const bool abort_on_warning, const bool first_row)
 {
   DBUG_ENTER("unpack_row");
   DBUG_ASSERT(row_data);
@@ -224,8 +225,35 @@ unpack_row(Relay_log_info const *rli,
       /* Field...::unpack() cannot return 0 */
       DBUG_ASSERT(pack_ptr != NULL);
 
-      if ((null_bits & null_mask) && f->maybe_null())
-        f->set_null();
+      if (null_bits & null_mask)
+      {
+        if (f->maybe_null())
+        {
+          DBUG_PRINT("debug", ("Was NULL; null mask: 0x%x; null bits: 0x%x",
+                               null_mask, null_bits));
+          f->set_null();
+        }
+        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->field_name);
+        }
+      }
       else
       {
         f->set_notnull();
@@ -315,7 +343,7 @@ unpack_row(Relay_log_info const *rli,
  */ 
 int prepare_record(TABLE *const table, 
                    const uint skip, const bool check,
-                   const bool abort_on_warning)
+                   const bool abort_on_warning, const bool first_row)
 {
   DBUG_ENTER("prepare_record");
 
@@ -343,14 +371,24 @@ int prepare_record(TABLE *const table,
     if ((f->flags &  NO_DEFAULT_VALUE_FLAG) &&
         (f->real_type() != MYSQL_TYPE_ENUM))
     {
-      push_warning_printf(current_thd, abort_on_warning?
-                          MYSQL_ERROR::WARN_LEVEL_ERROR :
-                          MYSQL_ERROR::WARN_LEVEL_WARN,
+
+      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_NO_DEFAULT_FOR_FIELD,
                           ER(ER_NO_DEFAULT_FOR_FIELD),
                           f->field_name);
-      if (abort_on_warning)
-        error = HA_ERR_ROWS_EVENT_APPLY;
     }
   }
 
diff --git a/sql/rpl_record.h b/sql/rpl_record.h
index ab2bcd382ca..6e8838f82b3 100644
--- a/sql/rpl_record.h
+++ b/sql/rpl_record.h
@@ -27,11 +27,13 @@ size_t pack_row(TABLE* table, MY_BITMAP const* cols,
 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);
+               uchar const **const row_end, ulong *const master_reclength,
+               const bool abort_on_warning= TRUE, const bool first_row= TRUE);
 
 // 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= FALSE);
+                   const bool abort_on_warning= TRUE,
+                   const bool first_row= TRUE);
 #endif
 
 #endif