MDEV-10092: Server crashes in in ha_heap::rnd_pos / Table_read_cursor::get_next

The bug was caused by several issues.
2 problems in seek_io_cache. Due to wrong offsets used, we would end up
seeking way too much (first change), or over the intended seek point
(second change). Fixing it requires correctly detecting available data
in buffer (first change), and not using "IO_SIZE alligned" reads. The
second is needed because _my_b_cache_read adjusts the pos_in_file itself
based on read_pos and read_end. Pretending buffer is empty when we want
to force a read will aleviate this problem.

Secondly, the big-table cursors didn't repect the interface definitions
of always returning the rownumber that Table_read_cursor::fetch() would activate.

At the same time, next(), prev() and move_to() should not perform any
row activation.

mysys/mf_iocache.c

@@ -397,8 +397,9 @@ void end_slave_io_cache(IO_CACHE *cache)
 void seek_io_cache(IO_CACHE *cache, my_off_t needed_offset)
 {
   my_off_t cached_data_start= cache->pos_in_file;
-  my_off_t cached_data_end= cache->pos_in_file + (cache->read_pos -
+  my_off_t cached_data_end= cache->pos_in_file + (cache->read_end -
                                                   cache->buffer);
+
   if (needed_offset >= cached_data_start &&
       needed_offset < cached_data_end)
   {
@@ -416,11 +417,17 @@ void seek_io_cache(IO_CACHE *cache, my_off_t needed_offset)
       The offset we're seeking to is not in the buffer.
       - Set the buffer to be exhausted.
       - Make the next read to a mysql_file_seek() call to the required 
-        offset (but still use aligned reads).
+        offset.
+      TODO(cvicentiu, spetrunia) properly implement aligned seeks for
+      efficiency.
     */
-    cache->read_pos= cache->read_end;
     cache->seek_not_done= 1;
-    cache->pos_in_file= (needed_offset / IO_SIZE) * IO_SIZE;
+    cache->pos_in_file= needed_offset;
+    /* When reading it must appear as if we've started from  the offset
+       that we've seeked here. We must let _my_b_cache_read assume that
+       by implying "no reading starting from pos_in_file" has happened. */
+    cache->read_pos= cache->buffer;
+    cache->read_end= cache->buffer;
   }
 }
 

sql/sql_window.cc

@@ -632,9 +632,10 @@ class Rowid_seq_cursor
   /* If io_cache=!NULL, use it */
   IO_CACHE *io_cache;
   uchar *ref_buffer;   /* Buffer for the last returned rowid */
-  uint rownum;     /* Number of the rowid that is about to be returned */
-  bool cache_eof; /* whether we've reached EOF */
-  
+  ha_rows rownum;     /* Number of the rowid that is about to be returned */
+  ha_rows current_ref_buffer_rownum;
+  bool ref_buffer_valid;
+
   /* The following are used when we are reading from an array of pointers */
   uchar *cache_start;
   uchar *cache_pos;
@@ -655,34 +656,26 @@ class Rowid_seq_cursor
     {
       //DBUG_ASSERT(info->read_record == rr_from_tempfile);
       rownum= 0;
-      cache_eof= false;
       io_cache= (IO_CACHE*)my_malloc(sizeof(IO_CACHE), MYF(0));
       init_slave_io_cache(info->io_cache, io_cache);
 
       ref_buffer= (uchar*)my_malloc(ref_length, MYF(0));
+      ref_buffer_valid= false;
     }
   }
 
   virtual int next()
   {
+    /* Allow multiple next() calls in EOF state. */
+    if (at_eof())
+        return -1;
+
     if (io_cache)
     {
-      if (cache_eof)
-        return 1;
-
-      if (my_b_read(io_cache,ref_buffer,ref_length))
-      {
-        cache_eof= 1; // TODO: remove cache_eof
-        return -1;
-      }
       rownum++;
-      return 0;
     }
     else
     {
-      /* Allow multiple next() calls in EOF state. */
-      if (cache_pos == cache_end)
-        return -1;
       cache_pos+= ref_length;
       DBUG_ASSERT(cache_pos <= cache_end);
     }
@@ -696,7 +689,7 @@ class Rowid_seq_cursor
       if (rownum == 0)
         return -1;
 
-      move_to(rownum - 1);
+      rownum--;
       return 0;
     }
     else
@@ -722,9 +715,7 @@ class Rowid_seq_cursor
   {
     if (io_cache)
     {
-      seek_io_cache(io_cache, row_number * ref_length);
       rownum= row_number;
-      Rowid_seq_cursor::next();
     }
     else
     {
@@ -738,18 +729,36 @@ class Rowid_seq_cursor
   {
     if (io_cache)
     {
-      return cache_eof;
+      return rownum * ref_length >= io_cache->end_of_file;
     }
     else
       return (cache_pos == cache_end);
   }
 
-  uchar *get_curr_rowid()
+  bool get_curr_rowid(uchar **row_id)
   {
     if (io_cache)
-      return ref_buffer;
+    {
+      DBUG_ASSERT(!at_eof());
+      if (!ref_buffer_valid || current_ref_buffer_rownum != rownum)
+      {
+        seek_io_cache(io_cache, rownum * ref_length);
+        if (my_b_read(io_cache,ref_buffer,ref_length))
+        {
+          /* Error reading from file. */
+          return true;
+        }
+        ref_buffer_valid= true;
+        current_ref_buffer_rownum = rownum;
+      }
+      *row_id = ref_buffer;
+      return false;
+    }
     else
-      return cache_pos;
+    {
+      *row_id= cache_pos;
+      return false;
+    }
   }
 };
 
@@ -775,7 +784,9 @@ class Table_read_cursor : public Rowid_seq_cursor
     if (at_eof())
       return -1;
 
-    uchar* curr_rowid= get_curr_rowid();
+    uchar* curr_rowid;
+    if (get_curr_rowid(&curr_rowid))
+      return -1;
     return table->file->ha_rnd_pos(record, curr_rowid);
   }