Hi, Thirunarayanan! On Nov 13, Thirunarayanan Balathandayuthapani wrote:

revision-id: a04c60e6c99097c9b52f582b2af891f48b8f662d (mariadb-10.2.18-40-ga04c60e6c99) parent(s): ab1ce2204e959bea596817494e932754ab5cbe88 author: Thirunarayanan Balathandayuthapani <thiru@mariadb.com> committer: Thirunarayanan Balathandayuthapani <thiru@mariadb.com> timestamp: 2018-11-12 16:30:34 +0530 message:

MDEV-16849 Extending indexed VARCHAR column should be instantaneous

Analysis: ======== Increasing the length of the indexed varchar column is not an instant operation for innodb.

diff --git a/mysql-test/suite/innodb/r/alter_varchar_change.result b/mysql-test/suite/innodb/r/alter_varchar_change.result new file mode 100644 index 00000000000..b60bd7847db --- /dev/null +++ b/mysql-test/suite/innodb/r/alter_varchar_change.result @@ -0,0 +1,269 @@ ... +CALL get_table_id("test/t1", @tbl1_id); +SELECT @tbl1_id - @tbl_id; +@tbl1_id - @tbl_id +1

That's a bit fragile, you test that table id was changed exactly by 1. I don't think the exact difference matters here, so it'd be safer to test for `SELECT @tbl1_id = @tbl_id` (here and everywhere).

+SHOW CREATE TABLE t1; diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 02780e7d1e0..b9668ad0979 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -6687,18 +6687,29 @@ static bool fill_alter_inplace_info(THD *thd, key_part < end; key_part++, new_part++) { + new_field= get_field_by_index(alter_info, new_part->fieldnr); + old_field= table->field[key_part->fieldnr - 1]; /* + If there is a change in index length due to column expansion + like varchar(X) changed to varchar(X + N) and has a compatible + packed data representation, we mark it for fast/INPLACE change + in index definition. InnoDB supports INPLACE for this cases

no tabs in the server code, please. If you're using vim, I can show you my config that has different formatting style for innodb and the rest of the server :)

+ Key definition has changed if we are using a different field or - if the used key part length is different. It makes sense to - check lengths first as in case when fields differ it is likely - that lengths differ too and checking fields is more expensive - in general case. + if the user key part length is different. */ - if (key_part->length != new_part->length) + if (key_part->length != new_part->length && + new_field->field != NULL &&

can new_field->field be NULL here? when?

+ old_field->pack_length() < new_field->pack_length &&

Do you need it? I believe IS_EQUAL_PACK_LENGTH includes this check.

+ (key_part->field->is_equal((Create_field*) new_field) + == IS_EQUAL_PACK_LENGTH)) + { + ha_alter_info->handler_flags |= + Alter_inplace_info::ALTER_COLUMN_INDEX_LENGTH; + } + else if (key_part->length != new_part->length) goto index_changed;

- new_field= get_field_by_index(alter_info, new_part->fieldnr); -

Regards, Sergei Chief Architect MariaDB and security@mariadb.org