List:Commits« Previous MessageNext Message »
From:Øystein Grøvlen Date:May 19 2011 1:46pm
Subject:Re: bzr commit into mysql-trunk branch (olav.sandstaa:3366) Bug#12355958
View as plain text  
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;
 >
 >
 >
 >
 >

Thread
bzr commit into mysql-trunk branch (olav.sandstaa:3366) Bug#12355958Olav Sandstaa5 May
  • Re: bzr commit into mysql-trunk branch (olav.sandstaa:3366) Bug#12355958Øystein Grøvlen19 May
    • Re: bzr commit into mysql-trunk branch (olav.sandstaa:3366) Bug#12355958Olav Sandstaa20 May