#At file:///export/home/tmp/olav/opt-icpbugs/ based on revid:vvaintroub@stripped
2963 Olav Sandstaa 2010-02-25
Fix for Bug#36981 innodb crash when selecting for update.
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/suite/optimizer_unfixed_bugs/r/bug36981.result
Moved to the new ICP test file icp_test.inc.
@ mysql-test/suite/optimizer_unfixed_bugs/t/bug36981.test
Moved to the new ICP test file icp_test.inc.
@ 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.
removed:
mysql-test/suite/optimizer_unfixed_bugs/r/bug36981.result
mysql-test/suite/optimizer_unfixed_bugs/t/bug36981.test
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'
--- a/mysql-test/include/icp_tests.inc 1970-01-01 00:00:00 +0000
+++ b/mysql-test/include/icp_tests.inc 2010-02-25 12:55:31 +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-02-25 12:55:31 +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-1994=w filler-1
+c-1994=w filler-2
+c-1995=w filler
+c-1995=w filler-1
+c-1995=w filler-2
+c-1997=w filler
+c-1997=w filler-1
+c-1997=w filler-2
+c-1998=w filler
+c-1998=w filler-1
+c-1998=w filler-2
+c-1999=w filler
+c-1999=w filler-1
+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-02-25 12:55:31 +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;
=== removed file 'mysql-test/suite/optimizer_unfixed_bugs/r/bug36981.result'
--- a/mysql-test/suite/optimizer_unfixed_bugs/r/bug36981.result 2009-10-09 19:45:32 +0000
+++ b/mysql-test/suite/optimizer_unfixed_bugs/r/bug36981.result 1970-01-01 00:00:00 +0000
@@ -1,8 +0,0 @@
-create table `t1` (`c1` char(1) default null,`c2` char(10) default null,
-key (`c1`))
-engine=innodb default charset=latin1;
-insert into `t1` values ('3',null);
-select * from `t1` where `c1`='3' for update;
-c1 c2
-3 NULL
-drop table `t1`;
=== removed file 'mysql-test/suite/optimizer_unfixed_bugs/t/bug36981.test'
--- a/mysql-test/suite/optimizer_unfixed_bugs/t/bug36981.test 2009-10-09 19:45:32 +0000
+++ b/mysql-test/suite/optimizer_unfixed_bugs/t/bug36981.test 1970-01-01 00:00:00 +0000
@@ -1,14 +0,0 @@
-# test for BUG#36981 "innodb crash when selecting for update"
-
---source include/have_debug.inc
---source include/have_innodb.inc
-
-# crash requires this
-set session debug="+d,optimizer_innodb_icp";
-
-create table `t1` (`c1` char(1) default null,`c2` char(10) default null,
-key (`c1`))
-engine=innodb default charset=latin1;
-insert into `t1` values ('3',null);
-select * from `t1` where `c1`='3' for update;
-drop table `t1`;
=== 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-02-25 12:55:31 +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-02-25 12:55:31 +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-02-22 11:50:47 +0000
+++ b/storage/innobase/handler/ha_innodb.cc 2010-02-25 12:55:31 +0000
@@ -8670,7 +8670,14 @@ C_MODE_END
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 2009-11-30 11:00:37 +0000
+++ b/storage/innobase/row/row0sel.c 2010-02-25 12:55:31 +0000
@@ -4149,7 +4149,18 @@ idx_cond_check:
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-20100225125531-9249z4xxvxqya96u.bundle
| Thread |
|---|
| • bzr commit into mysql-6.0-codebase-bugfixing branch (olav:2963) Bug#36981 | Olav Sandstaa | 25 Feb |
