List:Commits« Previous MessageNext Message »
From:Øystein Grøvlen Date:March 22 2011 2:48pm
Subject:Re: bzr commit into mysql-trunk branch (epotemkin:3003) Bug#11791705
View as plain text  
Hi Evgeny,

Thanks for the patch.  Changes looks good.  I have two issues with the 
patch:

1. You are tweaking the type system to try to fit function pointers
    into a List.  When you define a list as List<T>, push and pop
    functions takes *T as arguments. Since your base type is a function
    pointer, READ_RECORD::Setup_func, you should pass a pointer to a
    function pointer to these functions.  E.g,

      tab->save_read_first_record.push_front(&tab->read_first_record);

2. You should simplify the test case to make it more clear what kind
    of query is causing this issue.  I can reproduce the problem with
    the following test case:

    CREATE TABLE t1 (a INTEGER) ENGINE=InnoDB;
    INSERT INTO t1 VALUES (NULL);

    SELECT * FROM t1
    WHERE (a, a) NOT IN
      (SELECT * FROM (SELECT 8, 4 UNION SELECT 2, 3) tt) ;

    DROP TABLE t1;

--
Øystein

On 02/21/11 03:17 PM, Evgeny Potemkin wrote:
 > #At file:///work/bzrroot/wl5274-next-mr-bugfixing/ based on 
revid:epotemkin@stripped
 >
 >   3003 Evgeny Potemkin	2011-02-21
 >        Bug#11791705 - CRASH IN JOIN_MATERIALIZE_TABLE OR ASSERTION FAIL:
 >        !TAB->SAVE_READ_FIRST_RECORD
 >
 >        The JOIN_TAB::save_read_first_record field is used for 
disabling an optimization
 >        for IN subqueries and for materializing derived tables. Those 
two cases
 >        wasn't expected to happen in the same query, thus caused the 
assertion to
 >        fail.
 >        As the JOIN_TAB::save_read_first_record can't store two 
pointers at the same
 >        time it was substituted for the list of pointers to the 
read_first_record
 >        functions. The list is used as a stack.
 >       @ mysql-test/r/derived.result
 >          Added a test case for the bug#11791705.
 >       @ mysql-test/t/derived.test
 >          Added a test case for the bug#11791705.
 >       @ sql/item_subselect.cc
 >          Bug#11791705 - CRASH IN JOIN_MATERIALIZE_TABLE OR ASSERTION 
FAIL:
 >          !TAB->SAVE_READ_FIRST_RECORD
 >          To allow storing more that one pointer 
JOIN_TAB::save_read_first_record
 >          was substituted for the list of pointers, which is used as a 
stack.
 >       @ sql/sql_select.cc
 >          Bug#11791705 - CRASH IN JOIN_MATERIALIZE_TABLE OR ASSERTION 
FAIL:
 >          !TAB->SAVE_READ_FIRST_RECORD
 >          To allow storing more that one pointer 
JOIN_TAB::save_read_first_record
 >          was substituted for the list of pointers, which is used as a 
stack.
 >       @ sql/sql_select.h
 >          Bug#11791705 - CRASH IN JOIN_MATERIALIZE_TABLE OR ASSERTION 
FAIL:
 >          !TAB->SAVE_READ_FIRST_RECORD
 >          The JOIN_TAB::save_read_first_record field was substituted 
