List:Commits« Previous MessageNext Message »
From:Martin Hansson Date:September 21 2010 9:46am
Subject:bzr commit into mysql-5.1-bugteam branch (martin.hansson:3514) Bug#56423
View as plain text  
#At file:///data0/martin/bzr/bug56423/5.1bt-commit/ based on revid:alfranio.correia@stripped

 3514 Martin Hansson	2010-09-21
      Bug#56423: Different count with SELECT and CREATE SELECT queries
      
      This is a regression from the fix for bug no 38999. A storage engine updates
      certain 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 in the
      buffer. Bug no 38999 occured because the implementation of UPDATE statements
      compare the input buffer to the output buffer 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 in the buffer, even those that it had no
      knowledge of. This has devastating effects for the index merge algorithm,
      which counts on all NULL bits, except those explicitly requested, being left
      unchanged.
      
      The fix reverts the fix for bug no 38999 and changes the server's method of
      comparing records. Unless a whole row is being written, the irrelevant bits
      are now masked out before comparing and memcmp is avoided altogether for NULL
      bits.

    modified:
      mysql-test/include/index_merge2.inc
      mysql-test/r/index_merge_innodb.result
      mysql-test/r/index_merge_myisam.result
      sql/sql_update.cc
      storage/innobase/row/row0sel.c
      storage/innodb_plugin/row/row0sel.c
=== modified file 'mysql-test/include/index_merge2.inc'
--- a/mysql-test/include/index_merge2.inc	2006-09-14 19:44:17 +0000
+++ b/mysql-test/include/index_merge2.inc	2010-09-21 09:45:54 +0000
@@ -343,3 +343,55 @@ 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;

=== modified file 'mysql-test/r/index_merge_innodb.result'
--- a/mysql-test/r/index_merge_innodb.result	2010-09-16 12:13:53 +0000
+++ b/mysql-test/r/index_merge_innodb.result	2010-09-21 09:45:54 +0000
@@ -324,6 +324,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	index_merge	c,bd	c,bd	5,10	NULL	1	Using intersect(c,bd); Using where; Using index
+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 = InnoDB;
 drop table if exists t1;

=== modified file 'mysql-test/r/index_merge_myisam.result'
--- a/mysql-test/r/index_merge_myisam.result	2010-09-16 12:13:53 +0000
+++ b/mysql-test/r/index_merge_myisam.result	2010-09-21 09:45:54 +0000
@@ -1158,6 +1158,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_update.cc'
--- a/sql/sql_update.cc	2010-08-09 11:39:59 +0000
+++ b/sql/sql_update.cc	2010-09-21 09:45:54 +0000
@@ -25,23 +25,43 @@
 #include "sql_trigger.h"
 #include "debug_sync.h"
 
-/* Return 0 if row hasn't changed */
-
+/**
+   Compares the input and outbut buffer 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 false if row hasn't changed.
+   @return true otherwise.
+ */
 bool compare_record(TABLE *table)
 {
-  if (table->s->blob_fields + table->s->varchar_fields == 0)
-    return cmp_record(table,record[1]);
-  /* Compare null bits */
-  if (memcmp(table->null_flags,
-	     table->null_flags+table->s->rec_buff_length,
-	     table->s->null_bytes))
-    return TRUE;				// Diff in NULL value
+  if (bitmap_is_set_all(table->write_set) &&
+      table->s->blob_fields == 0 &&
+      table->s->varchar_fields == 0)
+    return cmp_record(table, record[1]);
+
   /* Compare updated fields */
   for (Field **ptr= table->field ; *ptr ; ptr++)
   {
-    if (bitmap_is_set(table->write_set, (*ptr)->field_index) &&
-	(*ptr)->cmp_binary_offset(table->s->rec_buff_length))
-      return TRUE;
+    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;
 }

=== modified file 'storage/innobase/row/row0sel.c'
--- a/storage/innobase/row/row0sel.c	2010-06-09 12:07:34 +0000
+++ b/storage/innobase/row/row0sel.c	2010-09-21 09:45:54 +0000
@@ -2621,12 +2621,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 this 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;

=== modified file 'storage/innodb_plugin/row/row0sel.c'
--- a/storage/innodb_plugin/row/row0sel.c	2010-08-18 11:01:10 +0000
+++ b/storage/innodb_plugin/row/row0sel.c	2010-09-21 09:45:54 +0000
@@ -2696,12 +2696,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@sun.com-20100921094554-kisia9ceaatxth1e.bundle
Thread
bzr commit into mysql-5.1-bugteam branch (martin.hansson:3514) Bug#56423Martin Hansson21 Sep
Re: bzr commit into mysql-5.1-bugteam branch (martin.hansson:3514)Bug#56423Martin Hansson22 Sep