Hi Olav,
Thanks for the patch. I have verified that it fixes the issues. I have
the following the questions to the patch:
- Why is it sufficient with an assert for some of
update_used_tables() functions? (I think the reason for that
should be added to the commit comments.)
- Did you forget Item_is_null::update_used_tables()?
I also think the commit comment for item_strfunc.cc should also be
added to the commit comment for item_cmpfunc.cc.
--
Øystein
(No comments in-line)
On 05/05/2011 14:49, Olav Sandstaa wrote:
> #At file:///export/home/tmp/mysql2/icp-subquery-bug-fix/ based on
revid:alexander.nozdrin@stripped
>
> 3366 Olav Sandstaa 2011-05-05
> 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() to update its value
> for with_subselect based on the value in its arguments.
> @ 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
> Add an assert to Item_func_make_set::update_used_tables()
> to verify that the value for with_subselect is the
> same as for the item it has a reference to.
> @ 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_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-05 12:49:15 +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-05 12:49:15 +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-05 12:49:15 +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-05 12:49:15 +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-05 12:49:15 +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-05 12:49:15 +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-05 12:49:15 +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-05 12:49:15 +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-04-26 08:49:10 +0000
> +++ b/sql/item_cmpfunc.cc 2011-05-05 12:49:15 +0000
> @@ -4603,11 +4603,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();
> +
> + // Propagte information about sub-queries
> + with_subselect|= item->with_subselect;
> }
> }
>
> @@ -4797,6 +4801,9 @@
> cached_value= (longlong) !args[0]->is_null();
> }
> }
> +
> + // Verify that with_subselect has been correctly propagated
> + DBUG_ASSERT(args[0]->with_subselect == with_subselect);
> }
>
>
> @@ -5821,6 +5828,9 @@
> used_tables_cache|= item->used_tables();
> /* see commentary at Item_equal::update_const() */
> const_item_cache&= item->const_item()&&
!item->is_outer_field();
> +
> + // Verify that with_subselect has been correctly propagated
> + DBUG_ASSERT(with_subselect == item->with_subselect);
> }
> }
>
>
> === modified file 'sql/item_func.cc'
> --- a/sql/item_func.cc 2011-04-26 20:35:24 +0000
> +++ b/sql/item_func.cc 2011-05-05 12:49:15 +0000
> @@ -403,11 +403,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-03-08 19:14:42 +0000
> +++ b/sql/item_row.cc 2011-05-05 12:49:15 +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-04-15 09:04:21 +0000
> +++ b/sql/item_strfunc.cc 2011-05-05 12:49:15 +0000
> @@ -2571,6 +2571,9 @@
> item->update_used_tables();
> used_tables_cache|=item->used_tables();
> const_item_cache&=item->const_item();
> +
> + // Verify that with_subselect is propagated correctly
> + DBUG_ASSERT(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-05 12:49:15 +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;
>
>
>
>
>
