diff --git a/mysql-test/main/alter_table_online.result b/mysql-test/main/alter_table_online.result index 9e2cee8ed74..58185dd99bc 100644 --- a/mysql-test/main/alter_table_online.result +++ b/mysql-test/main/alter_table_online.result @@ -3,7 +3,7 @@ # create table t (a int); alter ignore table t add primary key (a), algorithm=copy, lock=none; -ERROR 0A000: LOCK=NONE is not supported. Reason: COPY algorithm requires a lock. Try LOCK=SHARED +ERROR 0A000: LOCK=NONE is not supported. Reason: ALTER IGNORE TABLE. Try LOCK=SHARED drop table t; # # MDEV-28771 Assertion `table->in_use&&tdc->flushed' failed after ALTER @@ -91,13 +91,13 @@ references t1 (a) on update cascade) engine=InnoDB; insert into t2 values (1),(2),(3); alter table t2 add c int, algorithm=copy, lock=none; -ERROR 0A000: LOCK=NONE is not supported. Reason: COPY algorithm requires a lock. Try LOCK=SHARED +ERROR 0A000: LOCK=NONE is not supported. Reason: ON UPDATE CASCADE. Try LOCK=SHARED alter table t2 add c int, algorithm=inplace, lock=none; create or replace table t2 (b int, foreign key (b) references t1 (a) on delete set null) engine=InnoDB; alter table t2 add c int, algorithm=copy, lock=none; -ERROR 0A000: LOCK=NONE is not supported. Reason: COPY algorithm requires a lock. Try LOCK=SHARED +ERROR 0A000: LOCK=NONE is not supported. Reason: ON DELETE SET NULL. Try LOCK=SHARED alter table t2 add c int, algorithm=inplace, lock=none; create or replace table t2 (b int, foreign key (b) references t1 (a) @@ -116,7 +116,7 @@ create table t2 (a int references t1 (a), b int references t1 (b) on update cascade) engine=InnoDB; insert into t2 values (1, 1),(2, 2); alter table t2 add c int, algorithm=copy, lock=none; -ERROR 0A000: LOCK=NONE is not supported. Reason: COPY algorithm requires a lock. Try LOCK=SHARED +ERROR 0A000: LOCK=NONE is not supported. Reason: ON UPDATE CASCADE. Try LOCK=SHARED alter table t2 add c int, algorithm=copy; alter table t2 add d int, algorithm=inplace; drop table t2, t1; @@ -131,7 +131,7 @@ row_end bigint unsigned generated always as row end, period for system_time (row_start, row_end)) engine=innodb with system versioning; alter table t1 add c int, algorithm=copy, lock=none; -ERROR 0A000: LOCK=NONE is not supported. Reason: COPY algorithm requires a lock. Try LOCK=SHARED +ERROR 0A000: LOCK=NONE is not supported. Reason: BIGINT GENERATED ALWAYS AS ROW_START. Try LOCK=SHARED alter table t1 add c int, algorithm=inplace; alter table t1 add d int, lock=none; set system_versioning_alter_history= default; @@ -142,7 +142,7 @@ drop table t1; # create table t (a serial, b int) engine=innodb; alter table t drop a, modify b serial, algorithm=copy, lock=none; -ERROR 0A000: LOCK=NONE is not supported. Reason: COPY algorithm requires a lock. Try LOCK=SHARED +ERROR 0A000: LOCK=NONE is not supported. Reason: CHANGE COLUMN ... AUTO_INCREMENT. Try LOCK=SHARED set statement sql_mode= NO_AUTO_VALUE_ON_ZERO for alter table t drop a, modify b serial, algorithm=copy, lock=none; create or replace table t (a serial, b int) engine=innodb; @@ -156,7 +156,7 @@ t CREATE TABLE `t` ( # a is unique in the old table, but is shrunk in the new one. # Only unsafe approach is fine because of possible collisions. alter table t modify a int, modify b serial, algorithm=copy, lock=none; -ERROR 0A000: LOCK=NONE is not supported. Reason: COPY algorithm requires a lock. Try LOCK=SHARED +ERROR 0A000: LOCK=NONE is not supported. Reason: CHANGE COLUMN ... AUTO_INCREMENT. Try LOCK=SHARED # # Check that we treat autoinc columns correctly modify old autoinc is # fine, adding new autoinc for existed column is unsafe. @@ -179,25 +179,27 @@ t CREATE TABLE `t` ( UNIQUE KEY `b` (`b`) ) ENGINE=InnoDB DEFAULT CHARSET=latin1 COLLATE=latin1_swedish_ci alter table t drop b, change c c serial, algorithm=copy, lock=none; -ERROR 0A000: LOCK=NONE is not supported. Reason: COPY algorithm requires a lock. Try LOCK=SHARED +ERROR 0A000: LOCK=NONE is not supported. Reason: CHANGE COLUMN ... AUTO_INCREMENT. Try LOCK=SHARED # Check existed unique keys. create or replace table t(a int, b int not null, c int not null, d int); # No unique in the old table; alter table t add unique(b, c), modify d int auto_increment, add key(d), algorithm=copy, lock=none; -ERROR 0A000: LOCK=NONE is not supported. Reason: COPY algorithm requires a lock. Try LOCK=SHARED +ERROR 0A000: LOCK=NONE is not supported. Reason: CHANGE COLUMN ... AUTO_INCREMENT. Try LOCK=SHARED alter table t add unique(a, b); # Unique in the old table has nulls; alter table t modify d int auto_increment, add key(d), algorithm=copy, lock=none; -ERROR 0A000: LOCK=NONE is not supported. Reason: COPY algorithm requires a lock. Try LOCK=SHARED +ERROR 0A000: LOCK=NONE is not supported. Reason: CHANGE COLUMN ... AUTO_INCREMENT. Try LOCK=SHARED alter table t add unique(b, c); -# Change unique'scolumn; -alter table t change b x int, modify d int auto_increment, add key(d), +# Change unique's column; +alter table t change b x bigint, modify d int auto_increment, add key(d), algorithm=copy, lock=none; -ERROR 0A000: LOCK=NONE is not supported. Reason: COPY algorithm requires a lock. Try LOCK=SHARED -# Finally good. -alter table t modify d int auto_increment, add key(d), +ERROR 0A000: LOCK=NONE is not supported. Reason: CHANGE COLUMN ... AUTO_INCREMENT. Try LOCK=SHARED +# Finally good. Simple renames with a type unchanged will not affect +# the result. Also NOT NULL -> NULL transform is fine. +alter table t modify d int auto_increment, add key(d), +change b x int null, algorithm=copy, lock=none; drop table t; # MDEV-31172 Server crash or ASAN errors in online_alter_check_autoinc diff --git a/mysql-test/main/alter_table_online.test b/mysql-test/main/alter_table_online.test index f439f2a1fbf..1d968f80a45 100644 --- a/mysql-test/main/alter_table_online.test +++ b/mysql-test/main/alter_table_online.test @@ -190,13 +190,15 @@ alter table t modify d int auto_increment, add key(d), algorithm=copy, lock=none; alter table t add unique(b, c); ---echo # Change unique'scolumn; +--echo # Change unique's column; --error ER_ALTER_OPERATION_NOT_SUPPORTED_REASON -alter table t change b x int, modify d int auto_increment, add key(d), +alter table t change b x bigint, modify d int auto_increment, add key(d), algorithm=copy, lock=none; ---echo # Finally good. -alter table t modify d int auto_increment, add key(d), +--echo # Finally good. Simple renames with a type unchanged will not affect +--echo # the result. Also NOT NULL -> NULL transform is fine. +alter table t modify d int auto_increment, add key(d), + change b x int null, algorithm=copy, lock=none; drop table t; diff --git a/mysql-test/main/alter_table_online_debug.result b/mysql-test/main/alter_table_online_debug.result index 5d1a7ffa72e..021267a15f5 100644 --- a/mysql-test/main/alter_table_online_debug.result +++ b/mysql-test/main/alter_table_online_debug.result @@ -491,7 +491,7 @@ a b UNIX_TIMESTAMP(row_start) UNIX_TIMESTAMP(row_end) 3 44 1.000000 2147483647.999999 6 77 1.000000 2147483647.999999 alter table t1 drop system versioning, algorithm= copy, lock= none; -ERROR 0A000: LOCK=NONE is not supported. Reason: COPY algorithm requires a lock. Try LOCK=SHARED +ERROR 0A000: LOCK=NONE is not supported. Reason: DROP SYSTEM VERSIONING. Try LOCK=SHARED # # Test ROLLBACK TO SAVEPOINT # diff --git a/sql/sql_class.h b/sql/sql_class.h index b9716ae943a..fa23c843039 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -1233,6 +1233,16 @@ public: { return strdup_root(mem_root,str); } inline char *strmake(const char *str, size_t size) const { return strmake_root(mem_root,str,size); } + inline LEX_CSTRING strcat(const LEX_CSTRING &a, const LEX_CSTRING &b) const + { + char *buf= (char*)alloc(a.length + b.length + 1); + if (unlikely(!buf)) + return null_clex_str; + memcpy(buf, a.str, a.length); + memcpy(buf + a.length, b.str, b.length); + buf[a.length + b.length] = 0; + return {buf, a.length + b.length}; + } inline void *memdup(const void *str, size_t size) const { return memdup_root(mem_root,str,size); } inline void *memdup_w_gap(const void *str, size_t size, size_t gap) const diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 80a4f7fd871..c8ca8c0efd0 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -9871,26 +9871,21 @@ bool online_alter_check_autoinc(const THD *thd, const Alter_info *alter_info, */ for (uint k= 0; k < table->s->keys; k++) { - const KEY &key= table->s->key_info[k]; + const KEY &key= table->key_info[k]; if ((key.flags & HA_NOSAME) == 0 || key.flags & HA_NULL_PART_KEY) continue; bool key_parts_good= true; for (uint kp= 0; kp < key.user_defined_key_parts && key_parts_good; kp++) { - const char *field_name= key.key_part[kp].field->field_name.str; - for (const auto &c: alter_info->drop_list) - if (c.type == Alter_drop::COLUMN - && my_strcasecmp(system_charset_info, c.name, field_name) == 0) - { - key_parts_good= false; - break; - } + const Field *f= key.key_part[kp].field; + // tmp_set contains dropped fields after mysql_prepare_alter_table + key_parts_good= !bitmap_is_set(&table->tmp_set, f->field_index); + if (key_parts_good) for (const auto &c: alter_info->create_list) - if (c.change.str && my_strcasecmp(system_charset_info, c.change.str, - field_name) == 0) + if (c.field == f) { - key_parts_good= false; + key_parts_good= f->is_equal(c); break; } } @@ -9898,25 +9893,67 @@ bool online_alter_check_autoinc(const THD *thd, const Alter_info *alter_info, return true; } - const char *old_autoinc= table->found_next_number_field - ? table->found_next_number_field->field_name.str - : ""; - bool online= true; for (const auto &c: alter_info->create_list) { - if (c.change.str && c.flags & AUTO_INCREMENT_FLAG) + if (c.flags & AUTO_INCREMENT_FLAG) { - if (my_strcasecmp(system_charset_info, c.change.str, old_autoinc) != 0) - { - if (c.create_if_not_exists // check IF EXISTS option - && table->find_field_by_name(&c.change) == NULL) - continue; - online= false; - } + if (c.field && !(c.field->flags & AUTO_INCREMENT_FLAG)) + return false; break; } } - return online; + return true; +} + +static +const char *online_alter_check_supported(const THD *thd, + const Alter_info *alter_info, + const TABLE *table, bool *online) +{ + DBUG_ASSERT(*online); + + *online= thd->locked_tables_mode != LTM_LOCK_TABLES && !table->s->tmp_table; + if (!*online) + return NULL; + + *online= (alter_info->flags & ALTER_DROP_SYSTEM_VERSIONING) == 0; + if (!*online) + return "DROP SYSTEM VERSIONING"; + + *online= !thd->lex->ignore; + if (!*online) + return "ALTER IGNORE TABLE"; + + *online= !table->versioned(VERS_TRX_ID); + if (!*online) + return "BIGINT GENERATED ALWAYS AS ROW_START"; + + List fk_list; + table->file->get_foreign_key_list(thd, &fk_list); + for (auto &fk: fk_list) + { + if (fk_modifies_child(fk.delete_method) || + fk_modifies_child(fk.update_method)) + { + *online= false; + // Don't fall to a common unsupported case to avoid heavy string ops. + if (alter_info->requested_lock == Alter_info::ALTER_TABLE_LOCK_NONE) + { + return fk_modifies_child(fk.delete_method) + ? thd->strcat({STRING_WITH_LEN("ON DELETE ")}, + *fk_option_name(fk.delete_method)).str + : thd->strcat({STRING_WITH_LEN("ON UPDATE ")}, + *fk_option_name(fk.update_method)).str; + } + return NULL; + } + } + + *online= online_alter_check_autoinc(thd, alter_info, table); + if (!*online) + return "CHANGE COLUMN ... AUTO_INCREMENT"; + + return NULL; } @@ -10100,9 +10137,7 @@ bool mysql_alter_table(THD *thd, const LEX_CSTRING *new_db, table_list->required_type= TABLE_TYPE_NORMAL; if (alter_info->requested_lock > Alter_info::ALTER_TABLE_LOCK_NONE - || alter_info->flags & ALTER_DROP_SYSTEM_VERSIONING || thd->lex->sql_command == SQLCOM_OPTIMIZE - || ignore || alter_info->algorithm(thd) > Alter_info::ALTER_TABLE_ALGORITHM_COPY) online= false; @@ -10142,22 +10177,6 @@ bool mysql_alter_table(THD *thd, const LEX_CSTRING *new_db, table= table_list->table; bool is_reg_table= table->s->tmp_table == NO_TMP_TABLE; - online= online && !table->s->tmp_table && !table->versioned(VERS_TRX_ID); - - List fk_list; - table->file->get_foreign_key_list(thd, &fk_list); - for (auto &fk: fk_list) - { - if (fk_modifies_child(fk.delete_method) - || fk_modifies_child(fk.update_method)) - { - online= false; - break; - } - } - - online= online && online_alter_check_autoinc(thd, alter_info, table); - #ifdef WITH_WSREP if (WSREP(thd) && (thd->lex->sql_command == SQLCOM_ALTER_TABLE || @@ -10979,22 +10998,22 @@ do_continue:; if (fk_prepare_copy_alter_table(thd, table, alter_info, &alter_ctx)) goto err_new_table_cleanup; - if (!table->s->tmp_table) + if (online) { - // COPY algorithm doesn't work with concurrent writes. - if (!online && + const char *reason= online_alter_check_supported(thd, alter_info, table, + &online); + if (reason && alter_info->requested_lock == Alter_info::ALTER_TABLE_LOCK_NONE) { + DBUG_ASSERT(!online); my_error(ER_ALTER_OPERATION_NOT_SUPPORTED_REASON, MYF(0), - "LOCK=NONE", - ER_THD(thd, ER_ALTER_OPERATION_NOT_SUPPORTED_REASON_COPY), - "LOCK=SHARED"); + "LOCK=NONE", reason, "LOCK=SHARED"); goto err_new_table_cleanup; } + } - if (thd->locked_tables_mode == LTM_LOCK_TABLES) - online= false; - + if (!table->s->tmp_table) + { // If EXCLUSIVE lock is requested, upgrade already. if (alter_info->requested_lock == Alter_info::ALTER_TABLE_LOCK_EXCLUSIVE && wait_while_table_is_used(thd, table, HA_EXTRA_FORCE_REOPEN))