From f3d36951a62aa1a00a5a10083fb77e9919022e03 Mon Sep 17 00:00:00 2001
From: Manuel Ung <mung@fb.com>
Date: Thu, 11 Jan 2018 15:21:38 -0800
Subject: [PATCH] Skip trailing space bytes for non-unpackable fields

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 #784
Closes https://github.com/facebook/mysql-5.6/pull/785

Differential Revision: D6699483

fbshipit-source-id: fd5c9d6
---
 mysql-test/suite/rocksdb/r/type_varchar.result | 14 +++++++++++++-
 mysql-test/suite/rocksdb/t/type_varchar.test   | 15 +++++++++++----
 storage/rocksdb/rdb_datadic.cc                 | 12 ++++++++++++
 3 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/mysql-test/suite/rocksdb/r/type_varchar.result b/mysql-test/suite/rocksdb/r/type_varchar.result
index 9784b1a1d94d..dd01f7f80771 100644
--- a/mysql-test/suite/rocksdb/r/type_varchar.result
+++ b/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;
diff --git a/mysql-test/suite/rocksdb/t/type_varchar.test b/mysql-test/suite/rocksdb/t/type_varchar.test
index e45b6836f673..b631615c2666 100644
--- a/mysql-test/suite/rocksdb/t/type_varchar.test
+++ b/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;
+
diff --git a/storage/rocksdb/rdb_datadic.cc b/storage/rocksdb/rdb_datadic.cc
index 970290cdd949..068eace701b6 100644
--- a/storage/rocksdb/rdb_datadic.cc
+++ b/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);
+      }
     }
   }