Hi, Thirunarayanan! On Dec 13, Thirunarayanan Balathandayuthapani wrote: > revision-id: c16ad313bc4 (mariadb-10.2.19-4-gc16ad313bc4) > parent(s): 7e756437789 > author: Thirunarayanan Balathandayuthapani <th...@mariadb.com> > committer: Thirunarayanan Balathandayuthapani <th...@mariadb.com> > timestamp: 2018-11-15 15:14:02 +0530 > message: > > MDEV-16849 Extending indexed VARCHAR column should be instantaneous ... > 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..5d2c6403aab > --- /dev/null > +++ b/mysql-test/suite/innodb/r/alter_varchar_change.result > @@ -0,0 +1,335 @@ ... > +CREATE TABLE t1(f1 INT NOT NULL, > +f2 VARCHAR(100), > +INDEX idx(f2(10)))ENGINE=InnoDB; > +CALL get_table_id("test/t1", @tbl_id); > +CALL get_index_id(@tbl_id, "idx", @idx_id); > +ALTER TABLE t1 MODIFY f2 VARCHAR(200), DROP INDEX idx, ADD INDEX idx(f2(50)); > +CALL get_table_id("test/t1", @tbl1_id); > +CALL get_index_id(@tbl1_id, "idx", @idx1_id); > +SELECT @tbl1_id = @tbl_id; > +@tbl1_id = @tbl_id > +1 > +SELECT @idx1_id = @idx_id; > +@idx1_id = @idx_id > +1
Is that right? Old index would sort values "aaaaaaaaaa1", "aaaaaaaaaa2", "aaaaaaaaaa3" as equal, so they could be in the index in any order. But in the new index they aren't equal. > +CREATE TABLE t1(f1 INT NOT NULL, > +f2 VARCHAR(100), FULLTEXT idx(f2))ENGINE=InnoDB; > +CALL get_table_id("test/t1", @tbl_id); > +CALL get_index_id(@tbl_id, "idx", @idx_id); > +ALTER TABLE t1 MODIFY f2 VARCHAR(200); please, add a case where there's a FULLTEXT index, as above, but ALTER TABLE makes a column shorter. The index must be rebuilt. > +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); > +CALL get_index_id(@tbl_id, "idx", @idx_id); > +ALTER TABLE t1 MODIFY f2 VARCHAR(200), DROP INDEX idx1; Please also add a test case where VARCHAR is made shorter, but still longer than a prefix: ALTER TABLE t1 MODIFY f2 VARCHAR(50) The index should not be rebuilt. And shorter than a prefix: ALTER TABLE t1 MODIFY f2 VARCHAR(5) The index must be rebuilt. Also add a test when ALTER TABLE changes column's charset, or column type (CHAR/VARCHAR/TEXT). And a test where column's length is changed over 255. E.g. from 200 to 300. > diff --git a/sql/sql_table.cc b/sql/sql_table.cc > --- a/sql/sql_table.cc > +++ b/sql/sql_table.cc > @@ -6687,18 +6687,28 @@ 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 > + > 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 && > + old_field->pack_length() < new_field->pack_length && I still don't understand this condition. Why do you care whether a field length has changed, while you're should interested to know whether an index length have changed? Field length changes are completely irrelevant here. again, the only possible ALTER_COLUMN_INDEX_LENGTH case: * key part length was the same as column length, and it was not decreased. So, your condition should, probably, be something like if (key_part->length == old_field->pack_length() && key_part->length < new_part->length) > + (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); > - > /* > For prefix keys KEY_PART_INFO::field points to cloned Field > object with adjusted length. So below we have to check field Regards, Sergei Chief Architect MariaDB and secur...@mariadb.org _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp