maria-developers team mailing list archive

[Jeremy Cole <jeremycole@xxxxxxxxxx>]

Sergey,

Nice test case.

Would you mind cleaning up the end-space in your patch? 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?

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
>