Skip to content

Commit

Permalink
MDEV-31631 Adding auto-increment to table with history online misbehaves
Browse files Browse the repository at this point in the history
Adding an auto_increment column online leads to an undefined behavior.
Basically any DEFAULTs that depend on a row order in the table, or on
the non-deterministic (in scope of the ALTER TABLE statement) function
is UB.

For example, NOW() is considered generally non-deterministic
(Item_func_now_utc is marked with VCOL_NON_DETERMINISTIC), but it's fixed
in scope of a single statement.

Same for any other function that depends only on the session/status vars
apart from its arguments.

Only two UB cases are known:
* adding new AUTO_INCREMENT column. Modifying the existing column may be
fine under certain circumstances, see MDEV-31058.
* adding new column with DEFAULT(nextval(...)). Modifying the existing
column is possible, since its value will be always present in the online
event, except for the NULL -> NOT NULL modification
  • Loading branch information
FooBarrior authored and vuvova committed Aug 15, 2023
1 parent e026a36 commit 44ca37e
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 32 deletions.
14 changes: 14 additions & 0 deletions mysql-test/main/alter_table_online.result
Expand Up @@ -224,3 +224,17 @@ alter table s engine=Aria, lock=none;
ERROR 0A000: LOCK=NONE is not supported. Reason: SEQUENCE. Try LOCK=SHARED
alter table s engine=Aria;
drop sequence s;
# MDEV-31631 Adding auto-increment column to a table with history online
# behaves differently from non-online
create sequence s;
create table t1(a int, x int NULL default(nextval(s)));
alter table t1 add b int default (nextval(s)), lock=none;
ERROR 0A000: LOCK=NONE is not supported. Reason: Function or expression 'NEXTVAL()' cannot be used in the DEFAULT clause of `b`. Try LOCK=SHARED
alter table t1 add b int primary key auto_increment, lock=none;
ERROR 0A000: LOCK=NONE is not supported. Reason: ADD COLUMN ... AUTO_INCREMENT. Try LOCK=SHARED
create table t2(a int, b int NULL default(nextval(s)));
alter table t2 modify b int not null default (nextval(s)), lock=none;
ERROR 0A000: LOCK=NONE is not supported. Reason: Function or expression 'NEXTVAL()' cannot be used in the DEFAULT clause of `b`. Try LOCK=SHARED
drop table t2;
drop table t1;
drop sequence s;
18 changes: 18 additions & 0 deletions mysql-test/main/alter_table_online.test
Expand Up @@ -228,3 +228,21 @@ create sequence s engine=MyISAM;
alter table s engine=Aria, lock=none;
alter table s engine=Aria;
drop sequence s;


--echo # MDEV-31631 Adding auto-increment column to a table with history online
--echo # behaves differently from non-online
create sequence s;
create table t1(a int, x int NULL default(nextval(s)));
--error ER_ALTER_OPERATION_NOT_SUPPORTED_REASON
alter table t1 add b int default (nextval(s)), lock=none;
--error ER_ALTER_OPERATION_NOT_SUPPORTED_REASON
alter table t1 add b int primary key auto_increment, lock=none;

create table t2(a int, b int NULL default(nextval(s)));
--error ER_ALTER_OPERATION_NOT_SUPPORTED_REASON
alter table t2 modify b int not null default (nextval(s)), lock=none;

drop table t2;
drop table t1;
drop sequence s;
17 changes: 0 additions & 17 deletions mysql-test/main/alter_table_online_debug.result
Expand Up @@ -990,23 +990,6 @@ set debug_sync= reset;
#
set @old_dbug=@@debug_dbug;
set debug_dbug="+d,rpl_report_chosen_key";
create table t (a int);
insert into t values (10),(20),(30);
set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit';
alter table t add pk int auto_increment primary key, algorithm=copy, lock=none;
connection con2;
set debug_sync= 'now wait_for downgraded';
delete from t where a = 20;
update t set a = a + 1 where a = 10;
set debug_sync= 'now signal goforit';
connection default;
Warnings:
Note 1105 Key chosen: -1
Note 1105 Key chosen: -1
select * from t;
a pk
11 1
30 3
#
# Add clumsy DEFAULT
#
Expand Down
32 changes: 17 additions & 15 deletions mysql-test/main/alter_table_online_debug.test
Expand Up @@ -1172,21 +1172,23 @@ set debug_sync= reset;
set @old_dbug=@@debug_dbug;
set debug_dbug="+d,rpl_report_chosen_key";

create table t (a int);
insert into t values (10),(20),(30);

set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit';
--send
alter table t add pk int auto_increment primary key, algorithm=copy, lock=none;
--connection con2
set debug_sync= 'now wait_for downgraded';
delete from t where a = 20;
update t set a = a + 1 where a = 10;
set debug_sync= 'now signal goforit';

--connection default
--reap
select * from t;
# UB
#
# create table t (a int);
# insert into t values (10),(20),(30);
#
# set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit';
# --send
# alter table t add pk int auto_increment primary key, algorithm=copy, lock=none;
# --connection con2
# set debug_sync= 'now wait_for downgraded';
# delete from t where a = 20;
# update t set a = a + 1 where a = 10;
# set debug_sync= 'now signal goforit';
#
# --connection default
# --reap
# select * from t;

--echo #
--echo # Add clumsy DEFAULT
Expand Down
27 changes: 27 additions & 0 deletions sql/sql_table.cc
Expand Up @@ -9953,6 +9953,33 @@ const char *online_alter_check_supported(const THD *thd,
}
}

for (auto &c: alter_info->create_list)
{
*online= c.field || !(c.flags & AUTO_INCREMENT_FLAG);
if (!*online)
return "ADD COLUMN ... AUTO_INCREMENT";

auto *def= c.default_value;
*online= !(def && def->flags & VCOL_NEXTVAL
// either it's a new field, or a NULL -> NOT NULL change
&& (!c.field || (!(c.field->flags & NOT_NULL_FLAG)
&& (c.flags & NOT_NULL_FLAG))));
if (!*online)
{
if (alter_info->requested_lock != Alter_info::ALTER_TABLE_LOCK_NONE)
return NULL; // Avoid heavy string op
const char *fmt= ER_THD(thd, ER_GENERATED_COLUMN_FUNCTION_IS_NOT_ALLOWED);

LEX_CSTRING dflt{STRING_WITH_LEN("DEFAULT")};
LEX_CSTRING nxvl{STRING_WITH_LEN("NEXTVAL()")};
size_t len= strlen(fmt) + nxvl.length + c.field_name.length + dflt.length;
char *resp= (char*)thd->alloc(len);
// expression %s cannot be used in the %s clause of %`s
my_snprintf(resp, len, fmt, nxvl.str, dflt.str, c.field_name.str);
return resp;
}
}

*online= online_alter_check_autoinc(thd, alter_info, table);
if (!*online)
return "CHANGE COLUMN ... AUTO_INCREMENT";
Expand Down

0 comments on commit 44ca37e

Please sign in to comment.