maria-developers team mailing list archive

[Eugene Kosov <claprix@xxxxxxxxx>]

Hi, Sergei!

Thanks for you review.

04.02.2019, 21:59, "Sergei Golubchik" <serg@xxxxxxxxxxx>:
> Hi, Eugene!
>
> On Feb 04, Eugene Kosov wrote:
>>  revision-id: f93af1f93e2 (mariadb-10.3.12-19-gf93af1f93e2)
>>  parent(s): d4144c8e010
>>  author: Eugene Kosov <claprix@xxxxxxxxx>
>>  committer: Eugene Kosov <claprix@xxxxxxxxx>
>>  timestamp: 2019-01-23 19:32:02 +0300
>>  message:
>>
>>  MDEV-12836 Avoid table rebuild when adding or removing of auto_increment settings
>>
>>  ALTER_AUTO_INC: this flag indicates that ALTER operation
>>  adds or removes AUTO_INCREMENT from some field
>>
>>  Field::is_equal(): now sometime returns IS_EQUAL_BUT_AUTO_INC
>>  which means that only AUTO_INCREMENT was changed for that field
>
> You enable instant alter for adding auto_inc. This is not what
> Valerii asked for in MDEV-12836, and for a reason. Consider this test
> case:
>
>   create table t1 (a int unique);
>   insert t1 values (NULL),(NULL),(NULL);
>   alter table t1 modify a int auto_increment;

Here we ha_alter_info->handler_flags are:
ALTER_CHANGE_COLUMN_DEFAULT
ALTER_DROP_UNIQUE_INDEX
ALTER_ADD_PK_INDEX
ALTER_COLUMN_NOT_NULLABLE

This is in a tt-10.3-MDEV-12836-instant-auto-inc branch.
F.ex ALTER_COLUMN_NOT_NULLABLE requires rebuild in InnoDB not counting AUTO_INCREMENT at all.

So, I think this is not related to MDEV-12836


create table t1 (a int not null, unique key a_key (a)) engine=innodb;
insert into t1 values (19), (1), (100500);
alter table t1 modify a int not null auto_increment,algorithm=instant;
drop table t1;

In this modified test case we have ALTER_CHANGE_COLUMN_DEFAULT and ALTER_AUTO_INC which is can be done instantly now. And this is now the subject of MDEV-12836.
>
> here `alter table` modifies the data, actually generating auto-inc
> values for all NULL columns. I don't think you can do that instantly.
> You can only remove auto-inc instantly. And for that you don't need any
> big changes, just modify Field::is_equal() to allow it:
>
> diff --git a/sql/field.cc b/sql/field.cc
> --- a/sql/field.cc
> +++ b/sql/field.cc
> @@ -3507,7 +3507,7 @@ uint Field_new_decimal::is_equal(Create_field *new_field)
>    return ((new_field->type_handler() == type_handler()) &&
>            ((new_field->flags & UNSIGNED_FLAG) ==
>             (uint) (flags & UNSIGNED_FLAG)) &&
> - ((new_field->flags & AUTO_INCREMENT_FLAG) ==
> + ((new_field->flags & AUTO_INCREMENT_FLAG) <=
>             (uint) (flags & AUTO_INCREMENT_FLAG)) &&
>            (new_field->length == max_display_length()) &&
>            (new_field->decimals == dec));
> @@ -9508,7 +9508,7 @@ uint Field_num::is_equal(Create_field *new_field)
>    return ((new_field->type_handler() == type_handler()) &&
>            ((new_field->flags & UNSIGNED_FLAG) ==
>             (uint) (flags & UNSIGNED_FLAG)) &&
> - ((new_field->flags & AUTO_INCREMENT_FLAG) ==
> + ((new_field->flags & AUTO_INCREMENT_FLAG) <=
>            (uint) (flags & AUTO_INCREMENT_FLAG)) &&
>            (new_field->pack_length == pack_length()));
>  }
>
> And this should be enough.
>
> Regards,
> Sergei
> Chief Architect MariaDB
> and security@xxxxxxxxxxx

-- 
Eugene