diff --git a/mysql-test/suite/versioning/r/alter.result b/mysql-test/suite/versioning/r/alter.result index 7608b8511db..56f694f0d9d 100644 --- a/mysql-test/suite/versioning/r/alter.result +++ b/mysql-test/suite/versioning/r/alter.result @@ -88,9 +88,9 @@ t CREATE TABLE `t` ( `a` int(11) DEFAULT NULL ) ENGINE=MyISAM DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_uca1400_ai_ci WITH SYSTEM VERSIONING alter table t add column trx_start timestamp(6) as row start; -ERROR HY000: Duplicate ROW START column `trx_start` +ERROR HY000: Wrong parameters for `t`: missing 'AS ROW END' alter table t modify a int as row start; -ERROR HY000: Duplicate ROW START column `a` +ERROR 42000: Incorrect column specifier for column 'a' alter table t add column b int; show create table t; Table Create Table @@ -879,3 +879,58 @@ t1 CREATE TABLE `t1` ( ) ENGINE=MyISAM DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_uca1400_ai_ci WITH SYSTEM VERSIONING drop table t1; # End of 10.5 tests +# +# MDEV-27293 Allow converting a versioned table from implicit +# to explicit row_start/row_end columns +# +create table t1 (x int) with system versioning; +insert t1 values (0), (1); +update t1 set x= x + 10; +set @old_system_versioning_alter_history= @@system_versioning_alter_history; +set @@system_versioning_alter_history= keep; +alter table t1 add column y int; +show create table t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `x` int(11) DEFAULT NULL, + `y` int(11) DEFAULT NULL +) ENGINE=MyISAM DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_uca1400_ai_ci WITH SYSTEM VERSIONING +alter table t1 add column rs timestamp(6) as row start; +ERROR HY000: Wrong parameters for `t1`: missing 'AS ROW END' +alter table t1 add column re timestamp(6) as row end, add period for system_time (row_start, re); +ERROR HY000: Wrong parameters for `t1`: missing 'AS ROW START' +alter table t1 add column rs timestamp(6) as row start, add column re timestamp(6) as row end; +ERROR HY000: Wrong parameters for `t1`: missing 'PERIOD FOR SYSTEM_TIME' +alter table t1 rename column row_start to re; +ERROR 42S22: Unknown column 'row_start' in 't1' +alter table t1 add column rs timestamp(5) as row start, add column re timestamp(6) as row end, add period for system_time (rs,re); +ERROR 42000: Incorrect column specifier for column 'rs' +alter table t1 add column rs timestamp(6) as row start, add column re timestamp(6) as row end, add period for system_time (rs,re); +show create table t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `x` int(11) DEFAULT NULL, + `y` int(11) DEFAULT NULL, + `rs` timestamp(6) GENERATED ALWAYS AS ROW START, + `re` timestamp(6) GENERATED ALWAYS AS ROW END, + PERIOD FOR SYSTEM_TIME (`rs`, `re`) +) ENGINE=MyISAM DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_uca1400_ai_ci WITH SYSTEM VERSIONING +select x, check_row_ts(rs, re) from t1 for system_time all order by x; +x check_row_ts(rs, re) +0 HISTORICAL ROW +1 HISTORICAL ROW +10 CURRENT ROW +11 CURRENT ROW +create or replace table t1 (x int) engine innodb with system versioning; +alter table t1 add column rs timestamp(6) as row start, add column re timestamp(6) as row end, add period for system_time (rs,re); +show create table t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `x` int(11) DEFAULT NULL, + `rs` timestamp(6) GENERATED ALWAYS AS ROW START, + `re` timestamp(6) GENERATED ALWAYS AS ROW END, + PERIOD FOR SYSTEM_TIME (`rs`, `re`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_uca1400_ai_ci WITH SYSTEM VERSIONING +set @@system_versioning_alter_history= @old_system_versioning_alter_history; +drop table t1; +# End of 11.6 tests diff --git a/mysql-test/suite/versioning/t/alter.test b/mysql-test/suite/versioning/t/alter.test index 85d40b1e8e9..6460c5ad58d 100644 --- a/mysql-test/suite/versioning/t/alter.test +++ b/mysql-test/suite/versioning/t/alter.test @@ -74,9 +74,9 @@ alter table t add column trx_start timestamp(6) as row start; alter table t add system versioning; show create table t; ---error ER_VERS_DUPLICATE_ROW_START_END +--error ER_MISSING alter table t add column trx_start timestamp(6) as row start; ---error ER_VERS_DUPLICATE_ROW_START_END +--error ER_WRONG_FIELD_SPEC alter table t modify a int as row start; alter table t add column b int; @@ -553,7 +553,6 @@ create or replace table t1 (f1 int) with system versioning; alter table t1 drop system versioning, add f2 int with system versioning; drop table t1; ---source suite/versioning/common_finish.inc --echo # MDEV-16490 It's possible to make a system versioned table without any versioning field set @@system_versioning_alter_history=keep; @@ -781,3 +780,38 @@ show create table t1; drop table t1; --echo # End of 10.5 tests + +--echo # +--echo # MDEV-27293 Allow converting a versioned table from implicit +--echo # to explicit row_start/row_end columns +--echo # +create table t1 (x int) with system versioning; +insert t1 values (0), (1); +update t1 set x= x + 10; +set @old_system_versioning_alter_history= @@system_versioning_alter_history; +set @@system_versioning_alter_history= keep; +alter table t1 add column y int; +show create table t1; +--error ER_MISSING +alter table t1 add column rs timestamp(6) as row start; +--error ER_MISSING +alter table t1 add column re timestamp(6) as row end, add period for system_time (row_start, re); +--error ER_MISSING +alter table t1 add column rs timestamp(6) as row start, add column re timestamp(6) as row end; +--error ER_BAD_FIELD_ERROR +alter table t1 rename column row_start to re; +--error ER_WRONG_FIELD_SPEC +alter table t1 add column rs timestamp(5) as row start, add column re timestamp(6) as row end, add period for system_time (rs,re); +alter table t1 add column rs timestamp(6) as row start, add column re timestamp(6) as row end, add period for system_time (rs,re); +show create table t1; +select x, check_row_ts(rs, re) from t1 for system_time all order by x; + +create or replace table t1 (x int) engine innodb with system versioning; +alter table t1 add column rs timestamp(6) as row start, add column re timestamp(6) as row end, add period for system_time (rs,re); +show create table t1; + +set @@system_versioning_alter_history= @old_system_versioning_alter_history; +drop table t1; + +--echo # End of 11.6 tests +--source suite/versioning/common_finish.inc diff --git a/sql/field.h b/sql/field.h index 8f2f6768b3f..6b09da4dc3f 100644 --- a/sql/field.h +++ b/sql/field.h @@ -6033,6 +6033,12 @@ ulonglong TABLE::vers_start_id() const return static_cast(vers_start_field()->val_int()); } +inline +bool TABLE::vers_implicit() const +{ + return vers_end_field()->invisible == INVISIBLE_SYSTEM; +} + double pos_in_interval_for_string(CHARSET_INFO *cset, const uchar *midp_val, uint32 midp_len, const uchar *min_val, uint32 min_len, diff --git a/sql/handler.cc b/sql/handler.cc index 0bf74e3bc3b..9e9c5f7c0a1 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -8796,7 +8796,7 @@ bool Vers_parse_info::fix_alter_info(THD *thd, Alter_info *alter_info, if (!(alter_info->flags & ALTER_ADD_SYSTEM_VERSIONING)) { - List_iterator_fast it(alter_info->create_list); + List_iterator it(alter_info->create_list); while (Create_field *f= it++) { if (f->flags & VERS_SYSTEM_FIELD) @@ -8806,9 +8806,27 @@ bool Vers_parse_info::fix_alter_info(THD *thd, Alter_info *alter_info, my_error(ER_VERS_NOT_VERSIONED, MYF(0), table->s->table_name.str); return true; } - my_error(ER_VERS_DUPLICATE_ROW_START_END, MYF(0), - f->flags & VERS_ROW_START ? "START" : "END", f->field_name.str); - return true; + if (!table->vers_implicit()) + { + my_error(ER_VERS_DUPLICATE_ROW_START_END, MYF(0), + f->flags & VERS_ROW_START ? "START" : "END", f->field_name.str); + + return true; + } + Field *old= f->flags & VERS_ROW_START ? table->vers_start_field() : table->vers_end_field(); + if (old->type_handler() == f->type_handler() && + old->field_length == f->length && + (old->flags & UNSIGNED_FLAG) == (f->flags & UNSIGNED_FLAG)) + { + alter_info->flags|= ALTER_VERS_EXPLICIT; + alter_info->add_alter_list(thd, old->field_name, f->field_name, false); + it.remove(); + } + else + { + my_error(ER_WRONG_FIELD_SPEC, MYF(0), f->field_name.str); + return true; + } } } } @@ -8822,7 +8840,8 @@ bool Vers_parse_info::fix_alter_info(THD *thd, Alter_info *alter_info, if (share->versioned) { - if (alter_info->flags & ALTER_ADD_PERIOD) + if (!table->vers_implicit() && + (alter_info->flags & ALTER_ADD_PERIOD)) { my_error(ER_VERS_ALREADY_VERSIONED, MYF(0), table_name.str); return true; @@ -8833,42 +8852,41 @@ bool Vers_parse_info::fix_alter_info(THD *thd, Alter_info *alter_info, DBUG_ASSERT(share->vers_start_field()); DBUG_ASSERT(share->vers_end_field()); - Lex_ident_column start(share->vers_start_field()->field_name); - Lex_ident_column end(share->vers_end_field()->field_name); - DBUG_ASSERT(start.str); - DBUG_ASSERT(end.str); - as_row= start_end_t(start, end); - period= as_row; - - if (alter_info->create_list.elements) + if (!(alter_info->flags & ALTER_VERS_EXPLICIT)) { - List_iterator_fast it(alter_info->create_list); - while (Create_field *f= it++) - { - if (f->versioning == Column_definition::WITHOUT_VERSIONING) - f->flags|= VERS_UPDATE_UNVERSIONED_FLAG; + Lex_ident_column start(share->vers_start_field()->field_name); + Lex_ident_column end(share->vers_end_field()->field_name); + DBUG_ASSERT(start.str); + DBUG_ASSERT(end.str); - if (f->change.str && (start.streq(f->change) || end.streq(f->change))) + as_row= start_end_t(start, end); + period= as_row; + + if (alter_info->create_list.elements) + { + List_iterator_fast it(alter_info->create_list); + while (Create_field *f= it++) { - my_error(ER_VERS_ALTER_SYSTEM_FIELD, MYF(0), f->change.str); - return true; + if (f->versioning == Column_definition::WITHOUT_VERSIONING) + f->flags|= VERS_UPDATE_UNVERSIONED_FLAG; + + if (f->change.str && (start.streq(f->change) || end.streq(f->change))) + { + my_error(ER_VERS_ALTER_SYSTEM_FIELD, MYF(0), f->change.str); + return true; + } } } - } + } /* if (!convert_explicit) */ + return check_conditions(table_name, share->db); + } /* if (share->versioned) */ - return false; - } - - if (fix_implicit(thd, alter_info)) + if ((alter_info->flags & ALTER_ADD_SYSTEM_VERSIONING) && + (fix_implicit(thd, alter_info) || + check_sys_fields(table_name, share->db, alter_info))) return true; - if (alter_info->flags & ALTER_ADD_SYSTEM_VERSIONING) - { - if (check_sys_fields(table_name, share->db, alter_info)) - return true; - } - return false; } diff --git a/sql/handler.h b/sql/handler.h index 10061daf321..25f8314415a 100644 --- a/sql/handler.h +++ b/sql/handler.h @@ -730,6 +730,7 @@ typedef bool Log_func(THD*, TABLE*, Event_log *, binlog_cache_data *, bool, #define ALTER_DROP_SYSTEM_VERSIONING (1ULL << 32) #define ALTER_ADD_PERIOD (1ULL << 33) #define ALTER_DROP_PERIOD (1ULL << 34) +#define ALTER_VERS_EXPLICIT (1ULL << 35) /* Following defines are used by ALTER_INPLACE_TABLE diff --git a/sql/sql_alter.h b/sql/sql_alter.h index cf4ce3b3ce4..2c0d8323059 100644 --- a/sql/sql_alter.h +++ b/sql/sql_alter.h @@ -325,6 +325,9 @@ public: } uint check_vcol_field(Item_field *f) const; + bool add_alter_list(THD *thd, LEX_CSTRING name, LEX_CSTRING new_name, + bool exists); + private: Alter_info &operator=(const Alter_info &rhs); // not implemented Alter_info(const Alter_info &rhs); // not implemented diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index a8bb502a919..bebfef4d231 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -562,14 +562,14 @@ bool LEX::add_alter_list(LEX_CSTRING name, Virtual_column_info *expr, return false; } - -bool LEX::add_alter_list(LEX_CSTRING name, LEX_CSTRING new_name, bool exists) +bool Alter_info::add_alter_list(THD *thd, LEX_CSTRING name, + LEX_CSTRING new_name, bool exists) { Alter_column *ac= new (thd->mem_root) Alter_column(name, new_name, exists); if (unlikely(ac == NULL)) return true; - alter_info.alter_list.push_back(ac, thd->mem_root); - alter_info.flags|= ALTER_RENAME_COLUMN; + alter_list.push_back(ac, thd->mem_root); + flags|= ALTER_RENAME_COLUMN; return false; } diff --git a/sql/sql_lex.h b/sql/sql_lex.h index 1baf351d6ca..55fd7dc652a 100644 --- a/sql/sql_lex.h +++ b/sql/sql_lex.h @@ -4425,7 +4425,11 @@ public: } bool add_alter_list(LEX_CSTRING par_name, Virtual_column_info *expr, bool par_exists); - bool add_alter_list(LEX_CSTRING name, LEX_CSTRING new_name, bool exists); + bool add_alter_list(LEX_CSTRING name, LEX_CSTRING new_name, bool exists) + { + return alter_info.add_alter_list(thd, name, new_name, exists); + } + bool add_alter_list_item_convert_to_charset(Sql_used *used, const Charset_collation_map_st &map, CHARSET_INFO *cs) diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 83f024ecece..7c7349f06aa 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -6691,7 +6691,7 @@ static bool fill_alter_inplace_info(THD *thd, TABLE *table, alter_table_operations flags_to_remove= ALTER_ADD_INDEX | ALTER_DROP_INDEX | ALTER_PARSER_ADD_COLUMN | ALTER_PARSER_DROP_COLUMN | ALTER_COLUMN_ORDER | ALTER_RENAME_COLUMN | - ALTER_CHANGE_COLUMN; + ALTER_CHANGE_COLUMN | ALTER_VERS_EXPLICIT; if (!table->file->native_versioned()) flags_to_remove|= ALTER_COLUMN_UNVERSIONED; @@ -8425,7 +8425,8 @@ mysql_prepare_alter_table(THD *thd, TABLE *table, dropped_sys_vers_fields|= field->flags; drop_it.remove(); } - else if (field->invisible < INVISIBLE_SYSTEM) + else if (field->invisible < INVISIBLE_SYSTEM || + (alter_info->flags & ALTER_VERS_EXPLICIT)) { /* This field was not dropped and not changed, add it to the list @@ -8440,12 +8441,14 @@ mysql_prepare_alter_table(THD *thd, TABLE *table, if (field->field_name.streq(alter->name)) break; } - if (alter && field->invisible < INVISIBLE_SYSTEM) + if (alter) { if (alter->is_rename()) { def->change= Lex_ident_column(alter->name); def->field_name= Lex_ident_column(alter->new_name); + if (vers_system_invisible) + def->invisible= VISIBLE; column_rename_param.fields.push_back(def); if (field->flags & VERS_ROW_START) { diff --git a/sql/table.h b/sql/table.h index e3aec27d37a..6084701b913 100644 --- a/sql/table.h +++ b/sql/table.h @@ -1937,6 +1937,7 @@ public: bool vers_switch_partition(THD *thd, TABLE_LIST *table_list, Open_table_context *ot_ctx); #endif + bool vers_implicit() const; int update_generated_fields(); void period_prepare_autoinc();