Hi Øystein,
Thanks for reviewing and approving the updated patch.
Olav
On 20/05/2011 14:41, Øystein Grøvlen wrote:
> Looks good.
>
> Approved,
>
> --
> Øystein
>
>
>
> On 20/05/2011 13:59, Olav Sandstaa wrote:
>> #At file:///export/home/tmp/mysql2/icp-subquery-bug-fix/ based on
>> revid:mayank.prasad@stripped
>>
>> 3377 Olav Sandstaa 2011-05-20
>> Fix for bug Bug#12355958 "FAILING ASSERTION:
>> TRX->LOCK.N_ACTIVE_THRS == 1".
>>
>> The assert that crashes in InnoDB is caused by the ICP code
>> pushing
>> down an index condition that contains a subquery. When InnoDB
>> evaluates this index condition the subquery is executed and
>> this results in
>> a second access to data in InnoDB. InnoDB does not allow a
>> transaction
>> to have more than one concurrent executing table operation and
>> this
>> causes the assert to trigger.
>>
>> The ICP code should not have pushed down this index condition
>> to InnoDB
>> since it contains a subquery. The existing ICP code checks for
>> subqueries in
>> the condition by checking the value of the condition's
>> with_subselect
>> member. In this case the value for the Item_func condition's
>> with_subselect
>> is false even though one of the arguments for the Item_func is
>> a subquery.
>> This situation can occur when we initially create the item tree
>> for a condition and then later substitutes non-subquery parts
>> of it
>> with a subquery. In the test case for this bug this occurs when
>> optimize_conds() replaces a field reference with a subquery
>> due to the
>> subquery will be const when it is evaluated. This replacement
>> is done
>> without updating the Item_func node's with_subselect to true.
>>
>> The fix for this would be easy if it was possible to update the
>> with_subselect member locally when doing changes. What makes this
>> more complex is that this information needs to be propagated
>> upwards in the item tree. Since we have no "parent reference" in
>> the item tree this can not be done locally. In order to be able
>> to propagate this information upwards in the item tree this patch
>> extends Item::update_uses_tables() to also update
>> the value of with_subselect when traversing the item tree.
>> @ mysql-test/include/icp_tests.inc
>> Test for Bug#12355958 "FAILING ASSERTION:
>> TRX->LOCK.N_ACTIVE_THRS == 1".
>> @ mysql-test/r/innodb_icp.result
>> Test for Bug#12355958 "FAILING ASSERTION:
>> TRX->LOCK.N_ACTIVE_THRS == 1".
>> @ mysql-test/r/innodb_icp_all.result
>> Test for Bug#12355958 "FAILING ASSERTION:
>> TRX->LOCK.N_ACTIVE_THRS == 1".
>> @ mysql-test/r/innodb_icp_none.result
>> Test for Bug#12355958 "FAILING ASSERTION:
>> TRX->LOCK.N_ACTIVE_THRS == 1".
>> @ mysql-test/r/myisam_icp.result
>> Test for Bug#12355958 "FAILING ASSERTION:
>> TRX->LOCK.N_ACTIVE_THRS == 1".
>> @ mysql-test/r/myisam_icp_all.result
>> Test for Bug#12355958 "FAILING ASSERTION:
>> TRX->LOCK.N_ACTIVE_THRS == 1".
>> @ mysql-test/r/myisam_icp_none.result
>> Test for Bug#12355958 "FAILING ASSERTION:
>> TRX->LOCK.N_ACTIVE_THRS == 1".
>> @ sql/item.h
>> Extend Item_ref::update_used_tables() to propagate the value of
>> with_subselect.
>> @ sql/item_cmpfunc.cc
>> Extend Item_cond::update_used_tables(),
>> Item_is_not_null_test::update_used_tables(), and
>> Item_equal::update_used_tables() to update its value
>> for with_subselect based on the value in its arguments.
>> @ sql/item_cmpfunc.h
>> Extend Item_func_isnull::update_used_tables() to propagate
>> the value of with_subselect in the item tree.
>> @ sql/item_func.cc
>> Extend Item_func::update_used_tables() to update its value
>> for with_subselect based on the value in its arguments.
>> @ sql/item_row.cc
>> Extend Item_row::update_used_tables() to update its value
>> for with_subselect based on the value in its arguments.
>> @ sql/item_strfunc.cc
>> Extend Item_func_make_set::update_used_tables() to update its
>> value for with_subselect based on the value in its item object.
>> @ sql/item_sum.cc
>> Extend Item_sum::update_used_tables() to update its value
>> for with_subselect based on the value in its arguments.
>>
>> 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/item.h
>> sql/item_cmpfunc.cc
>> sql/item_cmpfunc.h
>> sql/item_func.cc
>> sql/item_row.cc
>> sql/item_strfunc.cc
>> sql/item_sum.cc
>> === modified file 'mysql-test/include/icp_tests.inc'
>>
>> === modified file 'mysql-test/include/icp_tests.inc'
>> --- a/mysql-test/include/icp_tests.inc 2011-03-24 13:16:36 +0000
>> +++ b/mysql-test/include/icp_tests.inc 2011-05-20 11:58:59 +0000
>> @@ -924,3 +924,27 @@
>> eval $query;
>>
>> DROP TABLE t1, t2, t3;
>> +
>> +--echo #
>> +--echo # Bug#12355958 "FAILING ASSERTION: TRX->LOCK.N_ACTIVE_THRS == 1"
>> +--echo #
>> +
>> +CREATE TABLE t1 (
>> + pk INTEGER PRIMARY KEY,
>> + a INTEGER NOT NULL,
>> + b CHAR(1),
>> + KEY(b)
>> +);
>> +
>> +INSERT INTO t1 VALUES (23,5,'d');
>> +
>> +let query=
>> +SELECT a1.pk
>> +FROM t1 AS a1 JOIN (SELECT * FROM t1 LIMIT 1) AS a2 ON a2.b = a1.b
>> +WHERE a1.a = (SELECT pk FROM t1 LIMIT 1)
>> + AND (a1.a != a2.a OR a1.b IS NULL);
>> +
>> +eval EXPLAIN $query;
>> +eval $query;
>> +
>> +DROP TABLE t1;
>>
>> === modified file 'mysql-test/r/innodb_icp.result'
>> --- a/mysql-test/r/innodb_icp.result 2011-03-24 13:16:36 +0000
>> +++ b/mysql-test/r/innodb_icp.result 2011-05-20 11:58:59 +0000
>> @@ -871,5 +871,30 @@
>> 1
>> 2
>> DROP TABLE t1, t2, t3;
>> +#
>> +# Bug#12355958 "FAILING ASSERTION: TRX->LOCK.N_ACTIVE_THRS == 1"
>> +#
>> +CREATE TABLE t1 (
>> +pk INTEGER PRIMARY KEY,
>> +a INTEGER NOT NULL,
>> +b CHAR(1),
>> +KEY(b)
>> +);
>> +INSERT INTO t1 VALUES (23,5,'d');
>> +EXPLAIN SELECT a1.pk
>> +FROM t1 AS a1 JOIN (SELECT * FROM t1 LIMIT 1) AS a2 ON a2.b = a1.b
>> +WHERE a1.a = (SELECT pk FROM t1 LIMIT 1)
>> +AND (a1.a != a2.a OR a1.b IS NULL);
>> +id select_type table type possible_keys key
>> key_len ref rows Extra
>> +1 PRIMARY <derived2> system NULL NULL NULL NULL 1
>> +1 PRIMARY a1 ref b b 2 const 1 Using where
>> +3 SUBQUERY t1 index NULL b 2 NULL 1 Using
>> index
>> +2 DERIVED t1 ALL NULL NULL NULL NULL 1
>> +SELECT a1.pk
>> +FROM t1 AS a1 JOIN (SELECT * FROM t1 LIMIT 1) AS a2 ON a2.b = a1.b
>> +WHERE a1.a = (SELECT pk FROM t1 LIMIT 1)
>> +AND (a1.a != a2.a OR a1.b IS NULL);
>> +pk
>> +DROP TABLE t1;
>> set default_storage_engine= @save_storage_engine;
>> set optimizer_switch=default;
>>
>> === modified file 'mysql-test/r/innodb_icp_all.result'
>> --- a/mysql-test/r/innodb_icp_all.result 2011-03-29 08:10:26 +0000
>> +++ b/mysql-test/r/innodb_icp_all.result 2011-05-20 11:58:59 +0000
>> @@ -871,5 +871,30 @@
>> 1
>> 2
>> DROP TABLE t1, t2, t3;
>> +#
>> +# Bug#12355958 "FAILING ASSERTION: TRX->LOCK.N_ACTIVE_THRS == 1"
>> +#
>> +CREATE TABLE t1 (
>> +pk INTEGER PRIMARY KEY,
>> +a INTEGER NOT NULL,
>> +b CHAR(1),
>> +KEY(b)
>> +);
>> +INSERT INTO t1 VALUES (23,5,'d');
>> +EXPLAIN SELECT a1.pk
>> +FROM t1 AS a1 JOIN (SELECT * FROM t1 LIMIT 1) AS a2 ON a2.b = a1.b
>> +WHERE a1.a = (SELECT pk FROM t1 LIMIT 1)
>> +AND (a1.a != a2.a OR a1.b IS NULL);
>> +id select_type table type possible_keys key
>> key_len ref rows Extra
>> +1 PRIMARY <derived2> system NULL NULL NULL NULL 1
>> +1 PRIMARY a1 ref b b 2 const 1 Using where
>> +3 SUBQUERY t1 index NULL b 2 NULL 1 Using
>> index
>> +2 DERIVED t1 ALL NULL NULL NULL NULL 1
>> +SELECT a1.pk
>> +FROM t1 AS a1 JOIN (SELECT * FROM t1 LIMIT 1) AS a2 ON a2.b = a1.b
>> +WHERE a1.a = (SELECT pk FROM t1 LIMIT 1)
>> +AND (a1.a != a2.a OR a1.b IS NULL);
>> +pk
>> +DROP TABLE t1;
>> set default_storage_engine= @save_storage_engine;
>> set optimizer_switch=default;
>>
>> === modified file 'mysql-test/r/innodb_icp_none.result'
>> --- a/mysql-test/r/innodb_icp_none.result 2011-03-24 13:16:36 +0000
>> +++ b/mysql-test/r/innodb_icp_none.result 2011-05-20 11:58:59 +0000
>> @@ -870,5 +870,30 @@
>> 1
>> 2
>> DROP TABLE t1, t2, t3;
>> +#
>> +# Bug#12355958 "FAILING ASSERTION: TRX->LOCK.N_ACTIVE_THRS == 1"
>> +#
>> +CREATE TABLE t1 (
>> +pk INTEGER PRIMARY KEY,
>> +a INTEGER NOT NULL,
>> +b CHAR(1),
>> +KEY(b)
>> +);
>> +INSERT INTO t1 VALUES (23,5,'d');
>> +EXPLAIN SELECT a1.pk
>> +FROM t1 AS a1 JOIN (SELECT * FROM t1 LIMIT 1) AS a2 ON a2.b = a1.b
>> +WHERE a1.a = (SELECT pk FROM t1 LIMIT 1)
>> +AND (a1.a != a2.a OR a1.b IS NULL);
>> +id select_type table type possible_keys key
>> key_len ref rows Extra
>> +1 PRIMARY <derived2> system NULL NULL NULL NULL 1
>> +1 PRIMARY a1 ref b b 2 const 1 Using where
>> +3 SUBQUERY t1 index NULL b 2 NULL 1 Using
>> index
>> +2 DERIVED t1 ALL NULL NULL NULL NULL 1
>> +SELECT a1.pk
>> +FROM t1 AS a1 JOIN (SELECT * FROM t1 LIMIT 1) AS a2 ON a2.b = a1.b
>> +WHERE a1.a = (SELECT pk FROM t1 LIMIT 1)
>> +AND (a1.a != a2.a OR a1.b IS NULL);
>> +pk
>> +DROP TABLE t1;
>> set default_storage_engine= @save_storage_engine;
>> set optimizer_switch=default;
>>
>> === modified file 'mysql-test/r/myisam_icp.result'
>> --- a/mysql-test/r/myisam_icp.result 2011-03-24 13:16:36 +0000
>> +++ b/mysql-test/r/myisam_icp.result 2011-05-20 11:58:59 +0000
>> @@ -868,4 +868,28 @@
>> 1
>> 2
>> DROP TABLE t1, t2, t3;
>> +#
>> +# Bug#12355958 "FAILING ASSERTION: TRX->LOCK.N_ACTIVE_THRS == 1"
>> +#
>> +CREATE TABLE t1 (
>> +pk INTEGER PRIMARY KEY,
>> +a INTEGER NOT NULL,
>> +b CHAR(1),
>> +KEY(b)
>> +);
>> +INSERT INTO t1 VALUES (23,5,'d');
>> +EXPLAIN SELECT a1.pk
>> +FROM t1 AS a1 JOIN (SELECT * FROM t1 LIMIT 1) AS a2 ON a2.b = a1.b
>> +WHERE a1.a = (SELECT pk FROM t1 LIMIT 1)
>> +AND (a1.a != a2.a OR a1.b IS NULL);
>> +id select_type table type possible_keys key
>> key_len ref rows Extra
>> +1 PRIMARY NULL NULL NULL NULL NULL NULL
>> NULL Impossible WHERE noticed after reading const tables
>> +3 SUBQUERY t1 system NULL NULL NULL NULL 1
>> +2 DERIVED t1 system NULL NULL NULL NULL 1
>> +SELECT a1.pk
>> +FROM t1 AS a1 JOIN (SELECT * FROM t1 LIMIT 1) AS a2 ON a2.b = a1.b
>> +WHERE a1.a = (SELECT pk FROM t1 LIMIT 1)
>> +AND (a1.a != a2.a OR a1.b IS NULL);
>> +pk
>> +DROP TABLE t1;
>> set optimizer_switch=default;
>>
>> === modified file 'mysql-test/r/myisam_icp_all.result'
>> --- a/mysql-test/r/myisam_icp_all.result 2011-03-29 08:10:26 +0000
>> +++ b/mysql-test/r/myisam_icp_all.result 2011-05-20 11:58:59 +0000
>> @@ -868,4 +868,28 @@
>> 1
>> 2
>> DROP TABLE t1, t2, t3;
>> +#
>> +# Bug#12355958 "FAILING ASSERTION: TRX->LOCK.N_ACTIVE_THRS == 1"
>> +#
>> +CREATE TABLE t1 (
>> +pk INTEGER PRIMARY KEY,
>> +a INTEGER NOT NULL,
>> +b CHAR(1),
>> +KEY(b)
>> +);
>> +INSERT INTO t1 VALUES (23,5,'d');
>> +EXPLAIN SELECT a1.pk
>> +FROM t1 AS a1 JOIN (SELECT * FROM t1 LIMIT 1) AS a2 ON a2.b = a1.b
>> +WHERE a1.a = (SELECT pk FROM t1 LIMIT 1)
>> +AND (a1.a != a2.a OR a1.b IS NULL);
>> +id select_type table type possible_keys key
>> key_len ref rows Extra
>> +1 PRIMARY NULL NULL NULL NULL NULL NULL
>> NULL Impossible WHERE noticed after reading const tables
>> +3 SUBQUERY t1 system NULL NULL NULL NULL 1
>> +2 DERIVED t1 system NULL NULL NULL NULL 1
>> +SELECT a1.pk
>> +FROM t1 AS a1 JOIN (SELECT * FROM t1 LIMIT 1) AS a2 ON a2.b = a1.b
>> +WHERE a1.a = (SELECT pk FROM t1 LIMIT 1)
>> +AND (a1.a != a2.a OR a1.b IS NULL);
>> +pk
>> +DROP TABLE t1;
>> set optimizer_switch=default;
>>
>> === modified file 'mysql-test/r/myisam_icp_none.result'
>> --- a/mysql-test/r/myisam_icp_none.result 2011-03-24 13:16:36 +0000
>> +++ b/mysql-test/r/myisam_icp_none.result 2011-05-20 11:58:59 +0000
>> @@ -867,4 +867,28 @@
>> 1
>> 2
>> DROP TABLE t1, t2, t3;
>> +#
>> +# Bug#12355958 "FAILING ASSERTION: TRX->LOCK.N_ACTIVE_THRS == 1"
>> +#
>> +CREATE TABLE t1 (
>> +pk INTEGER PRIMARY KEY,
>> +a INTEGER NOT NULL,
>> +b CHAR(1),
>> +KEY(b)
>> +);
>> +INSERT INTO t1 VALUES (23,5,'d');
>> +EXPLAIN SELECT a1.pk
>> +FROM t1 AS a1 JOIN (SELECT * FROM t1 LIMIT 1) AS a2 ON a2.b = a1.b
>> +WHERE a1.a = (SELECT pk FROM t1 LIMIT 1)
>> +AND (a1.a != a2.a OR a1.b IS NULL);
>> +id select_type table type possible_keys key
>> key_len ref rows Extra
>> +1 PRIMARY NULL NULL NULL NULL NULL NULL
>> NULL Impossible WHERE noticed after reading const tables
>> +3 SUBQUERY t1 system NULL NULL NULL NULL 1
>> +2 DERIVED t1 system NULL NULL NULL NULL 1
>> +SELECT a1.pk
>> +FROM t1 AS a1 JOIN (SELECT * FROM t1 LIMIT 1) AS a2 ON a2.b = a1.b
>> +WHERE a1.a = (SELECT pk FROM t1 LIMIT 1)
>> +AND (a1.a != a2.a OR a1.b IS NULL);
>> +pk
>> +DROP TABLE t1;
>> set optimizer_switch=default;
>>
>> === modified file 'sql/item.h'
>> --- a/sql/item.h 2011-04-08 13:41:38 +0000
>> +++ b/sql/item.h 2011-05-20 11:58:59 +0000
>> @@ -2623,8 +2623,13 @@
>> }
>> void update_used_tables()
>> {
>> - if (!depended_from)
>> - (*ref)->update_used_tables();
>> + if (!depended_from)
>> + {
>> + (*ref)->update_used_tables();
>> +
>> + // Propagate information about sub-queries
>> + with_subselect= (*ref)->with_subselect;
>> + }
>> }
>> virtual table_map resolved_used_tables() const;
>> table_map not_null_tables() const { return
>> (*ref)->not_null_tables(); }
>>
>> === modified file 'sql/item_cmpfunc.cc'
>> --- a/sql/item_cmpfunc.cc 2011-05-18 10:43:46 +0000
>> +++ b/sql/item_cmpfunc.cc 2011-05-20 11:58:59 +0000
>> @@ -4615,11 +4615,15 @@
>>
>> used_tables_cache=0;
>> const_item_cache=1;
>> + with_subselect= false;
>> while ((item=li++))
>> {
>> item->update_used_tables();
>> used_tables_cache|= item->used_tables();
>> const_item_cache&= item->const_item();
>> +
>> + // Propagate information about sub-queries
>> + with_subselect|= item->with_subselect;
>> }
>> }
>>
>> @@ -4803,6 +4807,10 @@
>> else
>> {
>> args[0]->update_used_tables();
>> +
>> + // Propagate information about sub-queries
>> + with_subselect= args[0]->with_subselect;
>> +
>> if (!(used_tables_cache=args[0]->used_tables())&&
>> !with_subselect)
>> {
>> /* Remember if the value is always NULL or never NULL */
>> @@ -5827,12 +5835,16 @@
>> not_null_tables_cache= used_tables_cache= 0;
>> if ((const_item_cache= cond_false))
>> return;
>> + with_subselect= false;
>> while ((item=li++))
>> {
>> item->update_used_tables();
>> used_tables_cache|= item->used_tables();
>> /* see commentary at Item_equal::update_const() */
>> const_item_cache&= item->const_item()&&
> !item->is_outer_field();
>> +
>> + // Propagate information about sub_queries
>> + with_subselect|= item->with_subselect;
>> }
>> }
>>
>>
>> === modified file 'sql/item_cmpfunc.h'
>> --- a/sql/item_cmpfunc.h 2011-03-22 11:44:40 +0000
>> +++ b/sql/item_cmpfunc.h 2011-05-20 11:58:59 +0000
>> @@ -1363,6 +1363,10 @@
>> else
>> {
>> args[0]->update_used_tables();
>> +
>> + // Propagate information about sub-queries
>> + with_subselect= args[0]->with_subselect;
>> +
>> if ((const_item_cache= !(used_tables_cache=
>> args[0]->used_tables())&&
>> !with_subselect))
>> {
>>
>> === modified file 'sql/item_func.cc'
>> --- a/sql/item_func.cc 2011-05-12 18:44:56 +0000
>> +++ b/sql/item_func.cc 2011-05-20 11:58:59 +0000
>> @@ -405,11 +405,15 @@
>> {
>> used_tables_cache=0;
>> const_item_cache=1;
>> + with_subselect= false;
>> for (uint i=0 ; i< arg_count ; i++)
>> {
>> args[i]->update_used_tables();
>> used_tables_cache|=args[i]->used_tables();
>> const_item_cache&=args[i]->const_item();
>> +
>> + // Propagate information about sub-queries
>> + with_subselect|= args[i]->with_subselect;
>> }
>> }
>>
>>
>> === modified file 'sql/item_row.cc'
>> --- a/sql/item_row.cc 2011-05-06 13:32:53 +0000
>> +++ b/sql/item_row.cc 2011-05-20 11:58:59 +0000
>> @@ -126,11 +126,15 @@
>> {
>> used_tables_cache= 0;
>> const_item_cache= 1;
>> + with_subselect= false;
>> for (uint i= 0; i< arg_count; i++)
>> {
>> items[i]->update_used_tables();
>> used_tables_cache|= items[i]->used_tables();
>> const_item_cache&= items[i]->const_item();
>> +
>> + // Propagate information about sub-queries
>> + with_subselect|= items[i]->with_subselect;
>> }
>> }
>>
>>
>> === modified file 'sql/item_strfunc.cc'
>> --- a/sql/item_strfunc.cc 2011-05-06 13:32:53 +0000
>> +++ b/sql/item_strfunc.cc 2011-05-20 11:58:59 +0000
>> @@ -2571,6 +2571,9 @@
>> item->update_used_tables();
>> used_tables_cache|=item->used_tables();
>> const_item_cache&=item->const_item();
>> +
>> + // Propagage information about sub-queries
>> + with_subselect= item->with_subselect;
>> }
>>
>>
>>
>> === modified file 'sql/item_sum.cc'
>> --- a/sql/item_sum.cc 2011-04-12 10:31:30 +0000
>> +++ b/sql/item_sum.cc 2011-05-20 11:58:59 +0000
>> @@ -529,10 +529,14 @@
>> if (!forced_const)
>> {
>> used_tables_cache= 0;
>> + with_subselect= false;
>> for (uint i=0 ; i< arg_count ; i++)
>> {
>> args[i]->update_used_tables();
>> used_tables_cache|= args[i]->used_tables();
>> +
>> + // Propagate information about sub-queries
>> + with_subselect|= args[i]->with_subselect;
>> }
>>
>> used_tables_cache&= PSEUDO_TABLE_BITS;
>>
>>
>>
>>
>>
>
>