List:Commits« Previous MessageNext Message »
From:Martin Hansson Date:October 7 2010 10:05am
Subject:bzr push into mysql-5.5-bugteam branch (martin.hansson:3214 to 3215)
Bug#56423
View as plain text  
 3215 Martin Hansson	2010-10-07
      Bug#56423: Different count with SELECT and CREATE SELECT queries
      
      This is the 5.5 version of the fix. The 5.1 version was too complicated to
      merge and was null merged.
      
      This is a regression from the fix for bug no 38999. A storage engine capable
      of reading only a subset of a table's columns updates corresponding bits in
      the read buffer to signal that it has read NULL values for the corresponding
      columns. It cannot, and should not, update any other bits. Bug no 38999
      occurred because the implementation of UPDATE statements compare the NULL bits
      using memcmp, inadvertently comparing bits that were never requested from the
      storage engine. The regression was caused by the storage engine trying to
      alleviate the situation by writing to all NULL bits, even those that it had no
      knowledge of. This has devastating effects for the index merge algorithm,
      which relies on all NULL bits, except those explicitly requested, being left
      unchanged.
      
      The fix reverts the fix for bug no 38999 in both InnoDB and InnoDB plugin and
      changes the server's method of comparing records. For engines that always read
      entire rows, we proceed as usual. For engines capable of reading only select
      columns, the record buffers are now compared on a column by column basis. An
      assertion was also added so that non comparable buffers are never read. Some
      relevant copy-pasted code was also consolidated in a new function.

    modified:
      mysql-test/include/index_merge2.inc
      mysql-test/r/index_merge_myisam.result
      sql/sql_insert.cc
      sql/sql_update.cc
      sql/sql_update.h
      storage/innobase/row/row0sel.c
 3214 Magnus Blåudd	2010-10-07
      Bug#56397 The version of NDB in MySQL Server should be constant
       - Fix the version of NDB in MySQL Server to 5.5.7(although it's actually 6.2.18)

    modified:
      storage/ndb/ndb_configure.m4
=== modified file 'mysql-test/include/index_merge2.inc'
--- a/mysql-test/include/index_merge2.inc	2010-10-07 08:24:44 +0000
+++ b/mysql-test/include/index_merge2.inc	2010-10-07 10:01:51 +0000
@@ -351,3 +351,115 @@ explain select * from t1 where (key3 > 3
 select * from t1 where (key3 > 30 and key3<35) or (key2 >32 and key2 < 40);
 drop table t1;
 
+--echo #
+--echo # Bug#56423: Different count with SELECT and CREATE SELECT queries
+--echo #
+
+CREATE TABLE t1 (
+  a INT,
+  b INT,
+  c INT,
+  d INT,
+  PRIMARY KEY (a),
+  KEY (c),
+  KEY bd (b,d)
+);
+
+INSERT INTO t1 VALUES
+(1, 0, 1, 0),
+(2, 1, 1, 1),
+(3, 1, 1, 1),
+(4, 0, 1, 1);
+
+EXPLAIN
+SELECT a
+FROM t1
+WHERE c = 1 AND b = 1 AND d = 1;
+
+CREATE TABLE t2 ( a INT )
+SELECT a
+FROM t1
+WHERE c = 1 AND b = 1 AND d = 1;
+
+SELECT * FROM t2;
+
+DROP TABLE t1, t2;
+
+CREATE TABLE t1( a INT, b INT, KEY(a), KEY(b) );
+INSERT INTO t1 VALUES (1, 2), (1, 2), (1, 2), (1, 2);
+SELECT * FROM t1 FORCE INDEX(a, b) WHERE a = 1 AND b = 2;
+
+DROP TABLE t1;
+
+--echo # Code coverage of fix.
+CREATE TABLE t1 ( a INT NOT NULL AUTO_INCREMENT PRIMARY KEY, b INT);
+INSERT INTO t1 (b) VALUES (1);
+UPDATE t1 SET b = 2 WHERE a = 1;
+SELECT * FROM t1;
+
+CREATE TABLE t2 ( a INT NOT NULL AUTO_INCREMENT PRIMARY KEY, b VARCHAR(1) );
+INSERT INTO t2 (b) VALUES ('a');
+UPDATE t2 SET b = 'b' WHERE a = 1;
+SELECT * FROM t2;
+
+DROP TABLE t1, t2;
+
+# The test was inactive for InnoDB at the time of pushing. The following is
+# expected result for the Bug#56423 test. It can be uncommented and pasted
+# into result file when reactivating the test.
+
+##
+## Bug#56423: Different count with SELECT and CREATE SELECT queries
+##
+#CREATE TABLE t1 (
+#a INT,
+#b INT,
+#c INT,
+#d INT,
+#PRIMARY KEY (a),
+#KEY (c),
+#KEY bd (b,d)
+#);
+#INSERT INTO t1 VALUES
+#(1, 0, 1, 0),
+#(2, 1, 1, 1),
+#(3, 1, 1, 1),
+#(4, 0, 1, 1);
+#EXPLAIN
+#SELECT a
+#FROM t1
+#WHERE c = 1 AND b = 1 AND d = 1;
+#id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+#1	SIMPLE	t1	ref	c,bd	bd	10	const,const	2	Using where
+#CREATE TABLE t2 ( a INT )
+#SELECT a
+#FROM t1
+#WHERE c = 1 AND b = 1 AND d = 1;
+#SELECT * FROM t2;
+#a
+#2
+#3
+#DROP TABLE t1, t2;
+#CREATE TABLE t1( a INT, b INT, KEY(a), KEY(b) );
+#INSERT INTO t1 VALUES (1, 2), (1, 2), (1, 2), (1, 2);
+#SELECT * FROM t1 FORCE INDEX(a, b) WHERE a = 1 AND b = 2;
+#a	b
+#1	2
+#1	2
+#1	2
+#1	2
+#DROP TABLE t1;
+## Code coverage of fix.
+#CREATE TABLE t1 ( a INT NOT NULL AUTO_INCREMENT PRIMARY KEY, b INT);
+#INSERT INTO t1 (b) VALUES (1);
+#UPDATE t1 SET b = 2 WHERE a = 1;
+#SELECT * FROM t1;
+#a	b
+#1	2
+#CREATE TABLE t2 ( a INT NOT NULL AUTO_INCREMENT PRIMARY KEY, b VARCHAR(1) );
+#INSERT INTO t2 (b) VALUES ('a');
+#UPDATE t2 SET b = 'b' WHERE a = 1;
+#SELECT * FROM t2;
+#a	b
+#1	b
+#DROP TABLE t1, t2;

=== modified file 'mysql-test/r/index_merge_myisam.result'
--- a/mysql-test/r/index_merge_myisam.result	2010-10-07 08:24:44 +0000
+++ b/mysql-test/r/index_merge_myisam.result	2010-10-07 10:01:51 +0000
@@ -1156,6 +1156,61 @@ key1	key2	key3
 38	38	38
 39	39	39
 drop table t1;
+#
+# Bug#56423: Different count with SELECT and CREATE SELECT queries
+#
+CREATE TABLE t1 (
+a INT,
+b INT,
+c INT,
+d INT,
+PRIMARY KEY (a),
+KEY (c),
+KEY bd (b,d)
+);
+INSERT INTO t1 VALUES
+(1, 0, 1, 0),
+(2, 1, 1, 1),
+(3, 1, 1, 1),
+(4, 0, 1, 1);
+EXPLAIN
+SELECT a
+FROM t1
+WHERE c = 1 AND b = 1 AND d = 1;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t1	ref	c,bd	bd	10	const,const	2	Using where
+CREATE TABLE t2 ( a INT )
+SELECT a
+FROM t1
+WHERE c = 1 AND b = 1 AND d = 1;
+SELECT * FROM t2;
+a
+2
+3
+DROP TABLE t1, t2;
+CREATE TABLE t1( a INT, b INT, KEY(a), KEY(b) );
+INSERT INTO t1 VALUES (1, 2), (1, 2), (1, 2), (1, 2);
+SELECT * FROM t1 FORCE INDEX(a, b) WHERE a = 1 AND b = 2;
+a	b
+1	2
+1	2
+1	2
+1	2
+DROP TABLE t1;
+# Code coverage of fix.
+CREATE TABLE t1 ( a INT NOT NULL AUTO_INCREMENT PRIMARY KEY, b INT);
+INSERT INTO t1 (b) VALUES (1);
+UPDATE t1 SET b = 2 WHERE a = 1;
+SELECT * FROM t1;
+a	b
+1	2
+CREATE TABLE t2 ( a INT NOT NULL AUTO_INCREMENT PRIMARY KEY, b VARCHAR(1) );
+INSERT INTO t2 (b) VALUES ('a');
+UPDATE t2 SET b = 'b' WHERE a = 1;
+SELECT * FROM t2;
+a	b
+1	b
+DROP TABLE t1, t2;
 #---------------- 2-sweeps read Index merge test 2 -------------------------------
 SET SESSION STORAGE_ENGINE = MyISAM;
 drop table if exists t1;

=== modified file 'sql/sql_insert.cc'
--- a/sql/sql_insert.cc	2010-10-07 08:24:44 +0000
+++ b/sql/sql_insert.cc	2010-10-07 10:01:51 +0000
@@ -1620,9 +1620,7 @@ int write_record(THD *thd, TABLE *table,
           table->file->adjust_next_insert_id_after_explicit_value(
             table->next_number_field->val_int());
         info->touched++;
-        if ((table->file->ha_table_flags() & HA_PARTIAL_COLUMN_READ &&
-             !bitmap_is_subset(table->write_set, table->read_set)) ||
-            compare_record(table))
+        if (!records_are_comparable(table) || compare_records(table))
         {
           if ((error=table->file->ha_update_row(table->record[1],
                                                 table->record[0])) &&

=== modified file 'sql/sql_update.cc'
--- a/sql/sql_update.cc	2010-10-07 08:24:44 +0000
+++ b/sql/sql_update.cc	2010-10-07 10:01:51 +0000
@@ -42,11 +42,68 @@
                          // mysql_handle_derived,
                          // mysql_derived_filling
 
-/* Return 0 if row hasn't changed */
 
-bool compare_record(TABLE *table)
+/**
+   True if the table's input and output record buffers are comparable using
+   compare_records(TABLE*).
+ */
+bool records_are_comparable(const TABLE *table) {
+  return ((table->file->ha_table_flags() & HA_PARTIAL_COLUMN_READ) == 0) ||
+    bitmap_is_subset(table->write_set, table->read_set);
+}
+
+
+/**
+   Compares the input and outbut record buffers of the table to see if a row
+   has changed. The algorithm iterates over updated columns and if they are
+   nullable compares NULL bits in the buffer before comparing actual
+   data. Special care must be taken to compare only the relevant NULL bits and
+   mask out all others as they may be undefined. The storage engine will not
+   and should not touch them.
+
+   @param table The table to evaluate.
+
+   @return true if row has changed.
+   @return false otherwise.
+*/
+bool compare_records(const TABLE *table)
 {
+  DBUG_ASSERT(records_are_comparable(table));
+
+  if ((table->file->ha_table_flags() & HA_PARTIAL_COLUMN_READ) != 0)
+  {
+    /*
+      Storage engine may not have read all columns of the record.  Fields
+      (including NULL bits) not in the write_set may not have been read and
+      can therefore not be compared.
+    */ 
+    for (Field **ptr= table->field ; *ptr != NULL; ptr++)
+    {
+      Field *field= *ptr;
+      if (bitmap_is_set(table->write_set, field->field_index))
+      {
+        if (field->real_maybe_null())
+        {
+          uchar null_byte_index= field->null_ptr - table->record[0];
+          
+          if (((table->record[0][null_byte_index]) & field->null_bit) !=
+              ((table->record[1][null_byte_index]) & field->null_bit))
+            return TRUE;
+        }
+        if (field->cmp_binary_offset(table->s->rec_buff_length))
+          return TRUE;
+      }
+    }
+    return FALSE;
+  }
+  
+  /* 
+     The storage engine has read all columns, so it's safe to compare all bits
+     including those not in the write_set. This is cheaper than the field-by-field
+     comparison done above.
+  */ 
   if (table->s->blob_fields + table->s->varchar_fields == 0)
+    // Fixed-size record: do bitwise comparison of the records 
     return cmp_record(table,record[1]);
   /* Compare null bits */
   if (memcmp(table->null_flags,
@@ -204,7 +261,6 @@ int mysql_update(THD *thd,
   bool		using_limit= limit != HA_POS_ERROR;
   bool		safe_update= test(thd->variables.option_bits & OPTION_SAFE_UPDATES);
   bool          used_key_is_modified= FALSE, transactional_table, will_batch;
-  bool		can_compare_record;
   int           res;
   int		error, loc_error;
   uint          used_index, dup_key_found;
@@ -579,15 +635,6 @@ int mysql_update(THD *thd,
   if (table->file->ha_table_flags() & HA_PARTIAL_COLUMN_READ)
     table->prepare_for_position();
 
-  /*
-    We can use compare_record() to optimize away updates if
-    the table handler is returning all columns OR if
-    if all updated columns are read
-  */
-  can_compare_record= (!(table->file->ha_table_flags() &
-                         HA_PARTIAL_COLUMN_READ) ||
-                       bitmap_is_subset(table->write_set, table->read_set));
-
   while (!(error=info.read_record(&info)) && !thd->killed)
   {
     thd->examined_row_count++;
@@ -605,7 +652,7 @@ int mysql_update(THD *thd,
 
       found++;
 
-      if (!can_compare_record || compare_record(table))
+      if (!records_are_comparable(table) || compare_records(table))
       {
         if ((res= table_list->view_check_option(thd, ignore)) !=
             VIEW_CHECK_OK)
@@ -1645,18 +1692,8 @@ bool multi_update::send_data(List<Item> 
     if (table->status & (STATUS_NULL_ROW | STATUS_UPDATED))
       continue;
 
-    /*
-      We can use compare_record() to optimize away updates if
-      the table handler is returning all columns OR if
-      if all updated columns are read
-    */
     if (table == table_to_update)
     {
-      bool can_compare_record;
-      can_compare_record= (!(table->file->ha_table_flags() &
-                             HA_PARTIAL_COLUMN_READ) ||
-                           bitmap_is_subset(table->write_set,
-                                            table->read_set));
       table->status|= STATUS_UPDATED;
       store_record(table,record[1]);
       if (fill_record_n_invoke_before_triggers(thd, *fields_for_table[offset],
@@ -1671,7 +1708,7 @@ bool multi_update::send_data(List<Item> 
       */
       table->auto_increment_field_not_null= FALSE;
       found++;
-      if (!can_compare_record || compare_record(table))
+      if (!records_are_comparable(table) || compare_records(table))
       {
 	int error;
         if ((error= cur_table->view_check_option(thd, ignore)) !=
@@ -1860,7 +1897,6 @@ int multi_update::do_updates()
     DBUG_RETURN(0);
   for (cur_table= update_tables; cur_table; cur_table= cur_table->next_local)
   {
-    bool can_compare_record;
     uint offset= cur_table->shared;
 
     table = cur_table->table;
@@ -1897,11 +1933,6 @@ int multi_update::do_updates()
     if ((local_error = tmp_table->file->ha_rnd_init(1)))
       goto err;
 
-    can_compare_record= (!(table->file->ha_table_flags() &
-                           HA_PARTIAL_COLUMN_READ) ||
-                         bitmap_is_subset(table->write_set,
-                                          table->read_set));
-
     for (;;)
     {
       if (thd->killed && trans_safe)
@@ -1942,7 +1973,7 @@ int multi_update::do_updates()
                                             TRG_ACTION_BEFORE, TRUE))
         goto err2;
 
-      if (!can_compare_record || compare_record(table))
+      if (!records_are_comparable(table) || compare_records(table))
       {
         int error;
         if ((error= cur_table->view_check_option(thd, ignore)) !=

=== modified file 'sql/sql_update.h'
--- a/sql/sql_update.h	2010-04-12 13:17:37 +0000
+++ b/sql/sql_update.h	2010-10-07 10:01:51 +0000
@@ -38,6 +38,7 @@ bool mysql_multi_update(THD *thd, TABLE_
                         enum enum_duplicates handle_duplicates, bool ignore,
                         SELECT_LEX_UNIT *unit, SELECT_LEX *select_lex,
                         multi_update **result);
-bool compare_record(TABLE *table);
+bool records_are_comparable(const TABLE *table);
+bool compare_records(const TABLE *table);
 
 #endif /* SQL_UPDATE_INCLUDED */

=== modified file 'storage/innobase/row/row0sel.c'
--- a/storage/innobase/row/row0sel.c	2010-10-07 08:24:44 +0000
+++ b/storage/innobase/row/row0sel.c	2010-10-07 10:01:51 +0000
@@ -2688,12 +2688,6 @@ row_sel_store_mysql_rec(
 		prebuilt->blob_heap = NULL;
 	}
 
-	/* init null bytes with default values as they might be
-	left uninitialized in some cases and these uninited bytes
-	might be copied into mysql record buffer that leads to
-	valgrind warnings */
-	memcpy(mysql_rec, prebuilt->default_rec, prebuilt->null_bitmap_len);
-
 	for (i = 0; i < prebuilt->n_template; i++) {
 
 		templ = prebuilt->mysql_template + i;

Attachment: [text/bzr-bundle] bzr/martin.hansson@oracle.com-20101007100151-s0zsldqupugpdi64.bundle
Thread
bzr push into mysql-5.5-bugteam branch (martin.hansson:3214 to 3215)Bug#56423Martin Hansson7 Oct