Skip trailing space bytes for non-unpackable fields (facebook#785) (f…

…acebook#785)

Summary:
We always generate unpack info bytes that indicate how much trailing space we need, even when the field can never be unpacked, because it is not implemented. When skipping these fields however, we only skip the key portion, and not the value portion which corrupts data for code that reads unpack_info downstream. Fix by skipping these bytes.

Closes facebook#784
Closes facebook#785

Differential Revision: D6699483 (facebook@f3d3695)

fbshipit-source-id: 7707a3173f7

mysql-test/suite/rocksdb/r/type_varchar.result

@@ -1,4 +1,3 @@
-drop table if exists t1,t2;
 #
 # A basic test whether endspace-aware variable length encoding 
 # works when in PK
@@ -756,3 +755,16 @@ email_i	1
 drop table t;
 set global rocksdb_checksums_pct = @save_rocksdb_checksums_pct;
 set session rocksdb_verify_row_debug_checksums = @save_rocksdb_verify_row_debug_checksums;
+drop table if exists t;
+Warnings:
+Note	1051	Unknown table 'test.t'
+create table t (h varchar(31) character set utf8 collate utf8_bin not null, i varchar(19) collate latin1_bin not null, primary key(i), key(h)) engine=rocksdb;
+insert into t(i,h) values('a','b');
+check table t;
+Table	Op	Msg_type	Msg_text
+test.t	check	status	OK
+alter table t modify h varchar(31) character set cp1257 collate cp1257_bin not null;
+check table t;
+Table	Op	Msg_type	Msg_text
+test.t	check	status	OK
+drop table t;

mysql-test/suite/rocksdb/t/type_varchar.test

@@ -1,9 +1,5 @@
 --source include/have_rocksdb.inc
 
---disable_warnings
-drop table if exists t1,t2;
---enable_warnings
-
 #
 # VARCHAR column types
 #
@@ -73,3 +69,14 @@ select 'email_i' as index_name, count(*) AS count from t force index(email_i);
 drop table t;
 set global rocksdb_checksums_pct = @save_rocksdb_checksums_pct;
 set session rocksdb_verify_row_debug_checksums = @save_rocksdb_verify_row_debug_checksums;
+
+# Issue #784 - Skip trailing space bytes for non-unpackable fields
+
+drop table if exists t;
+create table t (h varchar(31) character set utf8 collate utf8_bin not null, i varchar(19) collate latin1_bin not null, primary key(i), key(h)) engine=rocksdb;
+insert into t(i,h) values('a','b');
+check table t;
+alter table t modify h varchar(31) character set cp1257 collate cp1257_bin not null;
+check table t;
+drop table t;
+

storage/rocksdb/rdb_datadic.cc

@@ -1501,6 +1501,18 @@ int Rdb_key_def::unpack_record(TABLE *const table, uchar *const buf,
       }
       if ((this->*fpi->m_skip_func)(fpi, field, &reader))
         return HA_ERR_ROCKSDB_CORRUPT_DATA;
+
+      // If this is a space padded varchar, we need to skip the indicator
+      // bytes for trailing bytes. They're useless since we can't restore the
+      // field anyway.
+      //
+      // There is a special case for prefixed varchars where we do not
+      // generate unpack info, because we know prefixed varchars cannot be
+      // unpacked. In this case, it is not necessary to skip.
+      if (fpi->m_skip_func == &Rdb_key_def::skip_variable_space_pad &&
+          !fpi->m_unpack_info_stores_value) {
+        unp_reader.read(fpi->m_unpack_info_uses_two_bytes ? 2 : 1);
+      }
     }
   }