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@askmonty.org> wrote:
At file:///home/psergey/dev2/10.0/

------------------------------------------------------------
revno: 4445
revision-id: psergey@askmonty.org-20141016135713-md92qr4g9fzhitxh
parent: bar@mnogosearch.org-20141013083155-j3j31hqhfc8i95y1
committer: Sergey Petrunya <psergey@askmonty.org>
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@mariadb.org
https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits