List:Commits« Previous MessageNext Message »
From:Olav Sandstaa Date:May 7 2010 10:19am
Subject:bzr commit into mysql-next-mr-bugfixing branch (olav:3144) Bug#36981
View as plain text  
#At file:///export/home/tmp/mysql/opt-backport/ based on revid:oystein.grovlen@stripped

 3144 Olav Sandstaa	2010-05-07
      Fix for Bug#36981 innodb crash when selecting for update.
      
      (Backporting of revid:olav@stripped)
                        
      This crash occurred due to a mismatch between what the ICP code in
      InnoDB expected to be in the InnoDB record and what actually was there. 
      The ICP code expected the record to contain an index entry for the 
      index for the ICP condition while the record contained an entry from
      the clustered index. This happened due to InnoDB changing from using
      the ICP index to using the clustered index if the statement should use
      LOCK_X type locks (which happens when doing SELECT...FOR UPDATE or
      DELETE operations).
          
      The ICP implementation in InnoDB does not support ICP if the index is
      the primary key (using the clustered index). This change fixes the
      crash by extending the check for whether or not to accept the pushed
      index to also not support the case where InnoDB will change from using
      the ICP index to the clustered index when the lock type is LOCK_X.
                        
      The attached test case contains two tests. The first is based on the
      original reproduction case from the bug. The second test case is added
      to have a test to verify that the condition is evaluated even if it
      is refused by InnoDB.  
     @ mysql-test/include/icp_tests.inc
        Test for Bug#36981 innodb crash when selecting for update.
     @ mysql-test/r/innodb_icp.result
        Result file for ICP Bug#36981 when run against InnoDB.
     @ mysql-test/r/myisam_icp.result
        Result file for ICP Bug#36981 when run against MyISAM.
     @ mysql-test/t/innodb_icp.test
        Test case for ICP code for InnoDB. Note that currently this test
        will only run for debug builds due to ICP being disabled for InnoDB
        in release builds.
     @ mysql-test/t/myisam_icp.test
        Test case for ICP code in MyISAM.
     @ storage/innobase/handler/ha_innodb.cc
        Do not accept a pushed index condition if the lock strategy is LOCK_X
        since InnoDB in this case will switch from using the ICP index to the
        clustered index (which is not supported by the ICP implementation).
     @ storage/innobase/row/row0sel.c
        Change assert in the ICP implementation to test that the mysql_template
        is generated for a request for the index record's field and not for the
        entire base record.

    added:
      mysql-test/include/icp_tests.inc
      mysql-test/r/innodb_icp.result
      mysql-test/r/myisam_icp.result
      mysql-test/t/innodb_icp.test
      mysql-test/t/myisam_icp.test
    modified:
      storage/innobase/handler/ha_innodb.cc
      storage/innobase/row/row0sel.c
=== added file 'mysql-test/include/icp_tests.inc'

=== added file 'mysql-test/include/icp_tests.inc'
--- a/mysql-test/include/icp_tests.inc	1970-01-01 00:00:00 +0000
+++ b/mysql-test/include/icp_tests.inc	2010-05-07 10:19:22 +0000
@@ -0,0 +1,57 @@
+--echo #
+--echo # Bug#36981 - "innodb crash when selecting for update"
+--echo #
+
+#
+# Test 1: Test based on the reproduction test case for this bug.
+#         This query resulted in a crash in InnoDB due to
+#         InnoDB changing from using the index which the push condition
+#         where for to use the clustered index due to "SELECT ... FOR UPDATE".
+#
+
+CREATE TABLE t1 (
+  c1 CHAR(1),
+  c2 CHAR(10),
+  KEY (c1)
+);
+
+INSERT INTO t1 VALUES ('3', null);
+
+SELECT * FROM t1 WHERE c1='3' FOR UPDATE;
+
+DROP TABLE t1;
+
+#
+# Test 2: Extended test case to test that the correct rows are returned.
+#         This test is for ensuring that if InnoDB refuses to accept
+#         the pushed index condition it is still evaluated.
+#         
+
+CREATE TABLE t1 (a INT);
+INSERT INTO t1 VALUES (0),(1),(2),(3),(4),(5),(6),(7),(8),(9);
+
+CREATE TABLE t2 (a INT);
+INSERT INTO t2 SELECT A.a + 10*(B.a + 10*C.a) FROM t1 A, t1 B, t1 C;
+
+CREATE TABLE t3 (
+  c1 CHAR(10) NOT NULL,
+  c2 CHAR(10) NOT NULL,
+  c3 CHAR(200) NOT NULL,
+  KEY (c1)
+);
+
+INSERT INTO t3 
+  SELECT CONCAT('c-',1000+t2.a,'=w'), CONCAT('c-',1000+ t2.a,'=w'), 'filler'
+  FROM t2;
+
+INSERT INTO t3 
+  SELECT CONCAT('c-',1000+t2.a,'=w'), CONCAT('c-',2000+t2.a,'=w'), 'filler-1'
+  FROM t2;
+ 
+INSERT INTO t3
+  SELECT CONCAT('c-',1000+t2.a,'=w'), CONCAT('c-',3000+t2.a,'=w'), 'filler-2'
+  FROM t2;
+
+SELECT c1,c3 FROM t3 WHERE c1 >= 'c-1994=w' and c1 != 'c-1996=w' FOR UPDATE;
+
+DROP TABLE t1,t2,t3;

