maria-developers team mailing list archive

[Varun Gupta <varungupta1803@xxxxxxxxx>]

On Wed, May 24, 2017 at 9:45 PM, Sergey Petrunia <sergey@xxxxxxxxxxx> wrote:

> 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.


Yes I agree with the above change. I have added the testcase to the patch
and would be sending it in another commit.

>
>
> 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@xxxxxxxxxxx
> > 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
>
>
>