List:Commits« Previous MessageNext Message »
From:Martin Hansson Date:October 7 2010 11:53am
Subject:bzr commit into mysql-trunk-merge branch (martin.hansson:3237) Bug#56423
View as plain text  
#At file:///data0/martin/bzr/bug56423/t-m/ based on revid:magnus.blaudd@stripped

 3237 Martin Hansson	2010-10-07 [merge]
      Merge of 5.5 version of fix for Bug#56423

    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
=== 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:33:11 +0000
+++ b/mysql-test/r/index_merge_myisam.result	2010-10-07 11:53:15 +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:33:11 +0000
+++ b/sql/sql_insert.cc	2010-10-07 11:53:15 +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:33:11 +0000
+++ b/sql/sql_update.cc	2010-10-07 11:53:15 +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-07-02 02:58:51 +0000
+++ b/sql/sql_update.h	2010-10-07 11:53:15 +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:33:11 +0000
+++ b/storage/innobase/row/row0sel.c	2010-10-07 11:53:15 +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-20101007115315-tftvvkfhv0kd14gj.bundle
Thread
bzr commit into mysql-trunk-merge branch (martin.hansson:3237) Bug#56423Martin Hansson7 Oct