MDEV-6838: Using too big key for internal temp tables

This bug manifests due to wrong computation and evaluation of
keyinfo->key_length. The issues were:
* Using table->file->max_key_length() as an absolute value that must not be
  reached for a key, while it represents the maximum number of bytes
  possible for a table key.
* Incorrectly computing the keyinfo->key_length size during
  KEY_PART_INFO creation. The metadata information regarding the key
  such the field length (for strings) was added twice.

mysql-test/r/table_keyinfo-6838.result

+CREATE TABLE t1 (i INT, state VARCHAR(997)) ENGINE=MyISAM;
+INSERT INTO t1 VALUES (2,'Louisiana'),(9,'Maine');
+CREATE TABLE t2 (state VARCHAR(997), j INT) ENGINE=MyISAM;
+INSERT INTO t2 VALUES ('Louisiana',9),('Alaska',5);
+INSERT INTO t2 SELECT t2.* FROM t2 JOIN t2 AS t3 JOIN t2 AS t4 JOIN t2 AS t5 JOIN t2 AS t6;
+SET @@max_heap_table_size= 16384;
+set @@optimizer_switch='derived_merge=OFF';
+set @@optimizer_switch='extended_keys=ON';
+SELECT * FROM t1 AS t1_1 LEFT JOIN ( t1 AS t1_2 INNER JOIN (SELECT * FROM t2) v2 ON t1_2.i = j ) ON t1_1.state = v2.state LIMIT 1;
+i	state	i	state	state	j
+2	Louisiana	9	Maine	Louisiana	9
+DROP TABLE t1, t2;

mysql-test/t/table_keyinfo-6838.test

+# Test case for MDEV-6838.
+# Due to incorrect key_length computation, this usecase failed with the error
+# Using too big key for internal temp table.
+
+CREATE TABLE t1 (i INT, state VARCHAR(997)) ENGINE=MyISAM;
+INSERT INTO t1 VALUES (2,'Louisiana'),(9,'Maine');
+
+CREATE TABLE t2 (state VARCHAR(997), j INT) ENGINE=MyISAM;
+INSERT INTO t2 VALUES ('Louisiana',9),('Alaska',5);
+INSERT INTO t2 SELECT t2.* FROM t2 JOIN t2 AS t3 JOIN t2 AS t4 JOIN t2 AS t5 JOIN t2 AS t6;
+
+SET @@max_heap_table_size= 16384;
+set @@optimizer_switch='derived_merge=OFF';
+set @@optimizer_switch='extended_keys=ON';
+
+SELECT * FROM t1 AS t1_1 LEFT JOIN ( t1 AS t1_2 INNER JOIN (SELECT * FROM t2) v2 ON t1_2.i = j ) ON t1_1.state = v2.state LIMIT 1;
+
+DROP TABLE t1, t2;

sql/sql_select.cc

@@ -7957,7 +7957,8 @@ static bool create_hj_key_for_table(JOIN *join, JOIN_TAB *join_tab,
       {
         Field *field= table->field[keyuse->keypart];
         uint fieldnr= keyuse->keypart+1;
-        table->create_key_part_by_field(keyinfo, key_part_info, field, fieldnr);
+        table->create_key_part_by_field(key_part_info, field, fieldnr);
+        keyinfo->key_length += key_part_info->store_length;
         key_part_info++;
       }
     }
@@ -15921,7 +15922,7 @@ bool create_internal_tmp_table(TABLE *table, KEY *keyinfo,
       goto err;
 
     bzero(seg, sizeof(*seg) * keyinfo->key_parts);
-    if (keyinfo->key_length >= table->file->max_key_length() ||
+    if (keyinfo->key_length > table->file->max_key_length() ||
 	keyinfo->key_parts > table->file->max_key_parts() ||
 	share->uniques)
     {
@@ -16107,7 +16108,7 @@ bool create_internal_tmp_table(TABLE *table, KEY *keyinfo,
       goto err;
 
     bzero(seg, sizeof(*seg) * keyinfo->key_parts);
-    if (keyinfo->key_length >= table->file->max_key_length() ||
+    if (keyinfo->key_length > table->file->max_key_length() ||
 	keyinfo->key_parts > table->file->max_key_parts() ||
 	share->uniques)
     {

sql/structs.h

@@ -76,6 +76,7 @@ typedef struct st_key_part_info {	/* Info about a key part */
   */
   uint16 store_length;
   uint16 key_type;
+  /* Fieldnr begins counting from 1 */
   uint16 fieldnr;			/* Fieldnum in UNIREG */
   uint16 key_part_flag;			/* 0 or HA_REVERSE_SORT */
   uint8 type;

sql/table.cc

@@ -5968,8 +5968,19 @@ bool TABLE::alloc_keys(uint key_count)
 }
 
 
-void TABLE::create_key_part_by_field(KEY *keyinfo,
-                                     KEY_PART_INFO *key_part_info,
+/**
+  @brief
+  Populate a KEY_PART_INFO structure with the data related to a field entry.
+
+  @param key_part_info  The structure to fill.
+  @param field          The field entry that represents the key part.
+  @param fleldnr        The number of the field, count starting from 1.
+
+  TODO: This method does not make use of any table specific fields. It
+  could be refactored to act as a constructor for KEY_PART_INFO instead.
+*/
+
+void TABLE::create_key_part_by_field(KEY_PART_INFO *key_part_info,
                                      Field *field, uint fieldnr)
 {
   key_part_info->null_bit= field->null_bit;
@@ -5978,8 +5989,11 @@ void TABLE::create_key_part_by_field(KEY *keyinfo,
   key_part_info->field= field;
   key_part_info->fieldnr= fieldnr;
   key_part_info->offset= field->offset(record[0]);
-  key_part_info->length=   (uint16) field->pack_length();
-  keyinfo->key_length+= key_part_info->length;
+  /*
+     field->key_length() accounts for the raw length of the field, excluding
+     any metadata such as length of field or the NULL flag.
+  */
+  key_part_info->length= (uint16) field->key_length();
   key_part_info->key_part_flag= 0;
   /* TODO:
     The below method of computing the key format length of the
@@ -5991,17 +6005,20 @@ void TABLE::create_key_part_by_field(KEY *keyinfo,
   */
   key_part_info->store_length= key_part_info->length;
 
+  /*
+     The total store length of the key part is the raw length of the field +
+     any metadata information, such as its length for strings and/or the null
+     flag.
+  */
   if (field->real_maybe_null())
   {
     key_part_info->store_length+= HA_KEY_NULL_LENGTH;
-    keyinfo->key_length+= HA_KEY_NULL_LENGTH;
   }
   if (field->type() == MYSQL_TYPE_BLOB || 
       field->type() == MYSQL_TYPE_GEOMETRY ||
       field->real_type() == MYSQL_TYPE_VARCHAR)
   {
     key_part_info->store_length+= HA_KEY_BLOB_LENGTH;
-    keyinfo->key_length+= HA_KEY_BLOB_LENGTH; // ???
     key_part_info->key_part_flag|=
       field->type() == MYSQL_TYPE_BLOB ? HA_BLOB_PART: HA_VAR_LENGTH_PART;
   }
@@ -6124,7 +6141,8 @@ bool TABLE::add_tmp_key(uint key, uint key_parts,
     if (key_start)
       (*reg_field)->key_start.set_bit(key);
     (*reg_field)->part_of_key.set_bit(key);
-    create_key_part_by_field(keyinfo, key_part_info, *reg_field, fld_idx+1);
+    create_key_part_by_field(key_part_info, *reg_field, fld_idx+1);
+    keyinfo->key_length += key_part_info->store_length;
     (*reg_field)->flags|= PART_KEY_FLAG;
     key_start= FALSE;
     key_part_info++;

sql/table.h

@@ -1269,7 +1269,7 @@ public:
   bool add_tmp_key(uint key, uint key_parts,
                    uint (*next_field_no) (uchar *), uchar *arg,
                    bool unique);
-  void create_key_part_by_field(KEY *keyinfo, KEY_PART_INFO *key_part_info,
+  void create_key_part_by_field(KEY_PART_INFO *key_part_info,
                                 Field *field, uint fieldnr);
   void use_index(int key_to_save);
   void set_table_map(table_map map_arg, uint tablenr_arg)