FB8-82: Removing unchanged columns from AI when when row image is not…

… FULL (facebook#942) (facebook#942)

Summary:
JIRA: https://jira.percona.com/browse/FB8-82

Reference Patch: facebook@b1fec60
Some update and upsert (insert ... on duplicate key update) queries
used to log columns which have not changed in the after image (when using RBR)
even when the image format is not FULL.
Pull Request resolved: facebook#942

Reviewed By: lloyd

Differential Revision: D13930763

Pulled By: lth

mysql-test/suite/rpl/r/rpl_row_img_sanity.result

@@ -534,6 +534,9 @@ include/rpl_reset.inc
 include/sync_slave_sql_with_master.inc
 ####### MINIMAL OTHER PARTICULAR SCENARIO ######
 include/sync_slave_sql_with_master.inc
+####### MINIMAL AND COMPLETE PARTICULAR SCENARIO SETTING SAME VALUE ######
+include/sync_slave_sql_with_master.inc
+include/sync_slave_sql_with_master.inc
 CON: 'master', IMG: 'NOBLOB', RESTART SLAVE: 'N'
 Variable_name	Value
 binlog_row_image	NOBLOB
@@ -575,6 +578,9 @@ include/rpl_reset.inc
 include/sync_slave_sql_with_master.inc
 include/sync_slave_sql_with_master.inc
 include/rpl_reset.inc
+####### MINIMAL AND COMPLETE PARTICULAR SCENARIO SETTING SAME VALUE ######
+include/sync_slave_sql_with_master.inc
+include/sync_slave_sql_with_master.inc
 ################## SPECIAL CASES #########################
 include/rpl_reset.inc
 CON: 'master', IMG: 'NOBLOB', RESTART SLAVE: 'N'

mysql-test/suite/rpl/t/rpl_row_img_sanity.test

@@ -304,7 +304,7 @@ while (`SELECT HEX(@img_types) != HEX('')`)
     #      1. columns that are set to default value are in AI
     #
     #    UPDATE/MINIMAL:
-    #      2. columns that are set to the same value are in AI
+    #      2. columns that are set to the same value are not in AI
     #      3. columns that are not in WHERE clause but in PKE are in BI
     #      4. on slave, columns that are not in master's BI but are in slave's
     #         PKE are in slave's BI
@@ -330,11 +330,11 @@ while (`SELECT HEX(@img_types) != HEX('')`)
     -- source include/rpl_row_img_parts_master_slave.inc
 
     -- let $row_img_query= UPDATE t SET c2=100, c3=1000 WHERE c2=100;
-    #   2. columns that are set to the same value are in AI
+    #   2. columns that are set to the same value are not in AI
     #   3. columns that are not in WHERE clause but in PKE are in BI
     #   4. on slave, columns that are not in master's BI but are in slave's PKE are in slave's BI
-    -- let $row_img_expected_master= 1:1 | 2:100 3:1000
-    -- let $row_img_expected_slave = 2:100 3:1 | 2:100 3:1000
+    -- let $row_img_expected_master= 1:1 | 3:1000
+    -- let $row_img_expected_slave = 2:100 3:1 | 3:1000
     -- source include/rpl_row_img_parts_master_slave.inc
 
     -- let $row_img_query= DELETE FROM t WHERE c1=1
@@ -381,6 +381,54 @@ while (`SELECT HEX(@img_types) != HEX('')`)
     --source include/sync_slave_sql_with_master.inc
   }
 
+  if (`SELECT @@binlog_row_image = "MINIMAL" OR (SELECT @@binlog_row_image = "COMPLETE")`)
+  {
+    -- echo ####### MINIMAL AND COMPLETE PARTICULAR SCENARIO SETTING SAME VALUE ######
+
+    # ASSERTIONS:
+    #    1. columns that are set to the same value are not in AI
+    #    2. columns that are set to the same value by INSERT ... DUPLICATE KEY
+    #       are not in AI
+    #
+
+    -- connection master
+    CREATE TABLE t (c1 int, c2 int, c3 blob, primary key(c1));
+    INSERT INTO t VALUES (1,2,"a");
+
+
+    --source include/sync_slave_sql_with_master.inc
+    -- connection master
+
+    # Issue some statements
+    -- let $row_img_query= UPDATE t SET c1 = 1, c2 = 4 WHERE c3 = "a";
+    if (`SELECT @@binlog_row_image = "MINIMAL"`)
+    {
+      -- let $row_img_expected_master= 1:1 | 2:4
+    }
+    if (`SELECT @@binlog_row_image = "COMPLETE"`)
+    {
+      -- let $row_img_expected_master= 1:1 2:2 3:'a' | 2:4
+    }
+    -- let $row_img_expected_slave = $row_img_expected_master
+    -- source include/rpl_row_img_parts_master_slave.inc
+
+    -- let $row_img_query= INSERT INTO t VALUES (1, 4, "a") ON DUPLICATE KEY UPDATE c2 = c2 + 2, c3 = c3;
+    if (`SELECT @@binlog_row_image = "MINIMAL"`)
+    {
+      -- let $row_img_expected_master= 1:1 | 2:6
+    }
+    if (`SELECT @@binlog_row_image = "COMPLETE"`)
+    {
+      -- let $row_img_expected_master= 1:1 2:4 3:'a' | 2:6
+    }
+    -- let $row_img_expected_slave = $row_img_expected_master
+    -- source include/rpl_row_img_parts_master_slave.inc
+
+    -- connection master
+    DROP TABLE t;
+    --source include/sync_slave_sql_with_master.inc
+  }
+
   if (`SELECT @@binlog_row_image = "NOBLOB"`)
   {
     -- echo ####### NOBLOB PARTICULAR SCENARIO ######
@@ -390,19 +438,17 @@ while (`SELECT HEX(@img_types) != HEX('')`)
     #      1 non-blob columns that are set to default value are in AI
     #    
     #    UPDATE/NOBLOB:
-    #      2 non-blob columns that are set to default value are in AI
-    #      3 blob columns that are set to default value are in AI
-    #      4 blob columns that are in WHERE clause but not in PKE are not in BI
-    #      5 blob columns that are in NON-NULL UK but not in PK are not in BI
-    #      6 on slave, blob columns that are in master's BI clause but not in
+    #      2 blob columns that are in WHERE clause but not in PKE are not in BI
+    #      3 blob columns that are in NON-NULL UK but not in PK are not in BI
+    #      4 on slave, blob columns that are in master's BI clause but not in
     #        slave's PKE are not in BI
     #
     #    DELETE
-    #      7 non-blob columns that are not in WHERE clause but in PKE are in BI
-    #      8 blob columns that are used in WHERE clause but not in PKE are not
+    #      5 non-blob columns that are not in WHERE clause but in PKE are in BI
+    #      6 blob columns that are used in WHERE clause but not in PKE are not
     #        in BI
-    #      9 blob columns that are NOT NULL UK but not in PK are not in BI
-    #     10 on slave, blob columns that are in master's BI but not in slave's PKE
+    #      7 blob columns that are NOT NULL UK but not in PK are not in BI
+    #      8 on slave, blob columns that are in master's BI but not in slave's PKE
     #        are not in slave's BI
 
     -- connection master
@@ -425,23 +471,21 @@ while (`SELECT HEX(@img_types) != HEX('')`)
     -- connection master
     -- let $row_img_query= UPDATE t SET c2='aaa', c3=1000, c5=10000 WHERE c1=1 AND c4='bbb'
     # asserts that:
-    #  2. non-blob columns that are set to default value are in AI
-    #  3. blob columns that are set to default value are in AI
-    #  4. blob columns that are in WHERE clause but not in PKE are not in BI
-    #  5. blob columns that are in NON-NULL UK but not in PK are not in BI
-    #  6. on slave, blob columns that are in master's BI clause but not in
+    #  2. blob columns that are in WHERE clause but not in PKE are not in BI
+    #  3. blob columns that are in NON-NULL UK but not in PK are not in BI
+    #  4. on slave, blob columns that are in master's BI clause but not in
     #     slave's PKE are not in BI
-    -- let $row_img_expected_master= 1:1 2:'aaa' 3:1000 5:1 | 1:1 2:'aaa' 3:1000 5:10000
-    -- let $row_img_expected_slave = 1:1 3:1000 5:1 | 1:1 2:'aaa' 3:1000 5:10000
+    -- let $row_img_expected_master= 1:1 2:'aaa' 3:1000 5:1 | 1:1 3:1000 5:10000
+    -- let $row_img_expected_slave = 1:1 3:1000 5:1 | 1:1 3:1000 5:10000
     -- source include/rpl_row_img_parts_master_slave.inc
 
     -- connection master
     -- let $row_img_query= DELETE FROM t WHERE c1=1 AND c4='bbb'
     # asserts that:
-    #   7. non-blob columns that are not in WHERE clause but in PKE are in BI
-    #   8. blob columns that are used in WHERE clause but not in PKE are not n BI
-    #   9. blob columns that are NOT NULL UK but not in PK are not in BI
-    #  10. on slave, blob columns that are in master's BI but not in slave's PKE are not in slave's BI
+    #   5. non-blob columns that are not in WHERE clause but in PKE are in BI
+    #   6. blob columns that are used in WHERE clause but not in PKE are not n BI
+    #   7. blob columns that are NOT NULL UK but not in PK are not in BI
+    #   8. on slave, blob columns that are in master's BI but not in slave's PKE are not in slave's BI
     -- let $row_img_expected_master= 1:1 2:'aaa' 3:1000 5:10000 | 
     -- let $row_img_expected_slave = 1:1 3:1000 5:10000 | 
     -- source include/rpl_row_img_parts_master_slave.inc

sql/binlog.cc

@@ -219,7 +219,8 @@ static int binlog_prepare(handlerton *hton, THD *thd, bool all);
 */
 static int binlog_set_prepared_in_tc(handlerton *hton, THD *thd);
 static void exec_binlog_error_action_abort(const char *err_string);
-static void binlog_prepare_row_images(const THD *thd, TABLE *table);
+static void binlog_prepare_row_images(const THD *thd, TABLE *table,
+                                      bool is_update);
 static bool is_loggable_xa_prepare(THD *thd);
 static int check_instance_backup_locked();
 
@@ -11722,7 +11723,7 @@ int THD::binlog_update_row(TABLE *table, bool is_trans,
      not needed for binlogging. This is done according to the:
      binlog-row-image option.
    */
-  binlog_prepare_row_images(this, table);
+  binlog_prepare_row_images(this, table, true);
 
   Row_data_memory row_data(table, before_record, after_record,
                            variables.binlog_row_value_options);
@@ -11791,7 +11792,7 @@ int THD::binlog_delete_row(TABLE *table, bool is_trans, uchar const *record,
      not needed for binlogging. This is done according to the:
      binlog-row-image option.
    */
-  binlog_prepare_row_images(this, table);
+  binlog_prepare_row_images(this, table, false);
 
   /*
      Pack records into format for transfer. We are allocating more
@@ -11822,16 +11823,17 @@ int THD::binlog_delete_row(TABLE *table, bool is_trans, uchar const *record,
   return error;
 }
 
-void binlog_prepare_row_images(const THD *thd, TABLE *table) {
+void binlog_prepare_row_images(const THD *thd, TABLE *table, bool is_update) {
   DBUG_TRACE;
   /**
-    Remove from read_set spurious columns. The write_set has been
+    Remove spurious columns. The write_set has been partially
     handled before in table->mark_columns_needed_for_update.
    */
 
   DBUG_PRINT_BITSET("debug", "table->read_set (before preparing): %s",
                     table->read_set);
 
+  /* Handle the read set */
   /**
     if there is a primary key in the table (ie, user declared PK or a
     non-null unique index) and we dont want to ship the entire image,
@@ -11876,6 +11878,39 @@ void binlog_prepare_row_images(const THD *thd, TABLE *table) {
     table->column_bitmaps_set_no_signal(&table->tmp_set, table->write_set);
   }
 
+  /* Now, handle the write set */
+  if (is_update && thd->variables.binlog_row_image != BINLOG_ROW_IMAGE_FULL &&
+      !ha_check_storage_engine_flag(table->s->db_type(),
+                                    HTON_NO_BINLOG_ROW_OPT)) {
+    /**
+      Just to be sure that tmp_write_set is currently not in use as
+      the write_set already.
+    */
+    assert(table->write_set != &table->tmp_write_set);
+
+    bitmap_copy(&table->tmp_write_set, table->write_set);
+
+    for (Field **ptr = table->field; *ptr; ptr++) {
+      Field *field = (*ptr);
+      if (bitmap_is_set(&table->tmp_write_set, field->field_index())) {
+        /* When image type is NOBLOB, we prune only BLOB fields */
+        if (thd->variables.binlog_row_image == BINLOG_ROW_IMAGE_NOBLOB &&
+            field->type() != MYSQL_TYPE_BLOB)
+          continue;
+
+        /* compare null bit */
+        if (field->is_null() && field->is_null_in_record(table->record[1]))
+          bitmap_clear_bit(&table->tmp_write_set, field->field_index());
+
+        /* compare content, only if fields are not set to NULL */
+        else if (!field->is_null() &&
+                 !field->cmp_binary_offset(table->s->rec_buff_length))
+          bitmap_clear_bit(&table->tmp_write_set, field->field_index());
+      }
+    }
+    table->column_bitmaps_set_no_signal(table->read_set, &table->tmp_write_set);
+  }
+
   DBUG_PRINT_BITSET("debug", "table->read_set (after preparing): %s",
                     table->read_set);
 }

sql/table.cc

@@ -3036,7 +3036,7 @@ int open_table_from_share(THD *thd, TABLE_SHARE *share, const char *alias,
   */
 
   bitmap_size = share->column_bitmap_size;
-  if (!(bitmaps = root->ArrayAlloc<uchar>(bitmap_size * 7))) goto err;
+  if (!(bitmaps = root->ArrayAlloc<uchar>(bitmap_size * 8))) goto err;
   bitmap_init(&outparam->def_read_set, (my_bitmap_map *)bitmaps, share->fields);
   bitmap_init(&outparam->def_write_set,
               (my_bitmap_map *)(bitmaps + bitmap_size), share->fields);
@@ -3050,6 +3050,8 @@ int open_table_from_share(THD *thd, TABLE_SHARE *share, const char *alias,
               (my_bitmap_map *)(bitmaps + bitmap_size * 5), share->fields);
   bitmap_init(&outparam->pack_row_tmp_set,
               (my_bitmap_map *)(bitmaps + bitmap_size * 6), share->fields);
+  bitmap_init(&outparam->tmp_write_set,
+              (my_bitmap_map *)(bitmaps + bitmap_size * 7), share->fields);
   outparam->default_column_bitmaps();
 
   /*

sql/table.h

@@ -1581,7 +1581,8 @@ struct TABLE {
       nullptr};  ///< Saved null_flags while null_row is true
 
   /* containers */
-  MY_BITMAP def_read_set, def_write_set, tmp_set, pack_row_tmp_set;
+  MY_BITMAP def_read_set, def_write_set, tmp_set, tmp_write_set,
+      pack_row_tmp_set;
   /*
     Bitmap of fields that one or more query condition refers to. Only
     used if optimizer_condition_fanout_filter is turned 'on'.