From: Date: July 29 2008 12:51am Subject: bzr commit into mysql-6.0-opt branch (sergefp:2679) Bug#37842 List-Archive: http://lists.mysql.com/commits/50614 X-Bug: 37842 Message-Id: <20080728225145.7956D22AF5F@pslp.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit #At file:///home/psergey/dev/mysql-6.0-opt-r2/ 2679 Sergey Petrunia 2008-07-29 BUG#37842: Assertion in DsMrr_impl::dsmrr_init, at handler.cc:4307 - Make DsMrr_impl::dsmrr_init() handle the situation when the SQL layer re-starts the MRR scan - without having got eof, it can call h->ha_index_or_rnd_end(); h->ha_index_init(); h->multi_range_read_init(). - Remove unneded parameters from dsmrr_init() and dsmrr_next(). - Don't reserve buffer space for key tuple in dsmrr_init() as it is not needed anymore (we used key tuple to save/restore MRR scan position but we've switched to cloning handler objects). modified: mysql-test/r/myisam_mrr.result mysql-test/r/subselect3.result mysql-test/t/subselect3.test sql/handler.cc sql/handler.h storage/innobase/handler/ha_innodb.cc storage/myisam/ha_myisam.cc per-file messages: mysql-test/r/myisam_mrr.result BUG#37842: Assertion in DsMrr_impl::dsmrr_init, at handler.cc:4307 - Update test results (order changed since we no longer take space for key tuple from the MRR buffer) mysql-test/r/subselect3.result BUG#37842: Assertion in DsMrr_impl::dsmrr_init, at handler.cc:4307 - Testcase mysql-test/t/subselect3.test BUG#37842: Assertion in DsMrr_impl::dsmrr_init, at handler.cc:4307 - Testcase sql/handler.cc BUG#37842: Assertion in DsMrr_impl::dsmrr_init, at handler.cc:4307 - Make DsMrr_impl::dsmrr_init() handle the situation when the SQL layer re-starts the MRR scan - without having got eof, it can call h->ha_index_or_rnd_end(); h->ha_index_init(); h->multi_range_read_init(). - Remove unneded parameters from dsmrr_init() and dsmrr_next(). - Don't reserve buffer space for key tuple in dsmrr_init() as it is not needed anymore (we used key tuple to save/restore MRR scan position but we've switched to cloning handler objects). sql/handler.h BUG#37842: Assertion in DsMrr_impl::dsmrr_init, at handler.cc:4307 - Correct comments for DsMrr_impl::{h,h2} - Remove unneded parameters from dsmrr_init() and dsmrr_next(). storage/innobase/handler/ha_innodb.cc BUG#37842: Assertion in DsMrr_impl::dsmrr_init, at handler.cc:4307 - Remove junk comments - Update DS-MRR function calls - removed redundant parameters storage/myisam/ha_myisam.cc BUG#37842: Assertion in DsMrr_impl::dsmrr_init, at handler.cc:4307 - Remove junk comments - Update DS-MRR function calls - removed redundant parameters === modified file 'mysql-test/r/myisam_mrr.result' --- a/mysql-test/r/myisam_mrr.result 2008-01-10 22:04:59 +0000 +++ b/mysql-test/r/myisam_mrr.result 2008-07-28 22:51:38 +0000 @@ -29,15 +29,15 @@ a filler c-1011=w filler c-1012=w filler c-1013=w filler +c-1014=w filler c-1011=w filler-1 c-1012=w filler-1 c-1013=w filler-1 +c-1014=w filler-1 c-1011=w filler-2 c-1012=w filler-2 c-1013=w filler-2 -c-1014=w filler c-1015=w filler -c-1014=w filler-1 c-1015=w filler-1 c-1014=w filler-2 c-1015=w filler-2 @@ -47,15 +47,15 @@ a filler c-1011=w filler c-1012=w filler c-1013=w filler +c-1014=w filler c-1011=w filler-1 c-1012=w filler-1 c-1013=w filler-1 +c-1014=w filler-1 c-1011=w filler-2 c-1012=w filler-2 c-1013=w filler-2 -c-1014=w filler c-1015=w filler -c-1014=w filler-1 c-1015=w filler-1 c-1014=w filler-2 c-1015=w filler-2 @@ -67,15 +67,15 @@ a filler c-1011=w filler c-1012=w filler c-1013=w filler +c-1014=w filler c-1011=w filler-1 c-1012=w filler-1 c-1013=w filler-1 +c-1014=w filler-1 c-1011=w filler-2 c-1012=w filler-2 c-1013=w filler-2 -c-1014=w filler c-1015=w filler -c-1014=w filler-1 c-1015=w filler-1 c-1014=w filler-2 c-1015=w filler-2 @@ -86,15 +86,15 @@ a filler c-1011=w filler c-1012=w filler c-1013=w filler +c-1014=w filler c-1011=w filler-1 c-1012=w filler-1 c-1013=w filler-1 +c-1014=w filler-1 c-1011=w filler-2 c-1012=w filler-2 c-1013=w filler-2 -c-1014=w filler c-1015=w filler -c-1014=w filler-1 c-1015=w filler-1 c-1014=w filler-2 c-1015=w filler-2 @@ -105,19 +105,19 @@ a filler c-1011=w filler c-1012=w filler c-1013=w filler +c-1014=w filler c-1011=w filler-1 c-1012=w filler-1 c-1013=w filler-1 c-1011=w filler-2 c-1012=w filler-2 c-1013=w filler-2 -c-1014=w filler +c-1013=w inserted c-1015=w filler c-1014=w filler-1 c-1015=w filler-1 c-1014=w filler-2 c-1015=w filler-2 -c-1013=w inserted delete from t3 where b='del-me'; alter table t3 add primary key(b); select b,filler from t3 where (b>='c-1011=w' and b<= 'c-1018=w') or === modified file 'mysql-test/r/subselect3.result' --- a/mysql-test/r/subselect3.result 2008-05-22 18:40:15 +0000 +++ b/mysql-test/r/subselect3.result 2008-07-28 22:51:38 +0000 @@ -789,3 +789,23 @@ a b 1 0.123 drop table t1; End of 5.0 tests + +BUG#37842: Assertion in DsMrr_impl::dsmrr_init, at handler.cc:4307 + +CREATE TABLE t1 ( +`pk` int(11) NOT NULL AUTO_INCREMENT, +`int_key` int(11) DEFAULT NULL, +PRIMARY KEY (`pk`), +KEY `int_key` (`int_key`) +) ENGINE=MyISAM; +INSERT INTO t1 VALUES (1,9),(2,3),(3,8),(4,6),(5,9),(6,5),(7,5),(8,9),(9,1),(10,10); +SELECT `pk` FROM t1 AS OUTR WHERE `int_key` = ALL ( +SELECT `int_key` FROM t1 AS INNR WHERE INNR . `pk` >= 9 +); +pk +2 +4 +6 +8 +10 +DROP TABLE t1; === modified file 'mysql-test/t/subselect3.test' --- a/mysql-test/t/subselect3.test 2008-05-22 18:40:15 +0000 +++ b/mysql-test/t/subselect3.test 2008-07-28 22:51:38 +0000 @@ -631,3 +631,20 @@ select * from t1; drop table t1; --echo End of 5.0 tests + +--echo +--echo BUG#37842: Assertion in DsMrr_impl::dsmrr_init, at handler.cc:4307 +--echo +CREATE TABLE t1 ( + `pk` int(11) NOT NULL AUTO_INCREMENT, + `int_key` int(11) DEFAULT NULL, + PRIMARY KEY (`pk`), + KEY `int_key` (`int_key`) +) ENGINE=MyISAM; + +INSERT INTO t1 VALUES (1,9),(2,3),(3,8),(4,6),(5,9),(6,5),(7,5),(8,9),(9,1),(10,10); + +SELECT `pk` FROM t1 AS OUTR WHERE `int_key` = ALL ( + SELECT `int_key` FROM t1 AS INNR WHERE INNR . `pk` >= 9 +); +DROP TABLE t1; === modified file 'sql/handler.cc' --- a/sql/handler.cc 2008-07-19 14:21:51 +0000 +++ b/sql/handler.cc 2008-07-28 22:51:38 +0000 @@ -4290,17 +4290,20 @@ scan_it_again: @retval other Error */ -int DsMrr_impl::dsmrr_init(handler *h, KEY *key, - RANGE_SEQ_IF *seq_funcs, void *seq_init_param, - uint n_ranges, uint mode, HANDLER_BUFFER *buf) +int DsMrr_impl::dsmrr_init(handler *h_arg, RANGE_SEQ_IF *seq_funcs, + void *seq_init_param, uint n_ranges, uint mode, + HANDLER_BUFFER *buf) { uint elem_size; - uint keyno; Item *pushed_cond= NULL; handler *new_h2; DBUG_ENTER("DsMrr_impl::dsmrr_init"); - keyno= h->active_index; - DBUG_ASSERT(h2 == NULL); + + /* + index_merge may invoke a scan on an object for which dsmrr_info[_const] + has not been called, so set the owner handler here as well. + */ + h= h_arg; if (mode & HA_MRR_USE_DEFAULT_IMPL || mode & HA_MRR_SORTED) { use_default_impl= TRUE; @@ -4308,8 +4311,9 @@ int DsMrr_impl::dsmrr_init(handler *h, K n_ranges, mode, buf)); } rowids_buf= buf->buffer; + //KEY *key; //psergey-todo: don't add key_length as it is not needed anymore - rowids_buf += key->key_length + h->ref_length; + //rowids_buf += key->key_length + h->ref_length; is_mrr_assoc= !test(mode & HA_MRR_NO_ASSOCIATION); rowids_buf_end= buf->buffer_end; @@ -4320,36 +4324,68 @@ int DsMrr_impl::dsmrr_init(handler *h, K elem_size; rowids_buf_end= rowids_buf_last; - /* Create a separate handler object to do rndpos() calls. */ THD *thd= current_thd; - if (!(new_h2= h->clone(thd->mem_root)) || - new_h2->ha_external_lock(thd, F_RDLCK)) + + /* + There can be two cases: + - This is the first call since index_init(), h2==NULL + Need to setup h2 then. + - This is not the first call, h2 is initalized and set up appropriately. + The caller might have called h->index_init(), need to switch h to + rnd_pos calls. + */ + if (!h2) { - delete new_h2; - DBUG_RETURN(1); - } + DBUG_ASSERT(h->active_index != MAX_KEY); + uint mrr_keyno= h->active_index; + + /* Create a separate handler object to do rndpos() calls. */ + if (!(new_h2= h->clone(thd->mem_root)) || + new_h2->ha_external_lock(thd, F_RDLCK)) + { + delete new_h2; + DBUG_RETURN(1); + } + + if (mrr_keyno == h->pushed_idx_cond_keyno) + pushed_cond= h->pushed_idx_cond; + + /* + Caution: this call will invoke this->dsmrr_close(). Do not put the + created secondary table handler into this->h2 or it will delete it. + */ + if (h->ha_index_end()) + { + h2=new_h2; + goto error; + } - if (keyno == h->pushed_idx_cond_keyno) - pushed_cond= h->pushed_idx_cond; - if (h->ha_index_end()) + h2= new_h2; /* Ok, now can put it into h2 */ + table->prepare_for_position(); + new_h2->extra(HA_EXTRA_KEYREAD); + + if (h2->ha_index_init(mrr_keyno, FALSE) || + h2->handler::multi_range_read_init(seq_funcs, seq_init_param, n_ranges, + mode, buf)) + goto error; + use_default_impl= FALSE; + + if (pushed_cond) + h2->idx_cond_push(mrr_keyno, pushed_cond); + } + else { - new_h2= h2; - goto error; + /* index_end will invoke this->dsmrr_close(). Do not let it delete h2. */ + handler *save_h2= h2; + h2= NULL; + int res= (h->inited == handler::INDEX && h->ha_index_end()); + h2= save_h2; + use_default_impl= FALSE; + if (res) + goto error; } - h2= new_h2; - table->prepare_for_position(); - new_h2->extra(HA_EXTRA_KEYREAD); - - if (h2->ha_index_init(keyno, FALSE) || - h2->handler::multi_range_read_init(seq_funcs, seq_init_param, n_ranges, - mode, buf)) - goto error; - use_default_impl= FALSE; - - if (pushed_cond) - h2->idx_cond_push(keyno, pushed_cond); - if (dsmrr_fill_buffer(new_h2)) + if (dsmrr_fill_buffer()) goto error; /* @@ -4410,7 +4446,7 @@ static int rowid_cmp(void *h, uchar *a, @retval other Error */ -int DsMrr_impl::dsmrr_fill_buffer(handler *unused) +int DsMrr_impl::dsmrr_fill_buffer() { char *range_info; int res; @@ -4452,7 +4488,7 @@ int DsMrr_impl::dsmrr_fill_buffer(handle DS-MRR implementation: multi_range_read_next() function */ -int DsMrr_impl::dsmrr_next(handler *h, char **range_info) +int DsMrr_impl::dsmrr_next(char **range_info) { int res; @@ -4466,7 +4502,7 @@ int DsMrr_impl::dsmrr_next(handler *h, c res= HA_ERR_END_OF_FILE; goto end; } - res= dsmrr_fill_buffer(h); + res= dsmrr_fill_buffer(); if (res) goto end; } === modified file 'sql/handler.h' --- a/sql/handler.h 2008-07-10 16:02:38 +0000 +++ b/sql/handler.h 2008-07-28 22:51:38 +0000 @@ -2350,14 +2350,15 @@ public: DsMrr_impl() : h2(NULL) {}; - - handler *h; /* The "owner" handler object. It is used for scanning the index */ - TABLE *table; /* Always equal to h->table */ -private: + /* - Secondary handler object. It is used to retrieve full table rows by - calling rnd_pos(). + The "owner" handler object (the one that calls dsmrr_XXX functions. + It is used to retrieve full table rows by calling rnd_pos(). */ + handler *h; + TABLE *table; /* Always equal to h->table */ +private: + /* Secondary handler object. It is used for scanning the index */ handler *h2; /* Buffer to store rowids, or (rowid, range_id) pairs */ @@ -2378,12 +2379,11 @@ public: h= h_arg; table= table_arg; } - int dsmrr_init(handler *h, KEY *key, RANGE_SEQ_IF *seq_funcs, - void *seq_init_param, uint n_ranges, uint mode, - HANDLER_BUFFER *buf); + int dsmrr_init(handler *h, RANGE_SEQ_IF *seq_funcs, void *seq_init_param, + uint n_ranges, uint mode, HANDLER_BUFFER *buf); void dsmrr_close(); - int dsmrr_fill_buffer(handler *h); - int dsmrr_next(handler *h, char **range_info); + int dsmrr_fill_buffer(); + int dsmrr_next(char **range_info); int dsmrr_info(uint keyno, uint n_ranges, uint keys, uint *bufsz, uint *flags, COST_VECT *cost); === modified file 'storage/innobase/handler/ha_innodb.cc' --- a/storage/innobase/handler/ha_innodb.cc 2008-06-12 00:08:07 +0000 +++ b/storage/innobase/handler/ha_innodb.cc 2008-07-28 22:51:38 +0000 @@ -3292,7 +3292,6 @@ skip_field: prebuilt->idx_cond_func= NULL; prebuilt->n_index_fields= n_requested_fields; } - // file->in_range_read= FALSE; if (index != clust_index && prebuilt->need_to_access_clustered) { /* Change rec_field_no's to correspond to the clustered index @@ -6408,7 +6407,6 @@ ha_innobase::extra( /* Reset index condition pushdown state */ pushed_idx_cond= FALSE; pushed_idx_cond_keyno= MAX_KEY; - //in_range_read= FALSE; prebuilt->idx_cond_func= NULL; break; case HA_EXTRA_NO_KEYREAD: @@ -8318,13 +8316,12 @@ mysql_declare_plugin_end; int ha_innobase::multi_range_read_init(RANGE_SEQ_IF *seq, void *seq_init_param, uint n_ranges, uint mode, HANDLER_BUFFER *buf) { - return ds_mrr.dsmrr_init(this, &table->key_info[active_index], - seq, seq_init_param, n_ranges, mode, buf); + return ds_mrr.dsmrr_init(this, seq, seq_init_param, n_ranges, mode, buf); } int ha_innobase::multi_range_read_next(char **range_info) { - return ds_mrr.dsmrr_next(this, range_info); + return ds_mrr.dsmrr_next(range_info); } ha_rows ha_innobase::multi_range_read_info_const(uint keyno, RANGE_SEQ_IF *seq, @@ -8359,7 +8356,7 @@ C_MODE_START static my_bool index_cond_func_innodb(void *arg) { ha_innobase *h= (ha_innobase*)arg; - if (h->end_range) //was: h->in_range_read + if (h->end_range) { if (h->compare_key2(h->end_range) > 0) return 2; /* caller should return HA_ERR_END_OF_FILE already */ @@ -8389,11 +8386,7 @@ int ha_innobase::read_range_first(const bool sorted /* ignored */) { int res; - //if (!eq_range_arg) - //in_range_read= TRUE; res= handler::read_range_first(start_key, end_key, eq_range_arg, sorted); - //if (res) - // in_range_read= FALSE; return res; } @@ -8401,8 +8394,6 @@ int ha_innobase::read_range_first(const int ha_innobase::read_range_next() { int res= handler::read_range_next(); - //if (res) - // in_range_read= FALSE; return res; } === modified file 'storage/myisam/ha_myisam.cc' --- a/storage/myisam/ha_myisam.cc 2008-07-09 07:12:43 +0000 +++ b/storage/myisam/ha_myisam.cc 2008-07-28 22:51:38 +0000 @@ -1482,7 +1482,6 @@ C_MODE_START my_bool index_cond_func_myisam(void *arg) { ha_myisam *h= (ha_myisam*)arg; - /*if (h->in_range_read)*/ if (h->end_range) { if (h->compare_key2(h->end_range) > 0) @@ -1497,7 +1496,6 @@ C_MODE_END int ha_myisam::index_init(uint idx, bool sorted) { active_index=idx; - //in_range_read= FALSE; if (pushed_idx_cond_keyno == idx) mi_set_index_cond_func(file, index_cond_func_myisam, this); return 0; @@ -1605,13 +1603,9 @@ int ha_myisam::read_range_first(const ke bool sorted /* ignored */) { int res; - //if (!eq_range_arg) - // in_range_read= TRUE; res= handler::read_range_first(start_key, end_key, eq_range_arg, sorted); - //if (res) - // in_range_read= FALSE; return res; } @@ -1619,8 +1613,6 @@ int ha_myisam::read_range_first(const ke int ha_myisam::read_range_next() { int res= handler::read_range_next(); - //if (res) - // in_range_read= FALSE; return res; } @@ -2039,13 +2031,12 @@ int ha_myisam::multi_range_read_init(RAN uint n_ranges, uint mode, HANDLER_BUFFER *buf) { - return ds_mrr.dsmrr_init(this, &table->key_info[active_index], - seq, seq_init_param, n_ranges, mode, buf); + return ds_mrr.dsmrr_init(this, seq, seq_init_param, n_ranges, mode, buf); } int ha_myisam::multi_range_read_next(char **range_info) { - return ds_mrr.dsmrr_next(this, range_info); + return ds_mrr.dsmrr_next(range_info); } ha_rows ha_myisam::multi_range_read_info_const(uint keyno, RANGE_SEQ_IF *seq,