=== added file 'mysql-test/r/innodb_icp.result'
--- a/mysql-test/r/innodb_icp.result	1970-01-01 00:00:00 +0000
+++ b/mysql-test/r/innodb_icp.result	2010-05-07 10:19:22 +0000
@@ -0,0 +1,54 @@
+set session debug="+d,optimizer_innodb_icp";
+set @save_storage_engine= @@storage_engine;
+set storage_engine=InnoDB;
+#
+# Bug#36981 - "innodb crash when selecting for update"
+#
+CREATE TABLE t1 (
+c1 CHAR(1),
+c2 CHAR(10),
+KEY (c1)
+);
+INSERT INTO t1 VALUES ('3', null);
+SELECT * FROM t1 WHERE c1='3' FOR UPDATE;
+c1	c2
+3	NULL
+DROP TABLE t1;
+CREATE TABLE t1 (a INT);
+INSERT INTO t1 VALUES (0),(1),(2),(3),(4),(5),(6),(7),(8),(9);
+CREATE TABLE t2 (a INT);
+INSERT INTO t2 SELECT A.a + 10*(B.a + 10*C.a) FROM t1 A, t1 B, t1 C;
+CREATE TABLE t3 (
+c1 CHAR(10) NOT NULL,
+c2 CHAR(10) NOT NULL,
+c3 CHAR(200) NOT NULL,
+KEY (c1)
+);
+INSERT INTO t3 
+SELECT CONCAT('c-',1000+t2.a,'=w'), CONCAT('c-',1000+ t2.a,'=w'), 'filler'
+  FROM t2;
+INSERT INTO t3 
+SELECT CONCAT('c-',1000+t2.a,'=w'), CONCAT('c-',2000+t2.a,'=w'), 'filler-1'
+  FROM t2;
+INSERT INTO t3
+SELECT CONCAT('c-',1000+t2.a,'=w'), CONCAT('c-',3000+t2.a,'=w'), 'filler-2'
+  FROM t2;
+SELECT c1,c3 FROM t3 WHERE c1 >= 'c-1994=w' and c1 != 'c-1996=w' FOR UPDATE;
+c1	c3
+c-1994=w	filler
+c-1995=w	filler
+c-1997=w	filler
+c-1998=w	filler
+c-1999=w	filler
+c-1994=w	filler-1
+c-1995=w	filler-1
+c-1997=w	filler-1
+c-1998=w	filler-1
+c-1999=w	filler-1
+c-1994=w	filler-2
+c-1995=w	filler-2
+c-1997=w	filler-2
+c-1998=w	filler-2
+c-1999=w	filler-2
+DROP TABLE t1,t2,t3;
+set storage_engine= @save_storage_engine;

=== added file 'mysql-test/r/myisam_icp.result'
--- a/mysql-test/r/myisam_icp.result	1970-01-01 00:00:00 +0000
+++ b/mysql-test/r/myisam_icp.result	2010-05-07 10:19:22 +0000
@@ -0,0 +1,50 @@
+#
+# Bug#36981 - "innodb crash when selecting for update"
+#
+CREATE TABLE t1 (
+c1 CHAR(1),
+c2 CHAR(10),
+KEY (c1)
+);
+INSERT INTO t1 VALUES ('3', null);
+SELECT * FROM t1 WHERE c1='3' FOR UPDATE;
+c1	c2
+3	NULL
+DROP TABLE t1;
+CREATE TABLE t1 (a INT);
+INSERT INTO t1 VALUES (0),(1),(2),(3),(4),(5),(6),(7),(8),(9);
+CREATE TABLE t2 (a INT);
+INSERT INTO t2 SELECT A.a + 10*(B.a + 10*C.a) FROM t1 A, t1 B, t1 C;
+CREATE TABLE t3 (
+c1 CHAR(10) NOT NULL,
+c2 CHAR(10) NOT NULL,
+c3 CHAR(200) NOT NULL,
+KEY (c1)
+);
+INSERT INTO t3 
+SELECT CONCAT('c-',1000+t2.a,'=w'), CONCAT('c-',1000+ t2.a,'=w'), 'filler'
+  FROM t2;
+INSERT INTO t3 
+SELECT CONCAT('c-',1000+t2.a,'=w'), CONCAT('c-',2000+t2.a,'=w'), 'filler-1'
+  FROM t2;
+INSERT INTO t3
+SELECT CONCAT('c-',1000+t2.a,'=w'), CONCAT('c-',3000+t2.a,'=w'), 'filler-2'
+  FROM t2;
+SELECT c1,c3 FROM t3 WHERE c1 >= 'c-1994=w' and c1 != 'c-1996=w' FOR UPDATE;
+c1	c3
+c-1994=w	filler
+c-1995=w	filler
+c-1997=w	filler
+c-1998=w	filler
+c-1999=w	filler
+c-1994=w	filler-1
+c-1995=w	filler-1
+c-1997=w	filler-1
+c-1998=w	filler-1
+c-1999=w	filler-1
+c-1994=w	filler-2
+c-1995=w	filler-2
+c-1997=w	filler-2
+c-1998=w	filler-2
+c-1999=w	filler-2
+DROP TABLE t1,t2,t3;

=== added file 'mysql-test/t/innodb_icp.test'
--- a/mysql-test/t/innodb_icp.test	1970-01-01 00:00:00 +0000
+++ b/mysql-test/t/innodb_icp.test	2010-05-07 10:19:22 +0000
@@ -0,0 +1,22 @@
+#
+# ICP/InnoDB tests (Index Condition Pushdown)
+#
+
+--source include/have_innodb.inc
+
+# Index condition pushdown (ICP) is by default disabled in InnoDB. Support
+# for ICP can be enabled in InnoDB by using the optimizer_innodb_icp
+# debug flag. This will only be available in debug builds. 
+# When/if ICP get enabled by default the following two lines should
+# be removed.
+
+--source include/have_debug.inc
+set session debug="+d,optimizer_innodb_icp";
+
+
+set @save_storage_engine= @@storage_engine;
+set storage_engine=InnoDB;
+
+--source include/icp_tests.inc
+
+set storage_engine= @save_storage_engine;

=== added file 'mysql-test/t/myisam_icp.test'
--- a/mysql-test/t/myisam_icp.test	1970-01-01 00:00:00 +0000
+++ b/mysql-test/t/myisam_icp.test	2010-05-07 10:19:22 +0000
@@ -0,0 +1,6 @@
+#
+# ICP/MyISAM tests (Index Condition Pushdown)
+#
+
+--source include/icp_tests.inc
+

=== modified file 'storage/innobase/handler/ha_innodb.cc'
--- a/storage/innobase/handler/ha_innodb.cc	2010-04-22 14:55:40 +0000
+++ b/storage/innobase/handler/ha_innodb.cc	2010-05-07 10:19:22 +0000
@@ -10519,7 +10519,14 @@
 
 Item *ha_innobase::idx_cond_push(uint keyno_arg, Item* idx_cond_arg)
 {
-  if (keyno_arg != primary_key)
+  /* Accept to handle the pushed index condition when the following
+     conditions are true:
+     1. The index is not the primary key
+     2. The lock mode is not LOCK_X (since we in build_template
+        will change from using the index to using the clustered index
+        when the lock mode is LOCK_X). */
+  
+  if (keyno_arg != primary_key && prebuilt->select_lock_type != LOCK_X)
   {
     pushed_idx_cond_keyno= keyno_arg;
     pushed_idx_cond= idx_cond_arg;

=== modified file 'storage/innobase/row/row0sel.c'
--- a/storage/innobase/row/row0sel.c	2010-04-20 08:04:04 +0000
+++ b/storage/innobase/row/row0sel.c	2010-05-07 10:19:22 +0000
@@ -4238,7 +4238,18 @@
         if (prebuilt->idx_cond_func)
         {
           int res;
-          ut_ad(prebuilt->template_type != ROW_MYSQL_DUMMY_TEMPLATE);
+
+          /*
+            The current ICP code does not support the case where InnoDB
+            has decided to change from just reading the index entry
+            to reading the entire record. In this situation the
+            mysql_template array is set up to compare fields from
+            record not just the index entry's fields. ICP should not
+            be in use in this case but to detect if this anyway
+            happens the following assert is added.
+          */
+          ut_ad(prebuilt->template_type == ROW_MYSQL_REC_FIELDS);
+
           offsets = rec_get_offsets(rec, index, offsets, ULINT_UNDEFINED, &heap);
           row_sel_store_mysql_rec(buf, prebuilt, rec,
                                   offsets, 0, prebuilt->n_index_fields);


Attachment: [text/bzr-bundle] bzr/olav@sun.com-20100507101922-ztesp25nueojdt8i.bundle
Thread
bzr commit into mysql-next-mr-bugfixing branch (olav:3144) Bug#36981Olav Sandstaa7 May