Hi Varun, Please find my input below. I would like again to voice my dissatisfaction that I have to point out coding style violations in pretty much every patch I get. The other error in the patch is also pretty basic, ok to have it once but I think it is reasonable to expect more polished patches to be submitted for code review. On Sat, May 20, 2017 at 01:50:42AM +0530, Varun wrote:
revision-id: b400696386529fa63e79f723bbb06c05f7bae820 (mariadb-10.2.2-416-gb400696) parent(s): fff61e31eca83a0a9af7c30e2bcda8309e9c695a author: Varun Gupta committer: Varun Gupta timestamp: 2017-05-20 01:49:34 +0530 message:
MDEV-11196: Error:Run-Time Check Failure #2 - Stack around the variable 'key_buff' was corrupted, server crashes in opt_sum_query
extended keys feature disabled if the length of extended key is longer than MAX_KEY_LEN
--- mysql-test/r/innodb_ext_key.result | 45 ++++++++++++++++++++++++++++++++++++++ mysql-test/t/innodb_ext_key.test | 31 ++++++++++++++++++++++++++ sql/table.cc | 36 +++++++++++++++++++++++++++++- 3 files changed, 111 insertions(+), 1 deletion(-)
diff --git a/mysql-test/r/innodb_ext_key.result b/mysql-test/r/innodb_ext_key.result index 1305be8..66391d6 100644 --- a/mysql-test/r/innodb_ext_key.result +++ b/mysql-test/r/innodb_ext_key.result @@ -1133,5 +1133,50 @@ where index_date_updated= 10 and index_id < 800; id select_type table type possible_keys key key_len ref rows Extra 1 SIMPLE t2 range index_date_updated index_date_updated 13 NULL # Using index condition drop table t0,t1,t2; +# +# MDEV-11196: Error:Run-Time Check Failure #2 - Stack around the variable 'key_buff' +# was corrupted, server crashes in opt_sum_query +CREATE TABLE t1 ( +pk INT, +f1 VARCHAR(3), +f2 VARCHAR(1024), +PRIMARY KEY (pk), +KEY(f2) +) ENGINE=InnoDB CHARSET utf8; +INSERT INTO t1 VALUES (1,'foo','abc'),(2,'bar','def'); +SELECT MAX(t2.pk) FROM t1 t2 INNER JOIN t1 t3 ON t2.f1 = t3.f1 WHERE t2.pk <= 4; +MAX(t2.pk) +2 +drop table t1; +CREATE TABLE t1 ( +pk1 INT, +pk2 INT, +f1 VARCHAR(3), +f2 VARCHAR(1021), +PRIMARY KEY (pk1,pk2), +KEY(f2) +) ENGINE=InnoDB CHARSET utf8; +INSERT INTO t1 VALUES (1,2,'2','abc'),(2,3,'3','def'); +explain format= json +select * from t1 force index(f2) where pk1 <= 5 and pk2 <=5 and f2 = 'abc' and f1 <= '3'; +EXPLAIN +{ + "query_block": { + "select_id": 1, + "table": { + "table_name": "t1", + "access_type": "range", + "possible_keys": ["f2"], + "key": "f2", + "key_length": "3070", + "used_key_parts": ["f2", "pk1"], + "rows": 1, + "filtered": 100, + "index_condition": "t1.pk1 <= 5 and t1.pk2 <= 5 and t1.f2 = 'abc'", + "attached_condition": "t1.f1 <= '3'" + } + } +} +drop table t1; set optimizer_switch=@save_ext_key_optimizer_switch; SET SESSION STORAGE_ENGINE=DEFAULT; ...
diff --git a/sql/table.cc b/sql/table.cc index 398383e..7d88eba 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -2104,6 +2104,7 @@ int TABLE_SHARE::init_from_binary_frm_image(THD *thd, bool write, for (uint key=0 ; key < keys ; key++,keyinfo++) { uint usable_parts= 0; + KEY* key_first_info; This variable may be used un-initialized. If I change the above line to be KEY* key_first_info=(KEY*)0x1;
and run this statement: CREATE TABLE t1a ( pk INT, f1 VARCHAR(3), f2 VARCHAR(1024), f2a VARCHAR(1024), PRIMARY KEY (pk), KEY(f2), KEY(f2a) ) ENGINE=InnoDB CHARSET utf8; Then I get a crash.
keyinfo->name=(char*) share->keynames.type_names[key]; keyinfo->name_length= strlen(keyinfo->name); keyinfo->cache_name= @@ -2118,19 +2119,38 @@ int TABLE_SHARE::init_from_binary_frm_image(THD *thd, bool write, keyinfo->name_length+1); }
+ if(!key)
Coding style: 'if (' ...
+ key_first_info= keyinfo; + if (ext_key_parts > share->key_parts && key) { KEY_PART_INFO *new_key_part= (keyinfo-1)->key_part + (keyinfo-1)->ext_key_parts; uint add_keyparts_for_this_key= add_first_key_parts; + uint length_bytes= 0, len_null_byte= 0, ext_key_length= 0; + Field *field;
/* Do not extend the key that contains a component defined over the beginning of a field. */ for (i= 0; i < keyinfo->user_defined_key_parts; i++) - { + { Coding style: wrong identation.
uint fieldnr= keyinfo->key_part[i].fieldnr; + field= share->field[keyinfo->key_part[i].fieldnr-1]; + + if (field->null_ptr) + len_null_byte= HA_KEY_NULL_LENGTH; + + if (field->type() == MYSQL_TYPE_BLOB || + field->real_type() == MYSQL_TYPE_VARCHAR || + field->type() == MYSQL_TYPE_GEOMETRY) + { + length_bytes= HA_KEY_BLOB_LENGTH; + } + + ext_key_length+= keyinfo->key_part[i].length + len_null_byte + + length_bytes; if (share->field[fieldnr-1]->key_length() != keyinfo->key_part[i].length) { @@ -2139,6 +2159,20 @@ int TABLE_SHARE::init_from_binary_frm_image(THD *thd, bool write, } }
+ if (add_keyparts_for_this_key) + { + for (i= 0; i < add_keyparts_for_this_key; i++) + { + uint pk_part_length= key_first_info->key_part[i].store_length; + if (ext_key_length + pk_part_length > MAX_KEY_LENGTH) + { + add_keyparts_for_this_key= i; + break; + } + ext_key_length+= pk_part_length; + } + } + if (add_keyparts_for_this_key < (keyinfo->ext_key_parts - keyinfo->user_defined_key_parts)) {
BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog