diff --git a/mysql-test/r/table_keyinfo-6838.result b/mysql-test/r/table_keyinfo-6838.result new file mode 100644 index 00000000000..55b035069ce --- /dev/null +++ b/mysql-test/r/table_keyinfo-6838.result @@ -0,0 +1,12 @@ +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; diff --git a/mysql-test/t/table_keyinfo-6838.test b/mysql-test/t/table_keyinfo-6838.test new file mode 100644 index 00000000000..5f4c9f44f86 --- /dev/null +++ b/mysql-test/t/table_keyinfo-6838.test @@ -0,0 +1,18 @@ +# 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; diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 387fdb35ace..dabe46ed1fe 100644 --- a/sql/sql_select.cc +++ b/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) { diff --git a/sql/structs.h b/sql/structs.h index ae71819ae09..9840cec6a35 100644 --- a/sql/structs.h +++ b/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; diff --git a/sql/table.cc b/sql/table.cc index cfa0b6c09aa..7df2ae641ca 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -5968,18 +5968,32 @@ 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; key_part_info->null_offset= (uint) (field->null_ptr - (uchar*) record[0]); 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++; diff --git a/sql/table.h b/sql/table.h index 7a1e380f14c..832445486ab 100644 --- a/sql/table.h +++ b/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)