maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Thirunarayanan!

On Jan 28, Thirunarayanan Balathandayuthapani wrote:
> revision-id: 322301e2434 (mariadb-10.2.21-2-g322301e2434)
> parent(s): fe4c65db1dc
> author: Thirunarayanan Balathandayuthapani <thiru@xxxxxxxxxxx>
> committer: Thirunarayanan Balathandayuthapani <thiru@xxxxxxxxxxx>
> timestamp: 2019-01-01 11:01:03 +0530
> message:
> 
> MDEV-16849 Extending indexed VARCHAR column should be instantaneous
> 
> - Addressed Serg's review comments.
> 
> ---
> diff --git a/mysql-test/suite/innodb/r/alter_varchar_change.result b/mysql-test/suite/innodb/r/alter_varchar_change.result
> index 5d2c6403aab..2f412aecbf1 100644
> --- a/mysql-test/suite/innodb/r/alter_varchar_change.result
> +++ b/mysql-test/suite/innodb/r/alter_varchar_change.result
> @@ -316,6 +316,137 @@ t1	CREATE TABLE `t1` (
>  DROP TABLE t1;
>  CREATE TABLE t1(f1 INT NOT NULL,
>  f2 VARCHAR(100),
> +INDEX idx(f2(10)),
> +INDEX idx1(f1))ENGINE=InnoDB;
> +CALL get_table_id("test/t1", @tbl_id);
> +ALTER TABLE t1 MODIFY f2 VARCHAR(50), DROP INDEX idx1;
> +CALL get_table_id("test/t1", @tbl1_id);
> +SELECT @tbl1_id = @tbl_id;
> +@tbl1_id = @tbl_id
> +0
> +SELECT @idx1_id = @idx_id;
> +@idx1_id = @idx_id
> +0

Why `DROP INDEX idx1` ?

As far as I remember, the point was "make VARCHAR shorter, but still
longer than a prefix". In this case there is no need to rebuild the
index, why was it rebuilt?

> +SHOW CREATE TABLE t1;
> +Table	Create Table
> +t1	CREATE TABLE `t1` (
> +  `f1` int(11) NOT NULL,
> +  `f2` varchar(50) DEFAULT NULL,
> +  KEY `idx` (`f2`(10))
> +) ENGINE=InnoDB DEFAULT CHARSET=latin1
> +DROP TABLE t1;
> +CREATE TABLE t1(f1 INT NOT NULL,
> +f2 VARCHAR(100),
> +INDEX idx(f2(10)),
> +INDEX idx1(f1))ENGINE=InnoDB;
> +CALL get_table_id("test/t1", @tbl_id);
> +ALTER TABLE t1 MODIFY f2 VARCHAR(5), DROP INDEX idx1;
> +CALL get_table_id("test/t1", @tbl1_id);
> +SELECT @tbl1_id = @tbl_id;
> +@tbl1_id = @tbl_id
> +0
> +SELECT @idx1_id = @idx_id;
> +@idx1_id = @idx_id
> +0

Correct. "VARCHAR is changed to be shorter than a prefix" means that the
index must be rebuilt.

> +SHOW CREATE TABLE t1;
> +Table	Create Table
> +t1	CREATE TABLE `t1` (
> +  `f1` int(11) NOT NULL,
> +  `f2` varchar(5) DEFAULT NULL,
> +  KEY `idx` (`f2`)
> +) ENGINE=InnoDB DEFAULT CHARSET=latin1
> +DROP TABLE t1;
...
> diff --git a/sql/sql_table.cc b/sql/sql_table.cc
> index 2864474a8e8..7eca33812b9 100644
> --- a/sql/sql_table.cc
> +++ b/sql/sql_table.cc
> @@ -6629,6 +6629,7 @@ static bool fill_alter_inplace_info(THD *thd,
>      Go through keys and check if the original ones are compatible
>      with new table.
>    */
> +  uint old_field_len= 0;
>    KEY *table_key;
>    KEY *table_key_end= table->key_info + table->s->keys;
>    KEY *new_key;
> @@ -6698,8 +6699,16 @@ static bool fill_alter_inplace_info(THD *thd,
>          Key definition has changed if we are using a different field or
>          if the user key part length is different.
>        */
> -      if (key_part->length <= new_part->length &&
> -          old_field->pack_length() < new_field->pack_length &&
> +      old_field_len= old_field->pack_length();
> +
> +      if (old_field->type() == MYSQL_TYPE_VARCHAR)
> +      {
> +        old_field_len= (old_field->pack_length()
> +                        - ((Field_varstring*) old_field)->length_bytes);
> +      }
> +
> +      if (key_part->length == old_field_len &&
> +          key_part->length <= new_part->length &&


Why <= here? Doesn't it mean that if key_part->length == new_part->length
then you'll still set ALTER_COLUMN_INDEX_LENGTH even if _nothing_
has changed?

>  	  (key_part->field->is_equal((Create_field*) new_field)
>             == IS_EQUAL_PACK_LENGTH))
>        {
> 
Regards,
Sergei
Chief Architect MariaDB
and security@xxxxxxxxxxx