0.5: basic support for ALTER TABLE for InnoDB and other storage engines [closes #57]

mysql-test/suite/versioning/r/create.result

@@ -88,35 +88,35 @@ Sys_start int generated always as row start,
 Sys_end timestamp(6) generated always as row end,
 period for system_time (Sys_start, Sys_end)
 ) with system versioning;
-ERROR HY000: `Sys_start` must be of type `TIMESTAMP` for versioned table `t1`
+ERROR HY000: `Sys_start` must be of type `TIMESTAMP(6)` for versioned table `t1`
 create or replace table t1 (
 XNo int unsigned,
 Sys_start timestamp(6) generated always as row start,
 Sys_end int generated always as row end,
 period for system_time (Sys_start, Sys_end)
 ) with system versioning;
-ERROR HY000: `Sys_end` must be of type `TIMESTAMP` for versioned table `t1`
+ERROR HY000: `Sys_end` must be of type `TIMESTAMP(6)` for versioned table `t1`
 create or replace table t1 (
 XNo int unsigned,
 Sys_start timestamp(6) generated always as row start,
 Sys_end bigint generated always as row end,
 period for system_time (Sys_start, Sys_end)
 ) with system versioning engine innodb;
-ERROR HY000: `Sys_start` must be of type `BIGINT UNSIGNED` for versioned table `t1`
+ERROR HY000: `Sys_start` must be of type `BIGINT(20) UNSIGNED` for versioned table `t1`
 create or replace table t1 (
 XNo int unsigned,
 Sys_start bigint generated always as row start,
 Sys_end bigint generated always as row end,
 period for system_time (Sys_start, Sys_end)
 ) with system versioning engine innodb;
-ERROR HY000: `Sys_start` must be of type `BIGINT UNSIGNED` for versioned table `t1`
+ERROR HY000: `Sys_start` must be of type `BIGINT(20) UNSIGNED` for versioned table `t1`
 create or replace table t1 (
 XNo int unsigned,
 Sys_start bigint unsigned generated always as row start,
 Sys_end bigint generated always as row end,
 period for system_time (Sys_start, Sys_end)
 ) with system versioning engine innodb;
-ERROR HY000: `Sys_end` must be of type `BIGINT UNSIGNED` for versioned table `t1`
+ERROR HY000: `Sys_end` must be of type `BIGINT(20) UNSIGNED` for versioned table `t1`
 create or replace table t1 (
 A int with system versioning,
 B int
@@ -195,4 +195,32 @@ create or replace table t1 (
 A int without system versioning
 ) with system versioning;
 ERROR HY000: Wrong parameters for versioned table `t1`: versioned fields missing
+create or replace table t1 (
+A int without system versioning with system versioning
+);
+ERROR HY000: Wrong parameters for versioned table `t1`: Versioning specified more than once for the same field
+create or replace table t1 (
+A int with system versioning without system versioning
+);
+ERROR HY000: Wrong parameters for versioned table `t1`: Versioning specified more than once for the same field
+create table t(
+a int
+) without system versioning;
+ERROR HY000: Wrong parameters for versioned table `t`: 'WITHOUT SYSTEM VERSIONING' is not allowed
+create or replace table t1 (
+A int
+) without system versioning with system versioning;
+ERROR HY000: Wrong parameters for versioned table `t1`: Versioning specified more than once for the same table
+create or replace table t1 (
+A int
+) with system versioning without system versioning;
+ERROR HY000: Wrong parameters for versioned table `t1`: Versioning specified more than once for the same table
+create or replace table t1 (
+A int
+) with system versioning with system versioning;
+ERROR HY000: Wrong parameters for versioned table `t1`: Versioning specified more than once for the same table
+create or replace table t1 (
+A int
+) without system versioning without system versioning;
+ERROR HY000: Wrong parameters for versioned table `t1`: Versioning specified more than once for the same table
 drop table t1;

mysql-test/suite/versioning/t/alter.test

+create table t(
+  a int
+);
+show create table t;
+--error ER_VERS_WRONG_PARAMS
+alter table t without system versioning;
+--error ER_VERS_WRONG_PARAMS
+alter table t with system versioning without system versioning;
+
+alter table t with system versioning;
+show create table t;
+
+alter table t without system versioning;
+show create table t;
+
+--error ER_VERS_FIELD_WRONG_TYPE
+alter table t
+  add column trx_start bigint(20) unsigned generated always as row start,
+  add column trx_end bigint(20) unsigned generated always as row end,
+  add period for system_time(trx_start, trx_end),
+  with system versioning;
+
+--error ER_VERS_FIELD_WRONG_TYPE
+alter table t
+  add column trx_start timestamp generated always as row start,
+  add column trx_end timestamp generated always as row end,
+  add period for system_time(trx_start, trx_end),
+  with system versioning;
+
+--error ER_PARSE_ERROR
+alter table t
+  add column trx_start timestamp(6) not null generated always as row start,
+  add column trx_end timestamp(6) not null generated always as row end,
+  add period for system_time(trx_start, trx_end),
+  with system versioning;
+
+alter table t
+  add column trx_start timestamp(6) generated always as row start,
+  add column trx_end timestamp(6) generated always as row end,
+  add period for system_time(trx_start, trx_end),
+  with system versioning;
+show create table t;
+
+alter table t without system versioning;
+show create table t;
+
+alter table t with system versioning;
+show create table t;
+
+alter table t add column b int;
+show create table t;
+
+alter table t add column c int;
+show create table t;
+
+alter table t add column d int first;
+show create table t;
+
+alter table t add column e int after d;
+show create table t;
+
+--error ER_VERS_WRONG_PARAMS
+alter table t add column f int after sys_trx_start;
+--error ER_VERS_WRONG_PARAMS
+alter table t add column f int after sys_trx_end;
+
+alter table t drop column a;
+show create table t;
+
+--error ER_VERS_WRONG_PARAMS
+alter table t drop column sys_trx_start;
+--error ER_VERS_WRONG_PARAMS
+alter table t drop column sys_trx_end;
+
+create or replace table t(
+  a int
+);
+insert into t values(1);
+alter table t with system versioning;
+show create table t;
+insert into t values(2);
+select * from t for system_time all;
+select * from t;
+
+update t set a=3 where a=1;
+select * from t;
+select * from t for system_time all;
+select sys_trx_start from t where a=3 into @tm;
+alter table t add column b int;
+select @tm=sys_trx_start from t where a=3;
+show create table t;
+select * from t;
+select * from t for system_time all;
+
+alter table t without system versioning;
+select * from t;
+show create table t;
+
+--error ER_VERS_WRONG_PARAMS
+alter table t modify a int with system versioning;
+--error ER_VERS_WRONG_PARAMS
+alter table t modify a int with system versioning with system versioning;
+--error ER_VERS_WRONG_PARAMS
+alter table t modify a int with system versioning without system versioning;
+
+alter table t with system versioning;
+
+alter table t modify a int without system versioning;
+show create table t;
+
+alter table t modify a int with system versioning;
+show create table t;
+
+-- source suite/versioning/common.inc
+create or replace table t(
+  a int
+) engine=innodb;
+
+insert into t values(1);
+select * from t;
+
+--error ER_VERS_FIELD_WRONG_TYPE
+alter table t
+  add column trx_start timestamp(6) generated always as row start,
+  add column trx_end timestamp(6) generated always as row end,
+  add period for system_time(trx_start, trx_end),
+  with system versioning;
+
+alter table t
+  add column trx_start bigint(20) unsigned generated always as row start,
+  add column trx_end bigint(20) unsigned generated always as row end,
+  add period for system_time(trx_start, trx_end),
+  with system versioning;
+show create table t;
+alter table t without system versioning;
+
+--error ER_ALTER_OPERATION_NOT_SUPPORTED
+alter table t with system versioning, algorithm=inplace;
+
+alter table t with system versioning, algorithm=copy;
+show create table t;
+
+update t set a=2;
+select * from t for system_time all;
+
+alter table t add column b int, algorithm=copy;
+show create table t;
+select * from t;
+
+alter table t drop column b, algorithm=copy;
+show create table t;
+select * from t for system_time all;
+
+alter table t add column b int, algorithm=inplace;
+show create table t;
+select * from t;
+
+alter table t drop column b, algorithm=inplace;
+show create table t;
+select * from t for system_time all;
+
+--error ER_ALTER_OPERATION_NOT_SUPPORTED
+alter table t without system versioning, algorithm=inplace;
+
+alter table t without system versioning, algorithm=copy;
+show create table t;
+
+call verify_vtq;
+drop table t;
+drop procedure verify_vtq;

mysql-test/suite/versioning/t/create.test

@@ -169,4 +169,39 @@ create or replace table t1 (
   A int without system versioning
 ) with system versioning;
 
+--error ER_VERS_WRONG_PARAMS
+create or replace table t1 (
+  A int without system versioning with system versioning
+);
+
+--error ER_VERS_WRONG_PARAMS
+create or replace table t1 (
+  A int with system versioning without system versioning
+);
+
+--error ER_VERS_WRONG_PARAMS
+create table t(
+  a int
+) without system versioning;
+
+--error ER_VERS_WRONG_PARAMS
+create or replace table t1 (
+  A int
+) without system versioning with system versioning;
+
+--error ER_VERS_WRONG_PARAMS
+create or replace table t1 (
+  A int
+) with system versioning without system versioning;
+
+--error ER_VERS_WRONG_PARAMS
+create or replace table t1 (
+  A int
+) with system versioning with system versioning;
+
+--error ER_VERS_WRONG_PARAMS
+create or replace table t1 (
+  A int
+) without system versioning without system versioning;
+
 drop table t1;

sql/field.h

@@ -4208,6 +4208,7 @@ class Copy_field :public Sql_alloc {
   uint from_length,to_length;
   Field *from_field,*to_field;
   String tmp;					// For items
+  MYSQL_TIME now;                               // Used for sys_trx_start
 
   Copy_field() {}
   ~Copy_field() {}

sql/handler.h

@@ -1674,7 +1674,8 @@ struct Schema_specification_st
 struct Vers_parse_info
 {
   Vers_parse_info() :
-    declared_system_versioning(false),
+    declared_with_system_versioning(false),
+    declared_without_system_versioning(false),
     has_versioned_fields(false),
     has_unversioned_fields(false)
   {}
@@ -1698,12 +1699,36 @@ struct Vers_parse_info
   }
 
 private:
+  bool is_trx_start(const char *name) const;
+  bool is_trx_end(const char *name) const;
   bool fix_implicit(THD *thd, Alter_info *alter_info, bool integer_fields);
+  bool need_to_check() const
+  {
+    return
+      has_versioned_fields ||
+      has_unversioned_fields ||
+      declared_with_system_versioning ||
+      declared_without_system_versioning ||
+      period_for_system_time.start ||
+      period_for_system_time.end ||
+      generated_as_row.start ||
+      generated_as_row.end;
+  }
+  bool check_with_conditions(const char *table_name) const;
+  bool check_generated_type(const char *table_name, Alter_info *alter_info,
+                            bool integer_fields) const;
+
 public:
-  bool check_and_fix_implicit(THD *thd, Alter_info *alter_info, bool integer_fields, const char* table_name);
+  bool check_and_fix_implicit(THD *thd, Alter_info *alter_info,
+                              bool integer_fields, const char *table_name);
+  bool check_and_fix_alter(THD *thd, Alter_info *alter_info,
+                           HA_CREATE_INFO *create_info, TABLE_SHARE *share);
 
   /** User has added 'WITH SYSTEM VERSIONING' to table definition */
-  bool declared_system_versioning : 1;
+  bool declared_with_system_versioning : 1;
+
+  /** Use has added 'WITHOUT SYSTEM VERSIONING' to ALTER TABLE */
+  bool declared_without_system_versioning : 1;
 
   /**
      At least one field was specified 'WITH SYSTEM VERSIONING'. Useful for

sql/sql_table.cc

@@ -55,6 +55,8 @@
 #include "transaction.h"
 #include "sql_audit.h"
 #include "sql_sequence.h"
+#include "tztime.h"
+
 
 #ifdef __WIN__
 #include <io.h>
@@ -8718,6 +8720,12 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
   if (check_engine(thd, alter_ctx.new_db, alter_ctx.new_name, create_info))
     DBUG_RETURN(true);
 
+  if (create_info->vers_info.check_and_fix_alter(thd, alter_info, create_info,
+                                                 table->s))
+  {
+    DBUG_RETURN(true);
+  }
+
   if ((create_info->db_type != table->s->db_type() ||
        alter_info->flags & Alter_info::ALTER_PARTITION) &&
       !table->file->can_switch_engines())
@@ -9654,6 +9662,10 @@ copy_data_between_tables(THD *thd, TABLE *from, TABLE *to,
   sql_mode_t save_sql_mode= thd->variables.sql_mode;
   ulonglong prev_insert_id, time_to_report_progress;
   Field **dfield_ptr= to->default_field;
+  bool make_versioned= !from->versioned() && to->versioned();
+  bool make_unversioned= from->versioned() && !to->versioned();
+  Field *to_sys_trx_start= NULL, *from_sys_trx_end= NULL, *to_sys_trx_end= NULL;
+  MYSQL_TIME now;
   DBUG_ENTER("copy_data_between_tables");
 
   /* Two or 3 stages; Sorting, copying data and update indexes */
@@ -9753,6 +9765,19 @@ copy_data_between_tables(THD *thd, TABLE *from, TABLE *to,
     thd_progress_next_stage(thd);
   }
 
+  if (make_versioned)
+  {
+    thd->variables.time_zone->gmt_sec_to_TIME(&now, thd->query_start());
+    now.second_part= thd->query_start_sec_part();
+    thd->time_zone_used= 1;
+    to_sys_trx_start= to->field[to->s->row_start_field];
+    to_sys_trx_end= to->field[to->s->row_end_field];
+  }
+  else if (make_unversioned)
+  {
+    from_sys_trx_end= from->field[from->s->row_end_field];
+  }
+
   THD_STAGE_INFO(thd, stage_copy_to_tmp_table);
   /* Tell handler that we have values for all columns in the to table */
   to->use_all_columns();
@@ -9806,6 +9831,25 @@ copy_data_between_tables(THD *thd, TABLE *from, TABLE *to,
     {
       copy_ptr->do_copy(copy_ptr);
     }
+
+    if (make_versioned)
+    {
+      to_sys_trx_start->set_notnull(to_sys_trx_start->null_offset());
+      // TODO: write directly to record bypassing the same checks on every call
+      to_sys_trx_start->store_time(&now);
+
+      static const timeval max_tv= {0x7fffffff, 0};
+      static const uint dec= 6;
+      to_sys_trx_end->set_notnull(to_sys_trx_end->null_offset());
+      my_timestamp_to_binary(&max_tv, to_sys_trx_end->ptr, dec);
+    }
+    else if (make_unversioned)
+    {
+      // Drop history rows.
+      if (!from_sys_trx_end->is_max())
+        continue;
+    }
+
     prev_insert_id= to->file->next_insert_id;
     if (to->default_field)
       to->update_default_fields(0, ignore);

sql/sql_yacc.yy

@@ -5855,10 +5855,31 @@ create_table_option:
 	  }
         | WITH_SYSTEM_SYM VERSIONING
           {
+            const char *table_name=
+                Lex->create_last_non_select_table->table_name;
             Vers_parse_info &info= Lex->vers_get_info();
-            info.declared_system_versioning= true;
+            if (info.declared_with_system_versioning ||
+                info.declared_without_system_versioning)
+              my_yyabort_error(
+                  (ER_VERS_WRONG_PARAMS, MYF(0), table_name,
+                   "Versioning specified more than once for the same table"));
+
+            info.declared_with_system_versioning= true;
             Lex->create_info.options|= HA_VERSIONED_TABLE;
           }
+	| WITHOUT SYSTEM VERSIONING
+          {
+            const char *table_name=
+                Lex->create_last_non_select_table->table_name;
+            Vers_parse_info &info= Lex->vers_get_info();
+            if (info.declared_with_system_versioning ||
+                info.declared_without_system_versioning)
+              my_yyabort_error(
+                  (ER_VERS_WRONG_PARAMS, MYF(0), table_name,
+                   "Versioning specified more than once for the same table"));
+
+            info.declared_without_system_versioning= true;
+          }
         ;
 
 default_charset:
@@ -6679,12 +6700,26 @@ serial_attribute:
           }
         | WITH_SYSTEM_SYM VERSIONING
           {
+            if (Lex->last_field->versioning !=
+                Column_definition::VERSIONING_NOT_SET)
+              my_yyabort_error(
+                  (ER_VERS_WRONG_PARAMS, MYF(0),
+                   Lex->create_last_non_select_table->table_name,
+                   "Versioning specified more than once for the same field"));
+
             Lex->last_field->versioning = Column_definition::WITH_VERSIONING;
             Lex->create_info.vers_info.has_versioned_fields= true;
             Lex->create_info.options|= HA_VERSIONED_TABLE;
           }
         | WITHOUT SYSTEM VERSIONING
           {
+            if (Lex->last_field->versioning !=
+                Column_definition::VERSIONING_NOT_SET)
+              my_yyabort_error(
+                  (ER_VERS_WRONG_PARAMS, MYF(0),
+                   Lex->create_last_non_select_table->table_name,
+                   "Versioning specified more than once for the same field"));
+
             Lex->last_field->versioning = Column_definition::WITHOUT_VERSIONING;
             Lex->create_info.vers_info.has_unversioned_fields= true;
             Lex->create_info.options|= HA_VERSIONED_TABLE;

sql/table.cc

@@ -2551,44 +2551,15 @@ int TABLE_SHARE::init_from_binary_frm_image(THD *thd, bool write,
   else
   {
     DBUG_PRINT("info", ("Setting system versioning informations"));
-    uint16 row_start = uint2korr(system_period);
-    uint16 row_end =  uint2korr(system_period + sizeof(uint16));
+    uint16 row_start= uint2korr(system_period);
+    uint16 row_end= uint2korr(system_period + sizeof(uint16));
     if (row_start >= share->fields || row_end >= share->fields)
       goto err;
-    DBUG_PRINT("info", ("Columns with system versioning: [%d, %d]", row_start, row_end));
+    DBUG_PRINT("info", ("Columns with system versioning: [%d, %d]", row_start,
+                        row_end));
     share->enable_system_versioning(row_start, row_end);
     vers_start_field()->flags|= VERS_SYS_START_FLAG;
     vers_end_field()->flags|= VERS_SYS_END_FLAG;
-
-    DBUG_ASSERT(db_type());
-    if (db_type()->versioned())
-    {
-      if (vers_start_field()->type() != MYSQL_TYPE_LONGLONG
-        || !(vers_start_field()->flags & UNSIGNED_FLAG))
-      {
-        my_error(ER_VERS_FIELD_WRONG_TYPE, MYF(0), vers_start_field()->field_name, "BIGINT UNSIGNED", share->table_name);
-        goto err;
-      }
-      if (vers_end_field()->type() != MYSQL_TYPE_LONGLONG
-        || !(vers_end_field()->flags & UNSIGNED_FLAG))
-      {
-        my_error(ER_VERS_FIELD_WRONG_TYPE, MYF(0), vers_end_field()->field_name, "BIGINT UNSIGNED", share->table_name);
-        goto err;
-      }
-    }
-    else
-    {
-      if (vers_start_field()->type() != MYSQL_TYPE_TIMESTAMP)
-      {
-        my_error(ER_VERS_FIELD_WRONG_TYPE, MYF(0), vers_start_field()->field_name, "TIMESTAMP", share->table_name);
-        goto err;
-      }
-      if (vers_end_field()->type() != MYSQL_TYPE_TIMESTAMP)
-      {
-        my_error(ER_VERS_FIELD_WRONG_TYPE, MYF(0), vers_end_field()->field_name, "TIMESTAMP", share->table_name);
-        goto err;
-      }
-    } // if (db_type()->versioned())
   } // if (system_period == NULL)
 
   delete handler_file;

storage/innobase/handler/handler0alter.cc

@@ -588,6 +588,15 @@ ha_innobase::check_if_supported_inplace_alter(
 		DBUG_RETURN(HA_ALTER_INPLACE_NOT_SUPPORTED);
 	}
 
+	// It looks like it's actually possible to do this INPLACE, but we
+	// disallow this for now.
+	if (ha_alter_info->create_info->vers_info
+			.declared_with_system_versioning ||
+		ha_alter_info->create_info->vers_info
+			.declared_without_system_versioning) {
+		DBUG_RETURN(HA_ALTER_INPLACE_NOT_SUPPORTED);
+	}
+
 	update_thd();
 	trx_assert_no_search_latch(m_prebuilt->trx);
 
@@ -4625,6 +4634,15 @@ prepare_inplace_alter_table_dict(
 				field_type |= DATA_UNSIGNED;
 			}
 
+			if (altered_table->versioned()) {
+				if (i == altered_table->s->row_start_field) {
+					field_type |= DATA_VERS_ROW_START;
+				} else if (i ==
+					   altered_table->s->row_end_field) {
+					field_type |= DATA_VERS_ROW_END;
+				}
+			}
+
 			if (dtype_is_string_type(col_type)) {
 				charset_no = (ulint) field->charset()->number;