maria-developers team mailing list archive
-
maria-developers team
-
Mailing list archive
-
Message #07814
Re: [Commits] Rev 4445: MDEV-6878: Use of uninitialized saved_primary_key in Mrr_ordered_index_reader::resume_read() in file:///home/psergey/dev2/10.0/
Hi Jeremy,
On Thu, Oct 16, 2014 at 01:09:31PM -0700, Jeremy Cole wrote:
> Nice test case.
>
> Would you mind cleaning up the end-space in your patch?
Yes, I can
> Also, is there a
> reason to put the entire body of Mrr_ordered_index_reader::resume_read in
> the if() instead of an early return?
>
Since the function has no DBUG_ENTER/DBUG_RETURN, no specific reason.
I've committed a new variant:
http://lists.askmonty.org/pipermail/commits/2014-October/006815.html
is this one better? (If yes, I'll merge it, overwriting the previous variant)
> Regards,
>
> Jeremy
>
> On Thu, Oct 16, 2014 at 11:58 AM, Sergey Petrunya <psergey@xxxxxxxxxxxx>
> wrote:
>
> > At file:///home/psergey/dev2/10.0/
> >
> > ------------------------------------------------------------
> > revno: 4445
> > revision-id: psergey@xxxxxxxxxxxx-20141016135713-md92qr4g9fzhitxh
> > parent: bar@xxxxxxxxxxxxxxx-20141013083155-j3j31hqhfc8i95y1
> > committer: Sergey Petrunya <psergey@xxxxxxxxxxxx>
> > branch nick: 10.0
> > timestamp: Thu 2014-10-16 17:57:13 +0400
> > message:
> > MDEV-6878: Use of uninitialized saved_primary_key in
> > Mrr_ordered_index_reader::resume_read()
> >
> > - Make Mrr_ordered_index_reader::resume_read() restore index position
> > only if it was saved before with
> > Mrr_ordered_index_reader::interrupt_read().
> > === modified file 'mysql-test/r/innodb_mrr_cpk.result'
> > --- a/mysql-test/r/innodb_mrr_cpk.result 2013-09-26 18:20:15 +0000
> > +++ b/mysql-test/r/innodb_mrr_cpk.result 2014-10-16 13:57:13 +0000
> > @@ -194,3 +194,41 @@ id select_type table type possible_keys
> > 2 DERIVED t2 ALL NULL NULL NULL NULL #
> > set join_cache_level= @tmp_mdev5037;
> > drop table t0,t1,t2;
> > +#
> > +# MDEV-6878: Use of uninitialized saved_primary_key in
> > Mrr_ordered_index_reader::resume_read()
> > +#
> > +create table t0(a int);
> > +insert into t0 values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9);
> > +create table t1 (
> > +pk varchar(32) character set utf8 primary key,
> > +kp1 char(32) not null,
> > +col1 varchar(32),
> > +key (kp1)
> > +) engine=innodb;
> > +insert into t1
> > +select
> > +concat('pk-', 1000 +A.a),
> > +concat('kp1-', 1000 +A.a),
> > +concat('val-', 1000 +A.a)
> > +from test.t0 A ;
> > +create table t2 as select kp1 as a from t1;
> > +set join_cache_level=8;
> > +set optimizer_switch='mrr=on,mrr_sort_keys=on';
> > +explain
> > +select * from t2 straight_join t1 force index(kp1) where t1.kp1=t2.a;
> > +id select_type table type possible_keys key key_len
> > ref rows Extra
> > +1 SIMPLE t2 ALL NULL NULL NULL NULL 10
> > +1 SIMPLE t1 ref kp1 kp1 32 test.t2.a 1
> > Using join buffer (flat, BKAH join); Key-ordered Rowid-ordered scan
> > +select * from t2 straight_join t1 force index(kp1) where t1.kp1=t2.a;
> > +a pk kp1 col1
> > +kp1-1000 pk-1000 kp1-1000 val-1000
> > +kp1-1001 pk-1001 kp1-1001 val-1001
> > +kp1-1002 pk-1002 kp1-1002 val-1002
> > +kp1-1003 pk-1003 kp1-1003 val-1003
> > +kp1-1004 pk-1004 kp1-1004 val-1004
> > +kp1-1005 pk-1005 kp1-1005 val-1005
> > +kp1-1006 pk-1006 kp1-1006 val-1006
> > +kp1-1007 pk-1007 kp1-1007 val-1007
> > +kp1-1008 pk-1008 kp1-1008 val-1008
> > +kp1-1009 pk-1009 kp1-1009 val-1009
> > +drop table t0,t1,t2;
> >
> > === modified file 'mysql-test/t/innodb_mrr_cpk.test'
> > --- a/mysql-test/t/innodb_mrr_cpk.test 2013-09-26 18:20:15 +0000
> > +++ b/mysql-test/t/innodb_mrr_cpk.test 2014-10-16 13:57:13 +0000
> > @@ -192,3 +192,35 @@ explain SELECT 1 FROM (SELECT url, id FR
> > set join_cache_level= @tmp_mdev5037;
> >
> > drop table t0,t1,t2;
> > +
> > +--echo #
> > +--echo # MDEV-6878: Use of uninitialized saved_primary_key in
> > Mrr_ordered_index_reader::resume_read()
> > +--echo #
> > +create table t0(a int);
> > +insert into t0 values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9);
> > +
> > +create table t1 (
> > + pk varchar(32) character set utf8 primary key,
> > + kp1 char(32) not null,
> > + col1 varchar(32),
> > + key (kp1)
> > +) engine=innodb;
> > +
> > +insert into t1
> > +select
> > + concat('pk-', 1000 +A.a),
> > + concat('kp1-', 1000 +A.a),
> > + concat('val-', 1000 +A.a)
> > +from test.t0 A ;
> > +
> > +create table t2 as select kp1 as a from t1;
> > +
> > +set join_cache_level=8;
> > +set optimizer_switch='mrr=on,mrr_sort_keys=on';
> > +explain
> > +select * from t2 straight_join t1 force index(kp1) where t1.kp1=t2.a;
> > +select * from t2 straight_join t1 force index(kp1) where t1.kp1=t2.a;
> > +
> > +drop table t0,t1,t2;
> > +
> > +
> >
> > === modified file 'sql/multi_range_read.cc'
> > --- a/sql/multi_range_read.cc 2014-06-06 17:28:42 +0000
> > +++ b/sql/multi_range_read.cc 2014-10-16 13:57:13 +0000
> > @@ -426,6 +426,7 @@ bool Mrr_ordered_index_reader::set_inter
> > *space_start += key_len;
> >
> > have_saved_rowid= FALSE;
> > + read_was_interrupted= FALSE;
> > return FALSE;
> > }
> >
> > @@ -434,6 +435,7 @@ void Mrr_ordered_index_reader::set_no_in
> > support_scan_interruptions= FALSE;
> > saved_key_tuple= saved_rowid= saved_primary_key= NULL; /* safety */
> > have_saved_rowid= FALSE;
> > + read_was_interrupted= FALSE;
> > }
> >
> > void Mrr_ordered_index_reader::interrupt_read()
> > @@ -451,6 +453,7 @@ void Mrr_ordered_index_reader::interrupt
> > &table->key_info[table->s->primary_key],
> > table->key_info[table->s->primary_key].key_length);
> > }
> > + read_was_interrupted= TRUE;
> >
> > /* Save the last rowid */
> > memcpy(saved_rowid, file->ref, file->ref_length);
> > @@ -468,14 +471,17 @@ void Mrr_ordered_index_reader::position(
> > void Mrr_ordered_index_reader::resume_read()
> > {
> > TABLE *table= file->get_table();
> > - KEY *used_index= &table->key_info[file->active_index];
> > - key_restore(table->record[0], saved_key_tuple,
> > - used_index, used_index->key_length);
> > - if (saved_primary_key)
> > + if (read_was_interrupted)
> > {
> > - key_restore(table->record[0], saved_primary_key,
> > - &table->key_info[table->s->primary_key],
> > - table->key_info[table->s->primary_key].key_length);
> > + KEY *used_index= &table->key_info[file->active_index];
> > + key_restore(table->record[0], saved_key_tuple,
> > + used_index, used_index->key_length);
> > + if (saved_primary_key)
> > + {
> > + key_restore(table->record[0], saved_primary_key,
> > + &table->key_info[table->s->primary_key],
> > + table->key_info[table->s->primary_key].key_length);
> > + }
> > }
> > }
> >
> > @@ -557,8 +563,7 @@ int Mrr_ordered_index_reader::init(handl
> > is_mrr_assoc= !MY_TEST(mode & HA_MRR_NO_ASSOCIATION);
> > mrr_funcs= *seq_funcs;
> > source_exhausted= FALSE;
> > - if (support_scan_interruptions)
> > - bzero(saved_key_tuple, key_info->key_length);
> > + read_was_interrupted= false;
> > have_saved_rowid= FALSE;
> > return 0;
> > }
> >
> > === modified file 'sql/multi_range_read.h'
> > --- a/sql/multi_range_read.h 2013-09-26 18:20:15 +0000
> > +++ b/sql/multi_range_read.h 2014-10-16 13:57:13 +0000
> > @@ -339,6 +339,12 @@ class Mrr_ordered_index_reader : public
> >
> > uchar *saved_key_tuple; /* Saved current key tuple */
> > uchar *saved_primary_key; /* Saved current primary key tuple */
> > +
> > + /*
> > + TRUE<=> saved_key_tuple (and saved_primary_key when applicable) have
> > + valid values.
> > + */
> > + bool read_was_interrupted;
> >
> > static int compare_keys(void* arg, uchar* key1, uchar* key2);
> > static int compare_keys_reverse(void* arg, uchar* key1, uchar* key2);
> >
> > _______________________________________________
> > commits mailing list
> > commits@xxxxxxxxxxx
> > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits
> >
> _______________________________________________
> Mailing list: https://launchpad.net/~maria-developers
> Post to : maria-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~maria-developers
> More help : https://help.launchpad.net/ListHelp
--
BR
Sergei
--
Sergei Petrunia, Software Developer
MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog
References
