Issue MariaDB#15: SIGSEGV from reading in blob data

- convert_record_from_storage_format(buf) worked incorrectly
  when buf != table->record[0]. Length bytes were written
  to table->record[0] still.  Moved field->move_field_offset()
  call earlier to prevent that.
- Also, added comments and free() calls to free the retrieved
  blobs (the API is undocumented so we copy what InnoDB does).

mysql-test/r/rocksdb.result

@@ -1399,3 +1399,14 @@ select * from t1 where pk=4;
 pk	col1	col2
 4	4	4
 drop table t1;
+#
+# Issue #15: SIGSEGV from reading in blob data
+#
+CREATE TABLE t1 (
+id int not null,
+blob_col text,
+PRIMARY KEY (id)
+) ENGINE=ROCKSDB CHARSET=latin1;
+INSERT INTO t1 SET id=123, blob_col=repeat('z',64000) ON DUPLICATE KEY UPDATE blob_col=VALUES(blob_col);
+INSERT INTO t1 SET id=123, blob_col=''                ON DUPLICATE KEY UPDATE blob_col=VALUES(blob_col);
+DROP TABLE t1;

mysql-test/t/rocksdb.test

@@ -1235,5 +1235,19 @@ select * from t1 where col1=2;
 
 select * from t1 where pk=4;
 
-
 drop table t1;
+
+--echo #
+--echo # Issue #15: SIGSEGV from reading in blob data
+--echo #
+CREATE TABLE t1 (
+  id int not null,
+  blob_col text,
+  PRIMARY KEY (id)
+) ENGINE=ROCKSDB CHARSET=latin1;
+
+INSERT INTO t1 SET id=123, blob_col=repeat('z',64000) ON DUPLICATE KEY UPDATE blob_col=VALUES(blob_col); 
+INSERT INTO t1 SET id=123, blob_col=''                ON DUPLICATE KEY UPDATE blob_col=VALUES(blob_col);
+DROP TABLE t1;
+
+

storage/rocksdb/ha_rocksdb.cc

@@ -1073,16 +1073,20 @@ int ha_rocksdb::convert_record_from_storage_format(uchar * buf)
       const char *data_len_str;
       if (!(data_len_str= reader.read(length_bytes)))
         return HA_ERR_INTERNAL_ERROR;
+
+      blob->move_field_offset(ptr_diff);
       memcpy(blob->ptr, data_len_str, length_bytes);
 
       uint32 data_len= blob->get_length((uchar*)data_len_str, length_bytes, 
                                         table->s->db_low_byte_first);
-      const char *blob_ptr;      
+      const char *blob_ptr;
       if (!(blob_ptr= reader.read(data_len)))
+      {
+        blob->move_field_offset(-ptr_diff);
         return HA_ERR_INTERNAL_ERROR;
-
-      blob->move_field_offset(ptr_diff);
-      // set 8-byte pointer to 0, like innodb does.
+      }
+
+      // set 8-byte pointer to 0, like innodb does (relevant for 32-bit platforms)
       memset(blob->ptr + length_bytes, 0, 8); 
       memcpy(blob->ptr + length_bytes, &blob_ptr, sizeof(uchar**));
       blob->move_field_offset(-ptr_diff);
@@ -2836,6 +2840,13 @@ int ha_rocksdb::extra(enum ha_extra_function operation)
   case HA_EXTRA_NO_KEYREAD:
     keyread_only= FALSE;
     break;
+  case HA_EXTRA_FLUSH:
+    /*
+      If the table has blobs, then they are part of retrieved_record. This call
+      invalidates them.
+    */
+    retrieved_record.clear();
+    break;
   default:
     break;
   }

storage/rocksdb/ha_rocksdb.h

@@ -122,7 +122,13 @@ class ha_rocksdb: public handler
   /* Buffer used by convert_record_to_storage_format() */
   String storage_record;
 
-  /* Last retrieved record, in table->record[0] data format */
+  /*
+    Last retrieved record, in table->record[0] data format. It also serves as
+    as storage for blob data (Field_blob object have pointers to here)
+
+    TODO: Dont we lose one malloc() per record read by having it as std::string
+    instead of rocksdb::Slice?
+  */
   std::string retrieved_record;
 
   /* If TRUE, reads should place locks on rows */
@@ -312,6 +318,13 @@ class ha_rocksdb: public handler
   int delete_all_rows(void);
   int truncate();
 
+  int reset()
+  {
+    /* Free blob data */
+    retrieved_record.clear();
+    return 0;
+  }
+
   void remove_rows(RDBSE_TABLE_DEF *tbl);
   ha_rows records_in_range(uint inx, key_range *min_key,
                            key_range *max_key);