Issue#250: MyRocks/Innodb different output from query with order by on table with index and decimal type

Summary:
Make open_binary_frm() set TABLE_SHARE::primary_key before it computes
"handler->index_flags(keyno, ...) & HA_KEYREAD_ONLY".
In MyRocks, the value of this expression depends on whether keyno is a
clustered PK or not, so it's essential that MyRocks sees the correct
value in TABLE_SHARE::primary_key.

Test Plan: Run MTR

Reviewers: yoshinorim, alexyang, jtolmer, jkedgar, hermanlee4

Reviewed By: hermanlee4

Subscribers: webscalesql-eng

Differential Revision: https://reviews.facebook.net/D58329

Author: spetrunia

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

@@ -2379,4 +2379,23 @@ Warning	1264	Out of range value for column 'a' at row 2
 Warning	1264	Out of range value for column 'b' at row 2
 UNLOCK TABLES;
 DROP TABLE t1;
+#
+# Issue#250: MyRocks/Innodb different output from query with order by on table with index and decimal type
+#
+CREATE TABLE t1(
+c1 DECIMAL(65,0) UNSIGNED NOT NULL, 
+c2 DECIMAL(65,0) SIGNED NULL, 
+c3 INT, 
+INDEX idx(c1,c2)
+);
+INSERT INTO t1 VALUES (0,'1999',5);
+SELECT * FROM t1 force index(idx) WHERE c1 <> '1' ORDER BY c1 DESC;
+c1	c2	c3
+0	1999	5
+# This must not use ICP. when/if MyRocks supports index-only for
+#   DECIMAL, switch this testcase to use another datatype
+explain SELECT * FROM t1  force index(idx) WHERE c1 <> '1' ORDER BY c1 DESC;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t1	range	idx	idx	29	NULL	#	Using where
+drop table t1;
 SET GLOBAL ROCKSDB_PAUSE_BACKGROUND_WORK = @ORIG_PAUSE_BACKGROUND_WORK;

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

@@ -1865,5 +1865,23 @@ INSERT INTO t1 VALUES(X'042000200020',X'042000200020'),(X'200400200020',X'200400
 UNLOCK TABLES;
 DROP TABLE t1;
 
+--echo #
+--echo # Issue#250: MyRocks/Innodb different output from query with order by on table with index and decimal type
+--echo #
+
+CREATE TABLE t1(
+  c1 DECIMAL(65,0) UNSIGNED NOT NULL, 
+  c2 DECIMAL(65,0) SIGNED NULL, 
+  c3 INT, 
+  INDEX idx(c1,c2)
+);
+INSERT INTO t1 VALUES (0,'1999',5);
+SELECT * FROM t1 force index(idx) WHERE c1 <> '1' ORDER BY c1 DESC;
+--echo # This must not use ICP. when/if MyRocks supports index-only for
+--echo #   DECIMAL, switch this testcase to use another datatype
+--replace_column 9 #
+explain SELECT * FROM t1  force index(idx) WHERE c1 <> '1' ORDER BY c1 DESC;
+
+drop table t1;
 
 SET GLOBAL ROCKSDB_PAUSE_BACKGROUND_WORK = @ORIG_PAUSE_BACKGROUND_WORK;

sql/table.cc

@@ -2016,6 +2016,17 @@ static int open_binary_frm(THD *thd, TABLE_SHARE *share, uchar *head,
   {
     uint primary_key=(uint) (find_type(primary_key_name, &share->keynames,
                                        FIND_TYPE_NO_PREFIX) - 1);
+    /*
+      The following if-else is here for MyRocks:
+      set share->primary_key as early as possible, because the return value
+      of ha_rocksdb::index_flags(key, ...) (HA_KEYREAD_ONLY bit in particular)
+      depends on whether the key is the primary key.
+    */
+    if (primary_key < MAX_KEY && share->keys_in_use.is_set(primary_key))
+      share->primary_key= primary_key;
+    else
+      share->primary_key= MAX_KEY;
+
     longlong ha_option= handler_file->ha_table_flags();
     keyinfo= share->key_info;
     key_part= keyinfo->key_part;
@@ -2070,6 +2081,13 @@ static int open_binary_frm(THD *thd, TABLE_SHARE *share, uchar *head,
 	    break;
 	  }
 	}
+
+        /*
+          The following is here for MyRocks. See the comment above
+          about "set share->primary_key as early as possible"
+        */
+        if (primary_key < MAX_KEY && share->keys_in_use.is_set(primary_key))
+          share->primary_key= primary_key;
       }
 
       for (i=0 ; i < keyinfo->user_defined_key_parts ; key_part++,i++)
@@ -2230,10 +2248,8 @@ static int open_binary_frm(THD *thd, TABLE_SHARE *share, uchar *head,
       }
     }
 
-    if (primary_key < MAX_KEY &&
-	(share->keys_in_use.is_set(primary_key)))
+    if (share->primary_key != MAX_KEY)
     {
-      share->primary_key= primary_key;
       /*
 	If we are using an integer as the primary key then allow the user to
 	refer to it as '_rowid'
@@ -2249,8 +2265,6 @@ static int open_binary_frm(THD *thd, TABLE_SHARE *share, uchar *head,
         }
       }
     }
-    else
-      share->primary_key = MAX_KEY; // we do not have a primary key
   }
   else
     share->primary_key= MAX_KEY;

storage/rocksdb/ha_rocksdb.cc

@@ -3204,8 +3204,14 @@ const char **ha_rocksdb::bas_ext() const
 bool ha_rocksdb::init_with_fields()
 {
   uint pk= table_share->primary_key;
-  uint key_parts= table_share->key_info[pk].user_defined_key_parts;
-  check_keyread_allowed(pk /*PK*/, key_parts-1, true);
+  if (pk != MAX_KEY)
+  {
+    uint key_parts= table_share->key_info[pk].user_defined_key_parts;
+    check_keyread_allowed(pk /*PK*/, key_parts-1, true);
+  }
+  else
+    m_pk_can_be_decoded= false;
+
   cached_table_flags= table_flags();
   return false; /* Ok */
 }