for the
 >          same named list.
 >
 >      modified:
 >        mysql-test/r/derived.result
 >        mysql-test/t/derived.test
 >        sql/item_subselect.cc
 >        sql/sql_select.cc
 >        sql/sql_select.h
 > === modified file 'mysql-test/r/derived.result'
 > --- a/mysql-test/r/derived.result	2011-02-18 14:42:19 +0000
 > +++ b/mysql-test/r/derived.result	2011-02-21 14:17:57 +0000
 > @@ -1388,3 +1388,70 @@ GROUP BY field2 ;
 >   field1	field2
 >   1	2
 >   DROP TABLE t1,t2;
 > +#
 > +# Bug#11791705 - CRASH IN JOIN_MATERIALIZE_TABLE OR ASSERTION FAIL:
 > +#                !TAB->SAVE_READ_FIRST_RECORD
 > +#
 > +CREATE TABLE t1 (
 > +pk INT NOT NULL AUTO_INCREMENT,
 > +col_int_nokey INT,
 > +col_int_key INT,
 > +PRIMARY KEY (pk),
 > +KEY col_int_key (col_int_key)
 > +) ENGINE=InnoDB;
 > +INSERT INTO t1 VALUES
 > +(10,7,8),
 > +(11,1,9),
 > +(12,5,9),
 > +(13,3,186),
 > +(14,6,NULL)
 > +;
 > +CREATE TABLE t2 (
 > +pk INT NOT NULL AUTO_INCREMENT,
 > +col_int_key INT,
 > +PRIMARY KEY (pk),
 > +KEY col_int_key (col_int_key)
 > +) ENGINE=InnoDB;
 > +INSERT INTO t2 VALUES (10,8);
 > +SELECT alias2.col_int_nokey
 > +FROM t2 AS alias1
 > +RIGHT JOIN t1 AS alias2
 > +JOIN t1 AS alias3
 > +ON alias3.pk ON alias2.pk
 > +WHERE
 > +( alias3.col_int_key , alias1.col_int_key )
 > +NOT IN
 > +(
 > +SELECT *
 > +FROM (SELECT 8 AS `8`, 4 AS `4` UNION SELECT 2 AS `2`, 3 AS `3`) tt
 > +)
 > +;
 > +col_int_nokey
 > +7
 > +1
 > +5
 > +3
 > +6
 > +7
 > +1
 > +5
 > +3
 > +6
 > +7
 > +1
 > +5
 > +3
 > +6
 > +7
 > +1
 > +5
 > +3
 > +6
 > +7
 > +1
 > +5
 > +3
 > +6
 > +DROP TABLE t1;
 > +DROP TABLE t2;
 > +#
 >
 > === modified file 'mysql-test/t/derived.test'
 > --- a/mysql-test/t/derived.test	2011-02-18 14:42:19 +0000
 > +++ b/mysql-test/t/derived.test	2011-02-21 14:17:57 +0000
 > @@ -758,3 +758,51 @@ GROUP BY field2 ;
 >
 >   DROP TABLE t1,t2;
 >
 > +--echo #
 > +--echo # Bug#11791705 - CRASH IN JOIN_MATERIALIZE_TABLE OR ASSERTION 
FAIL:
 > +--echo #                !TAB->SAVE_READ_FIRST_RECORD
 > +--echo #
 > +
 > +CREATE TABLE t1 (
 > +  pk INT NOT NULL AUTO_INCREMENT,
 > +  col_int_nokey INT,
 > +  col_int_key INT,
 > +  PRIMARY KEY (pk),
 > +  KEY col_int_key (col_int_key)
 > +) ENGINE=InnoDB;
 > +
 > +INSERT INTO t1 VALUES
 > +  (10,7,8),
 > +  (11,1,9),
 > +  (12,5,9),
 > +  (13,3,186),
 > +  (14,6,NULL)
 > +;
 > +
 > +CREATE TABLE t2 (
 > +  pk INT NOT NULL AUTO_INCREMENT,
 > +  col_int_key INT,
 > +  PRIMARY KEY (pk),
 > +  KEY col_int_key (col_int_key)
 > +) ENGINE=InnoDB;
 > +
 > +INSERT INTO t2 VALUES (10,8);
 > +
 > +SELECT alias2.col_int_nokey
 > +FROM t2 AS alias1
 > +  RIGHT JOIN t1 AS alias2
 > +    JOIN t1 AS alias3
 > +    ON alias3.pk ON alias2.pk
 > +WHERE
 > +  ( alias3.col_int_key , alias1.col_int_key )
 > + NOT IN
 > +  (
 > +   SELECT *
 > +   FROM (SELECT 8 AS `8`, 4 AS `4` UNION SELECT 2 AS `2`, 3 AS `3`) tt
 > +  )
 > +;
 > +
 > +DROP TABLE t1;
 > +DROP TABLE t2;
 > +--echo #
 > +
 >
 > === modified file 'sql/item_subselect.cc'
 > --- a/sql/item_subselect.cc	2011-02-18 10:46:59 +0000
 > +++ b/sql/item_subselect.cc	2011-02-21 14:17:57 +0000
 > @@ -2370,12 +2370,27 @@ int subselect_single_select_engine::exec
 >                   Make sure that save_read_first_record usage doesn't
 >                   intersect with join_materialize_table()'s.
 >                 */
 > -              DBUG_ASSERT(!tab->save_read_first_record);
 >                 /* Change the access method to full table scan */
 > -              tab->save_read_first_record= tab->read_first_record;
 > +              if (tab->save_read_first_record.elements)
 > +              {
 > +                TABLE_LIST *tl= tab->table->pos_in_table_list;
 > +                /*
 > +                  This is a materializable derived table and it's not
 > +                  materialized yet.
 > +                */
 > +                DBUG_ASSERT(tl->uses_materialization()&& 
!tl->materialized);
 > +                // Preserve join_materialize_table to be the first 
handler
 > +                tab->save_read_first_record.push_front(
 > +                  (READ_RECORD::Setup_func*)read_first_record_seq);
 > +              }
 > +              else
 > +              {
 > +                tab->save_read_first_record.push_front(
 > +                  (READ_RECORD::Setup_func*)tab->read_first_record);
 > +                tab->read_first_record= read_first_record_seq;
 > +              }
 >                 tab->save_read_record= tab->read_record.read_record;
 >                 tab->read_record.read_record= rr_sequential;
 > -              tab->read_first_record= read_first_record_seq;
 >                 tab->read_record.record= tab->table->record[0];
 >                 tab->read_record.thd= join->thd;
 >                 tab->read_record.ref_length= 
tab->table->file->ref_length;
 > @@ -2396,9 +2411,10 @@ int subselect_single_select_engine::exec
 >         JOIN_TAB *tab= *ptab;
 >         tab->read_record.record= 0;
 >         tab->read_record.ref_length= 0;
 > -      tab->read_first_record= tab->save_read_first_record;
 > +      DBUG_ASSERT(tab->save_read_first_record.elements);
 > +      tab->read_first_record=
 > +        (READ_RECORD::Setup_func)tab->save_read_first_record.pop();
 >         tab->read_record.read_record= tab->save_read_record;
 > -      tab->save_read_first_record= NULL;
 >       }
 >       executed= true;
 >
 >
 > === modified file 'sql/sql_select.cc'
 > --- a/sql/sql_select.cc	2011-02-17 12:14:26 +0000
 > +++ b/sql/sql_select.cc	2011-02-21 14:17:57 +0000
 > @@ -11415,7 +11415,8 @@ make_join_readinfo(JOIN *join, ulonglong
 >       if (tab->table->pos_in_table_list->uses_materialization()&&
 >           !tab->table->pos_in_table_list->materialized)
 >       {
 > -      tab->save_read_first_record= tab->read_first_record;
 > +      tab->save_read_first_record.push_front(
 > +        (READ_RECORD::Setup_func*)tab->read_first_record);
 >         tab->read_first_record= join_materialize_table;
 >       }
 >     }
 > @@ -18517,8 +18518,8 @@ join_materialize_table(JOIN_TAB *tab)
 >                                 derived,&mysql_derived_cleanup);
 >     if (res)
 >       return -1;
 > -  tab->read_first_record= tab->save_read_first_record;
 > -  tab->save_read_first_record= NULL;
 > +  tab->read_first_record=
 > +    (READ_RECORD::Setup_func)tab->save_read_first_record.pop();
 >     return (*tab->read_first_record)(tab);
 >   }
 >
 >
 > === modified file 'sql/sql_select.h'
 > --- a/sql/sql_select.h	2011-02-18 10:46:59 +0000
 > +++ b/sql/sql_select.h	2011-02-21 14:17:57 +0000
 > @@ -273,7 +273,8 @@ typedef struct st_join_table : public Sq
 >       the subquery predicate is evaluated to NULL.
 >       save_read_first_record is also used during derived tables 
materialization.
 >     */
 > -  READ_RECORD::Setup_func save_read_first_record;/* to save 
read_first_record */
 > +  /* to save read_first_record */
 > +  List<READ_RECORD::Setup_func>  save_read_first_record;
 >     READ_RECORD::Read_func save_read_record;/* to save 
read_record.read_record */
 >     double	worst_seeks;
 >     key_map	const_keys;			/**<  Keys with constant part */
 > @@ -456,7 +457,6 @@ st_join_table::st_join_table()
 >       read_first_record(NULL),
 >       next_select(NULL),
 >       read_record(),
 > -    save_read_first_record(NULL),
 >       save_read_record(NULL),
 >       worst_seeks(0.0),
 >       const_keys(),
 >
 >
 >
 >
 >
Thread
bzr commit into mysql-trunk branch (epotemkin:3003) Bug#11791705Evgeny Potemkin21 Feb
  • Re: bzr commit into mysql-trunk branch (epotemkin:3003) Bug#11791705Øystein Grøvlen22 Mar
    • Re: bzr commit into mysql-trunk branch (epotemkin:3003) Bug#11791705Evgeny Potemkin23 Mar
      • Re: bzr commit into mysql-trunk branch (epotemkin:3003) Bug#11791705Øystein Grøvlen24 Mar
  • Re: bzr commit into mysql-trunk branch (epotemkin:3003) Bug#11791705Øystein Grøvlen22 Mar