Hi Varun,
Good to see that the feedback from the last time was addressed.
There is another issue, though.
Consider a case where some (but not necessarily all) PK columns are present in
the secondary key:
create table t1 (
pk1 VARCHAR(1000) not null,
pk2 INT not null,
col2 INT not null,
KEY key1(pk1, col2),
PRIMARY KEY(pk1, pk2)
) CHARSET utf8 engine=innodb;
insert into t1 select a,a,a from ten;
In this case, key1 still has a suffix, but the suffix only includes columns
that are not already present in the key.
That is, the "real" definition of key1 is
(pk1, col2, pk2)
and not
(pk1, col2, pk1, pk2)
.
Let's try that.
explain
select * from t1 force index(key1) where pk1 < 'zz' and col2 <=2 and pk2 <=4;
Without the patch, I have all 3 key parts used:
| {
"query_block": {
"select_id": 1,
"table": {
"table_name": "t1",
"access_type": "range",
"possible_keys": ["key1"],
"key": "key1",
"key_length": "3010",
"used_key_parts": ["pk1", "col2", "pk2"],
"rows": 1,
"filtered": 100,
"attached_condition": "t1.pk1 <= '0' and t1.col2 <= 2 and t1.pk2 <= 4",
"using_index": true
}
}
} |
With the patch, I only see the explicitly defined key parts used:
| {
"query_block": {
"select_id": 1,
"table": {
"table_name": "t1",
"access_type": "range",
"possible_keys": ["key1"],
"key": "key1",
"key_length": "3006",
"used_key_parts": ["pk1", "col2"],
"rows": 1,
"filtered": 100,
"attached_condition": "t1.pk1 <= '0' and t1.col2 <= 2 and t1.pk2 <= 4",
"using_index": true
}
}
} |
Which I think is wrong, because the key length never exceeds MAX_KEY_LEN.
This can be easily fixed by appyling this patch over your patch:
--- table.cc.varun 2017-05-24 18:30:39.076341082 +0300
+++ table.cc 2017-05-24 19:04:25.637740027 +0300
@@ -2164,10 +2164,13 @@
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)
+ if (keyinfo->ext_key_part_map & 1<<i)
{
- add_keyparts_for_this_key= i;
- break;
+ if (ext_key_length + pk_part_length > MAX_KEY_LENGTH)
+ {
+ add_keyparts_for_this_key= i;
+ break;
+ }
}
ext_key_length+= pk_part_length;
}
Do you agree with this change? Any amendments? If it's ok, please add this
(together with the testcase) into your patch.
I've checked MySQL 5.7 also, and they do something weird:
...
insert into t1 select A.a + 1000*B.a, A.a + 1000*B.a, A.a + 1000*B.a from one_k A, ten B;
explain format=json
select * from t1 force index(key1) where pk1 <= '0' and col2 <=2 and pk2 <=4;
EXPLAIN
{
"query_block": {
"select_id": 1,
"cost_info": {
"query_cost": "1.41"
},
"table": {
"table_name": "t1",
"access_type": "range",
"possible_keys": [
"key1"
],
"key": "key1",
"used_key_parts": [
"pk1"
],
"key_length": "3010",
"rows_examined_per_scan": 1,
"rows_produced_per_join": 0,
"filtered": "11.11",
"using_index": true,
"cost_info": {
"read_cost": "1.39",
"eval_cost": "0.02",
"prefix_cost": "1.41",
"data_read_per_join": "335"
},
"used_columns": [
"pk1",
"pk2",
"col2"
],
"attached_condition": "((`test`.`t1`.`pk1` <= '0') and (`test`.`t1`.`col2` <= 2) and (`test`.`t1`.`pk2` <= 4))"
}
}
}
key_length is 3010, but used_key_parts only includes "pk1"... The numbers do
not match.
--
On Tue, May 23, 2017 at 07:08:48PM +0530, Varun wrote:
> revision-id: 5ac7ee32a8e373d989bf9d665b5e01f770ac583f (mariadb-10.1.20-282-g5ac7ee3)
> parent(s): a1b6128dedb4419db9fadaf94c356d3477d4e06f
> author: Varun Gupta
> committer: Varun Gupta
> timestamp: 2017-05-23 19:00:15 +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 | 51 ++++++++++++++++++++++++++++++ ++++++++
> mysql-test/t/innodb_ext_key.test | 38 ++++++++++++++++++++++++++++
> sql/table.cc | 36 ++++++++++++++++++++++++++-
> 3 files changed, 124 insertions(+), 1 deletion(-)
>
> diff --git a/mysql-test/r/innodb_ext_key.result b/mysql-test/r/innodb_ext_key. result
> index 1305be8..c76660c 100644
> --- a/mysql-test/r/innodb_ext_key.result
> +++ b/mysql-test/r/innodb_ext_key.result
> @@ -1133,5 +1133,56 @@ 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
> +set @save_innodb_file_format= @@innodb_file_format;
> +set @save_innodb_large_prefix= @@innodb_large_prefix;
> +set global innodb_file_format = BARRACUDA;
> +set global innodb_large_prefix = ON;
> +CREATE TABLE t1 (
> +pk INT,
> +f1 VARCHAR(3),
> +f2 VARCHAR(1024),
> +PRIMARY KEY (pk),
> +KEY(f2)
> +) ENGINE=InnoDB CHARSET utf8 ROW_FORMAT= DYNAMIC;
> +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 ROW_FORMAT= DYNAMIC;
> +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 global innodb_file_format = @save_innodb_file_format;
> +set global innodb_large_prefix = @save_innodb_large_prefix;
> SET SESSION STORAGE_ENGINE=DEFAULT;
> diff --git a/mysql-test/t/innodb_ext_key.test b/mysql-test/t/innodb_ext_key. test
> index bf94b7d..adb032f 100644
> --- a/mysql-test/t/innodb_ext_key.test
> +++ b/mysql-test/t/innodb_ext_key.test
> @@ -778,5 +778,43 @@ where index_date_updated= 10 and index_id < 800;
>
> drop table t0,t1,t2;
>
> +
> +--echo #
> +--echo # MDEV-11196: Error:Run-Time Check Failure #2 - Stack around the variable 'key_buff'
> +--echo # was corrupted, server crashes in opt_sum_query
> +
> +set @save_innodb_file_format= @@innodb_file_format;
> +set @save_innodb_large_prefix= @@innodb_large_prefix;
> +set global innodb_file_format = BARRACUDA;
> +set global innodb_large_prefix = ON;
> +
> +CREATE TABLE t1 (
> + pk INT,
> + f1 VARCHAR(3),
> + f2 VARCHAR(1024),
> + PRIMARY KEY (pk),
> + KEY(f2)
> +) ENGINE=InnoDB CHARSET utf8 ROW_FORMAT= DYNAMIC;
> +
> +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;
> +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 ROW_FORMAT= DYNAMIC;
> +
> +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';
> +drop table t1;
> +
> set optimizer_switch=@save_ext_key_optimizer_switch;
> +set global innodb_file_format = @save_innodb_file_format;
> +set global innodb_large_prefix = @save_innodb_large_prefix;
> SET SESSION STORAGE_ENGINE=DEFAULT;
> diff --git a/sql/table.cc b/sql/table.cc
> index 37c0b63..c8c9696 100644
> --- a/sql/table.cc
> +++ b/sql/table.cc
> @@ -1721,6 +1721,7 @@ int TABLE_SHARE::init_from_binary_frm_image(THD *thd, bool write,
> keyinfo= share->key_info;
> uint primary_key= my_strcasecmp(system_charset_info, share->keynames.type_names[0],
> primary_key_name) ? MAX_KEY : 0;
> + KEY* key_first_info;
>
> if (primary_key >= MAX_KEY && keyinfo->flags & HA_NOSAME)
> {
> @@ -1800,19 +1801,38 @@ int TABLE_SHARE::init_from_binary_frm_image(THD *thd, bool write,
> keyinfo->name_length+1);
> }
>
> + if (!key)
> + 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++)
> - {
> + {
> 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)
> {
> @@ -1821,6 +1841,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))
> {
> _______________________________________________
> commits mailing list
> commits@mariadb.org
> https://lists.askmonty.org/cgi-bin/mailman/listinfo/ commits
BR
Sergei
--
Sergei Petrunia, Software Developer
MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog