Hi Jørgen,
Thanks for the review. I will make the test case less verbose before
committing this patch.
Olav
On 15/11/2010 15:28, Jorgen Loland wrote:
> Olav,
>
> The patch looks good. See one comment inline.
>
> On 11/15/2010 01:42 PM, Olav Sandstaa wrote:
>> #At file:///export/home/tmp/mysql/opt-bug56529-v2/ based on
>> revid:olav.sandstaa@stripped
>>
>> 3281 Olav Sandstaa 2010-11-15
>> Fix for Bug#56529 Crash due to long semaphore wait in InnoDB
>> with ICP and subqueries
>>
>> This crash was caused by the server pushing down an index
>> condition to
>> InnoDB that contained a subselect. When InnoDB executed this
>> index
>> condition the subselect would open a new handler object and
>> access
>> InnoDB a second time. This caused the same transaction to have
>> two
>> active data accesses to InnoDB which triggered the assert in
>> InnoDB.
>>
>> The fix for this is to not push down index conditions to the
>> handler
>> that contain a subselect.
>> @ mysql-test/include/icp_tests.inc
>> Test case for Bug#56529 Crash due to long semaphore wait in
>> InnoDB
>> with ICP and subqueries.
>> @ mysql-test/r/innodb_icp.result
>> Result file for Test case for Bug#56529 Crash due to long
>> semaphore
>> wait in InnoDB with ICP and subqueries.
>> @ mysql-test/r/innodb_icp_all.result
>> Result file for Test case for Bug#56529 Crash due to long
>> semaphore
>> wait in InnoDB with ICP and subqueries.
>> @ mysql-test/r/innodb_icp_none.result
>> Result file for Test case for Bug#56529 Crash due to long
>> semaphore
>> wait in InnoDB with ICP and subqueries.
>> @ mysql-test/r/myisam_icp.result
>> Result file for Test case for Bug#56529 Crash due to long
>> semaphore
>> wait in InnoDB with ICP and subqueries.
>> @ mysql-test/r/myisam_icp_all.result
>> Result file for Test case for Bug#56529 Crash due to long
>> semaphore
>> wait in InnoDB with ICP and subqueries.
>> @ mysql-test/r/myisam_icp_none.result
>> Result file for Test case for Bug#56529 Crash due to long
>> semaphore
>> wait in InnoDB with ICP and subqueries.
>> @ sql/sql_select.cc
>> We should not push down index conditions to the handler that
>> contains
>> a subselect. This was already handled by the default case of
>> the
>> switch in uses_index_fields_only() but due to an earlier
>> check in
>> uses_index_fields() the code for checking that the condition
>> only
>> used "other tables" could return ok early.
>>
>> Fix this by adding an explicite check for if the condition
>> contains a subselect and if so return false.
>>
>> modified:
>> mysql-test/include/icp_tests.inc
>> mysql-test/r/innodb_icp.result
>> mysql-test/r/innodb_icp_all.result
>> mysql-test/r/innodb_icp_none.result
>> mysql-test/r/myisam_icp.result
>> mysql-test/r/myisam_icp_all.result
>> mysql-test/r/myisam_icp_none.result
>> sql/sql_select.cc
>> === modified file 'mysql-test/include/icp_tests.inc'
>>
>> === modified file 'mysql-test/include/icp_tests.inc'
>> --- a/mysql-test/include/icp_tests.inc 2010-11-08 08:49:16 +0000
>> +++ b/mysql-test/include/icp_tests.inc 2010-11-15 12:42:32 +0000
>> @@ -515,3 +515,61 @@
>> EXPLAIN SELECT * FROM t1 WHERE (c1<= '6566-06-15' AND c2<> 3);
>>
>> DROP TABLE t1;
>> +
>> +--echo #
>> +--echo # Bug#56529 - "Crash due to long semaphore wait in InnoDB
>> +--echo # with ICP and subqueries"
>> +--echo #
>> +
>> +CREATE TABLE t1 (
>> + col_int_nokey INTEGER,
>> + col_int_key INTEGER,
>> + col_varchar_key VARCHAR(1),
>> +
>> + KEY (col_int_key),
>> + KEY (col_varchar_key, col_int_key)
>> +);
>> +
>> +INSERT INTO t1 VALUES (NULL,2,'w');
>> +INSERT INTO t1 VALUES (7,9,'m');
>> +INSERT INTO t1 VALUES (9,3,'m');
>> +INSERT INTO t1 VALUES (7,9,'k');
>> +INSERT INTO t1 VALUES (4,NULL,'r');
>> +INSERT INTO t1 VALUES (2,9,'t');
>> +INSERT INTO t1 VALUES (6,3,'j');
>> +INSERT INTO t1 VALUES (8,8,'u');
>> +INSERT INTO t1 VALUES (NULL,8,'h');
>> +INSERT INTO t1 VALUES (5,53,'o');
>> +INSERT INTO t1 VALUES (NULL,0,NULL);
>> +INSERT INTO t1 VALUES (6,5,'k');
>> +INSERT INTO t1 VALUES (188,166,'e');
>> +INSERT INTO t1 VALUES (2,3,'n');
>> +INSERT INTO t1 VALUES (1,0,'t');
>> +INSERT INTO t1 VALUES (1,1,'c');
>> +INSERT INTO t1 VALUES (0,9,'m');
>> +INSERT INTO t1 VALUES (9,5,'y');
>> +INSERT INTO t1 VALUES (NULL,6,'f');
>> +
>> +CREATE TABLE t2 (
>> + c1 INTEGER NOT NULL
>> +);
>> +
>> +let query=
>> +SELECT table1.col_int_nokey
>> +FROM
>> + (
>> + t1 AS table1 STRAIGHT_JOIN
>> + (
>> + (
>> + t1 AS table2 INNER JOIN t1 AS table3
>> + ON (table3.col_varchar_key = table2.col_varchar_key)
>> + )
>> + )
>> + ON (table3.col_int_nokey = table1.col_int_key)
>> + )
>> +WHERE table3.col_int_key != SOME ( SELECT c1 FROM t2 );
>
> This is a quite a bit more verbose than we normally do in our test
> files. I suggest removing as many newlines as possible while
> maintaining fair readability.
>