From 4bc268d40686fb94bb53c5bb0b85f4ea97d70fa6 Mon Sep 17 00:00:00 2001 From: Eugene Kosov <claprix@yandex.ru> Date: Wed, 20 Dec 2017 19:42:15 +0300 Subject: [PATCH] MDEV-14632 Assertion `!((new_col->prtype ^ col->prtype) & ~256U)' failed in row_log_table_apply_convert_mrec SQL, IB: proper fix is to disable unimplemented Online DDL for system-versioned tables inside InnoDB --- mysql-test/suite/versioning/r/alter.result | 4 -- mysql-test/suite/versioning/r/online.result | 32 +++++++++++++++ mysql-test/suite/versioning/t/alter.test | 6 +-- mysql-test/suite/versioning/t/online.test | 39 ++++++++++++++++++ sql/handler.cc | 45 +++++++++------------ sql/handler.h | 20 +++------ sql/sql_alter.h | 2 + sql/sql_table.cc | 4 ++ sql/sql_yacc.yy | 6 +-- storage/innobase/handler/handler0alter.cc | 14 +++++-- 10 files changed, 118 insertions(+), 54 deletions(-) create mode 100644 mysql-test/suite/versioning/r/online.result create mode 100644 mysql-test/suite/versioning/t/online.test diff --git a/mysql-test/suite/versioning/r/alter.result b/mysql-test/suite/versioning/r/alter.result index 279dfb7d10d..679bee34d35 100644 --- a/mysql-test/suite/versioning/r/alter.result +++ b/mysql-test/suite/versioning/r/alter.result @@ -305,10 +305,6 @@ a 1 call verify_vtq; No A B C D -alter table t drop system versioning, algorithm=inplace; -call verify_vtq; -No A B C D -alter table t add system versioning; alter table t drop system versioning, algorithm=copy; show create table t; Table Create Table diff --git a/mysql-test/suite/versioning/r/online.result b/mysql-test/suite/versioning/r/online.result new file mode 100644 index 00000000000..f56ae6244d5 --- /dev/null +++ b/mysql-test/suite/versioning/r/online.result @@ -0,0 +1,32 @@ +set system_versioning_alter_history=keep; +create or replace table t (a int) engine=innodb; +alter table t add system versioning, lock=none; +ERROR 0A000: LOCK=NONE is not supported for this operation. Try LOCK=SHARED +alter table t add system versioning; +alter table t add index idx(a), lock=none; +ERROR 0A000: LOCK=NONE is not supported for this operation. Try LOCK=SHARED +alter table t drop system versioning, lock=none; +ERROR 0A000: LOCK=NONE is not supported for this operation. Try LOCK=SHARED +set global system_versioning_transaction_registry=on; +Warnings: +Warning 4144 Transaction-based system versioning is EXPERIMENTAL and is subject to change in future. +create or replace table t (a int) engine=innodb; +alter table t +add s bigint unsigned as row start, +add e bigint unsigned as row end, +add period for system_time(s, e), +add system versioning, +lock=none; +ERROR 0A000: LOCK=NONE is not supported for this operation. Try LOCK=SHARED +alter table t +add s bigint unsigned as row start, +add e bigint unsigned as row end, +add period for system_time(s, e), +add system versioning; +alter table t add index idx(a), lock=none; +ERROR 0A000: LOCK=NONE is not supported for this operation. Try LOCK=SHARED +alter table t drop column s, drop column e; +alter table t drop system versioning, lock=none; +ERROR 0A000: LOCK=NONE is not supported for this operation. Try LOCK=SHARED +set global system_versioning_transaction_registry=off; +drop table t; diff --git a/mysql-test/suite/versioning/t/alter.test b/mysql-test/suite/versioning/t/alter.test index 2d88b7ad6f0..0a3beaf90fa 100644 --- a/mysql-test/suite/versioning/t/alter.test +++ b/mysql-test/suite/versioning/t/alter.test @@ -198,11 +198,12 @@ show create table t; select * from t for system_time all; call verify_vtq; -alter table t drop system versioning, algorithm=inplace; -call verify_vtq; ## FIXME: #414 IB: inplace for VERS_TIMESTAMP versioning if (0) { +alter table t drop system versioning, algorithm=inplace; +call verify_vtq; + alter table t add system versioning, algorithm=inplace; call verify_vtq; show create table t; @@ -220,7 +221,6 @@ alter table t drop column b, algorithm=inplace; show create table t; select * from t for system_time all; } -alter table t add system versioning; ## FIXME END alter table t drop system versioning, algorithm=copy; diff --git a/mysql-test/suite/versioning/t/online.test b/mysql-test/suite/versioning/t/online.test new file mode 100644 index 00000000000..6f182c6128d --- /dev/null +++ b/mysql-test/suite/versioning/t/online.test @@ -0,0 +1,39 @@ +--source include/have_innodb.inc + +set system_versioning_alter_history=keep; + +create or replace table t (a int) engine=innodb; + +--error ER_ALTER_OPERATION_NOT_SUPPORTED +alter table t add system versioning, lock=none; +alter table t add system versioning; +--error ER_ALTER_OPERATION_NOT_SUPPORTED +alter table t add index idx(a), lock=none; +--error ER_ALTER_OPERATION_NOT_SUPPORTED +alter table t drop system versioning, lock=none; + + +set global system_versioning_transaction_registry=on; +create or replace table t (a int) engine=innodb; + +--error ER_ALTER_OPERATION_NOT_SUPPORTED +alter table t + add s bigint unsigned as row start, + add e bigint unsigned as row end, + add period for system_time(s, e), + add system versioning, + lock=none; +alter table t + add s bigint unsigned as row start, + add e bigint unsigned as row end, + add period for system_time(s, e), + add system versioning; +--error ER_ALTER_OPERATION_NOT_SUPPORTED +alter table t add index idx(a), lock=none; +alter table t drop column s, drop column e; +--error ER_ALTER_OPERATION_NOT_SUPPORTED +alter table t drop system versioning, lock=none; + +set global system_versioning_transaction_registry=off; + +drop table t; diff --git a/sql/handler.cc b/sql/handler.cc index 921ac14e969..d5e3dbc66a8 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -6885,7 +6885,7 @@ bool Table_scope_and_contents_source_st::vers_fix_system_fields( List<Item> *items, bool *versioned_write) { - DBUG_ASSERT(!vers_info.without_system_versioning); + DBUG_ASSERT(!(alter_info->flags & Alter_info::ALTER_DROP_SYSTEM_VERSIONING)); int vers_tables= 0; if (select_tables) @@ -6901,13 +6901,13 @@ bool Table_scope_and_contents_source_st::vers_fix_system_fields( // then created table will be versioned. if (thd->variables.vers_force) { - vers_info.with_system_versioning= true; + alter_info->flags|= Alter_info::ALTER_ADD_SYSTEM_VERSIONING; options|= HA_VERSIONED_TABLE; } // Possibly override default storage engine to match one used in source table. - if (vers_tables && vers_info.with_system_versioning && - !(used_fields & HA_CREATE_USED_ENGINE)) + if (vers_tables && alter_info->flags & Alter_info::ALTER_ADD_SYSTEM_VERSIONING && + !(used_fields & HA_CREATE_USED_ENGINE)) { List_iterator_fast<Create_field> it(alter_info->create_list); while (Create_field *f= it++) @@ -6923,19 +6923,18 @@ bool Table_scope_and_contents_source_st::vers_fix_system_fields( } } - if (!vers_info.need_check()) + if (!vers_info.need_check(alter_info)) return false; - if (!vers_info.versioned_fields && - vers_info.unversioned_fields && - !vers_info.with_system_versioning) + if (!vers_info.versioned_fields && vers_info.unversioned_fields && + !(alter_info->flags & Alter_info::ALTER_ADD_SYSTEM_VERSIONING)) { // All is correct but this table is not versioned. options&= ~HA_VERSIONED_TABLE; return false; } - if (!vers_info.with_system_versioning && vers_info) + if (!(alter_info->flags & Alter_info::ALTER_ADD_SYSTEM_VERSIONING) && vers_info) { my_error(ER_MISSING, MYF(0), create_table.table_name, "WITH SYSTEM VERSIONING"); return true; @@ -6953,7 +6952,7 @@ bool Table_scope_and_contents_source_st::vers_fix_system_fields( while (Create_field *f= it++) { if ((f->versioning == Column_definition::VERSIONING_NOT_SET && - !vers_info.with_system_versioning) || + !(alter_info->flags & Alter_info::ALTER_ADD_SYSTEM_VERSIONING)) || f->versioning == Column_definition::WITHOUT_VERSIONING) { f->flags|= VERS_UPDATE_UNVERSIONED_FLAG; @@ -7110,28 +7109,16 @@ bool Vers_parse_info::fix_alter_info(THD *thd, Alter_info *alter_info, TABLE_SHARE *share= table->s; const char *table_name= share->table_name.str; - if (!need_check() && !share->versioned) + if (!need_check(alter_info) && !share->versioned) return false; - if (with_system_versioning || without_system_versioning) - { - // Disable Online DDL which is not implemented yet for ADD/DROP SYSTEM VERSIONING. - if (thd->mdl_context.upgrade_shared_lock(table->mdl_ticket, MDL_EXCLUSIVE, - thd->variables.lock_wait_timeout)) - { - my_error(ER_LOCK_WAIT_TIMEOUT, MYF(0)); - return true; - } - alter_info->requested_lock= Alter_info::ALTER_TABLE_LOCK_EXCLUSIVE; - } - - if (with_system_versioning && table->versioned()) + if (alter_info->flags & Alter_info::ALTER_ADD_SYSTEM_VERSIONING && table->versioned()) { my_error(ER_VERS_ALREADY_VERSIONED, MYF(0), table_name); return true; } - if (without_system_versioning) + if (alter_info->flags & Alter_info::ALTER_DROP_SYSTEM_VERSIONING) { if (!share->versioned) { @@ -7300,7 +7287,7 @@ bool Vers_parse_info::fix_alter_info(THD *thd, Alter_info *alter_info, if (fix_implicit(thd, alter_info)) return true; - if (with_system_versioning) + if (alter_info->flags & Alter_info::ALTER_ADD_SYSTEM_VERSIONING) { if (check_with_conditions(table_name)) return true; @@ -7370,6 +7357,12 @@ Vers_parse_info::fix_create_like(Alter_info &alter_info, HA_CREATE_INFO &create_ return false; } +bool Vers_parse_info::need_check(const Alter_info *alter_info) const +{ + return versioned_fields || unversioned_fields || + alter_info->flags & Alter_info::ALTER_ADD_SYSTEM_VERSIONING || + alter_info->flags & Alter_info::ALTER_DROP_SYSTEM_VERSIONING || *this; +} bool Vers_parse_info::check_with_conditions(const char *table_name) const { diff --git a/sql/handler.h b/sql/handler.h index d73661b9a77..13b8dc41f65 100644 --- a/sql/handler.h +++ b/sql/handler.h @@ -1715,8 +1715,6 @@ class Create_field; struct Vers_parse_info { Vers_parse_info() : - with_system_versioning(false), - without_system_versioning(false), versioned_fields(false), unversioned_fields(false) {} @@ -1762,15 +1760,7 @@ struct Vers_parse_info { return as_row.start || as_row.end || system_time.start || system_time.end; } - bool need_check() const - { - return - versioned_fields || - unversioned_fields || - with_system_versioning || - without_system_versioning || - *this; - } + bool need_check(const Alter_info *alter_info) const; bool check_with_conditions(const char *table_name) const; bool check_sys_fields(const char *table_name, Alter_info *alter_info, bool native) const; @@ -1784,10 +1774,6 @@ struct Vers_parse_info bool fix_create_like(Alter_info &alter_info, HA_CREATE_INFO &create_info, TABLE_LIST &src_table, TABLE_LIST &table); - /** Table definition has 'WITH/WITHOUT SYSTEM VERSIONING' */ - bool with_system_versioning : 1; - bool without_system_versioning : 1; - /** At least one field was specified 'WITH/WITHOUT SYSTEM VERSIONING'. Useful for error handling. @@ -2170,6 +2156,10 @@ class Alter_inplace_info static const HA_ALTER_FLAGS ALTER_COLUMN_UNVERSIONED = 1ULL << 42; + static const HA_ALTER_FLAGS ALTER_ADD_SYSTEM_VERSIONING= 1ULL << 43; + + static const HA_ALTER_FLAGS ALTER_DROP_SYSTEM_VERSIONING= 1ULL << 44; + /** Create options (like MAX_ROWS) for the new version of table. diff --git a/sql/sql_alter.h b/sql/sql_alter.h index 5dc41ebf413..5478ce74c10 100644 --- a/sql/sql_alter.h +++ b/sql/sql_alter.h @@ -97,6 +97,8 @@ class Alter_info ALTER_ADD_CHECK_CONSTRAINT = 1L << 27, ALTER_DROP_CHECK_CONSTRAINT = 1L << 28, ALTER_COLUMN_UNVERSIONED = 1L << 29, + ALTER_ADD_SYSTEM_VERSIONING = 1L << 30, + ALTER_DROP_SYSTEM_VERSIONING= 1L << 31, }; enum enum_enable_or_disable { LEAVE_AS_IS, ENABLE, DISABLE }; diff --git a/sql/sql_table.cc b/sql/sql_table.cc index c4c8361c92d..ebde0b6f726 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -6564,6 +6564,10 @@ static bool fill_alter_inplace_info(THD *thd, ha_alter_info->handler_flags|= Alter_inplace_info::ALTER_DROP_HISTORICAL; if (alter_info->flags & Alter_info::ALTER_COLUMN_UNVERSIONED) ha_alter_info->handler_flags|= Alter_inplace_info::ALTER_COLUMN_UNVERSIONED; + if (alter_info->flags & Alter_info::ALTER_ADD_SYSTEM_VERSIONING) + ha_alter_info->handler_flags|= Alter_inplace_info::ALTER_ADD_SYSTEM_VERSIONING; + if (alter_info->flags & Alter_info::ALTER_DROP_SYSTEM_VERSIONING) + ha_alter_info->handler_flags|= Alter_inplace_info::ALTER_DROP_SYSTEM_VERSIONING; /* If we altering table with old VARCHAR fields we will be automatically diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index dfc82a70784..8d3a57ed411 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -6242,7 +6242,7 @@ versioning_option: } else { - Lex->vers_get_info().with_system_versioning= true; + Lex->alter_info.flags|= Alter_info::ALTER_ADD_SYSTEM_VERSIONING; Lex->create_info.options|= HA_VERSIONED_TABLE; } } @@ -8250,12 +8250,12 @@ alter_list_item: | alter_lock_option | ADD SYSTEM VERSIONING_SYM { - Lex->vers_get_info().with_system_versioning= true; + Lex->alter_info.flags|= Alter_info::ALTER_ADD_SYSTEM_VERSIONING; Lex->create_info.options|= HA_VERSIONED_TABLE; } | DROP SYSTEM VERSIONING_SYM { - Lex->vers_get_info().without_system_versioning= true; + Lex->alter_info.flags|= Alter_info::ALTER_DROP_SYSTEM_VERSIONING; } ; diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc index 8944f27a041..346a94e6fa0 100644 --- a/storage/innobase/handler/handler0alter.cc +++ b/storage/innobase/handler/handler0alter.cc @@ -633,7 +633,7 @@ instant_alter_column_possible( const TABLE* table) { // Making table system-versioned instantly is not implemented yet. - if (ha_alter_info->create_info->vers_info.with_system_versioning) { + if (ha_alter_info->handler_flags & Alter_inplace_info::ALTER_ADD_SYSTEM_VERSIONING) { return false; } @@ -694,9 +694,10 @@ ha_innobase::check_if_supported_inplace_alter( { DBUG_ENTER("check_if_supported_inplace_alter"); - if (altered_table->versioned(VERS_TIMESTAMP)) { + if (altered_table->versioned(VERS_TIMESTAMP) + || ha_alter_info->handler_flags & Alter_inplace_info::ALTER_DROP_SYSTEM_VERSIONING) { DBUG_RETURN(HA_ALTER_INPLACE_NOT_SUPPORTED); - } + } /* Before 10.2.2 information about virtual columns was not stored in system tables. We need to do a full alter to rebuild proper 10.2.2+ @@ -1226,6 +1227,13 @@ ha_innobase::check_if_supported_inplace_alter( } } + // FIXME: implement Online DDL for system-versioned tables + DBUG_ASSERT(!altered_table->versioned(VERS_TIMESTAMP)); + if (altered_table->versioned(VERS_TRX_ID) + || ha_alter_info->handler_flags & Alter_inplace_info::ALTER_DROP_SYSTEM_VERSIONING) { + online = false; + } + DBUG_RETURN(online ? HA_ALTER_INPLACE_NO_LOCK_AFTER_PREPARE : HA_ALTER_INPLACE_SHARED_LOCK_AFTER_PREPARE); -- 2.30.9