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(),
>
>
>
>
>
