SQL: remove versioning only when hidden system fields

[closes #211]

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

@@ -57,6 +57,7 @@ t	CREATE TABLE `t` (
   `trx_end` timestamp(6) GENERATED ALWAYS AS ROW END,
   PERIOD FOR SYSTEM_TIME (`trx_start`, `trx_end`)
 ) ENGINE=MyISAM DEFAULT CHARSET=latin1 WITH SYSTEM VERSIONING
+alter table t drop column trx_start, drop column trx_end;
 alter table t without system versioning;
 show create table t;
 Table	Create Table
@@ -338,6 +339,7 @@ t	CREATE TABLE `t` (
   `trx_end` bigint(20) unsigned GENERATED ALWAYS AS ROW END,
   PERIOD FOR SYSTEM_TIME (`trx_start`, `trx_end`)
 ) ENGINE=InnoDB DEFAULT CHARSET=latin1 WITH SYSTEM VERSIONING
+alter table t drop column trx_start, drop column trx_end;
 alter table t without system versioning;
 alter table t with system versioning, algorithm=copy;
 show create table t;
@@ -511,6 +513,30 @@ period for system_time(sys_trx_start, sys_trx_end)
 ) with system versioning engine myisam;
 alter table t change column sys_trx_start asdf timestamp(6);
 ERROR HY000: Can not change system versioning field 'sys_trx_start'
+create or replace table t (
+a int,
+sys_trx_start timestamp(6) generated always as row start,
+sys_trx_end timestamp(6) generated always as row end,
+period for system_time(sys_trx_start, sys_trx_end)
+) with system versioning;
+select * from t;
+a	sys_trx_start	sys_trx_end
+alter table t without system versioning;
+ERROR HY000: System versioning field 'sys_trx_start' is not hidden
+alter table t drop column sys_trx_start;
+select * from t;
+a	sys_trx_end
+alter table t without system versioning;
+ERROR HY000: System versioning field 'sys_trx_end' is not hidden
+alter table t drop column sys_trx_end;
+select * from t;
+a
+alter table t without system versioning;
+show create table t;
+Table	Create Table
+t	CREATE TABLE `t` (
+  `a` int(11) DEFAULT NULL
+) ENGINE=MyISAM DEFAULT CHARSET=latin1
 call verify_vtq;
 No	A	B	C	D
 1	1	1	1	1

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

@@ -41,6 +41,7 @@ alter table t
   with system versioning;
 show create table t;
 
+alter table t drop column trx_start, drop column trx_end;
 alter table t without system versioning;
 show create table t;
 
@@ -150,6 +151,7 @@ alter table t
   add period for system_time(trx_start, trx_end),
   with system versioning;
 show create table t;
+alter table t drop column trx_start, drop column trx_end;
 alter table t without system versioning;
 
 alter table t with system versioning, algorithm=copy;
@@ -238,6 +240,27 @@ create or replace table t (
 --error ER_VERS_ALTER_SYSTEM_FIELD
 alter table t change column sys_trx_start asdf timestamp(6);
 
+create or replace table t (
+  a int,
+  sys_trx_start timestamp(6) generated always as row start,
+  sys_trx_end timestamp(6) generated always as row end,
+  period for system_time(sys_trx_start, sys_trx_end)
+) with system versioning;
+select * from t;
+
+--error ER_VERS_SYS_FIELD_NOT_HIDDEN
+alter table t without system versioning;
+alter table t drop column sys_trx_start;
+select * from t;
+
+--error ER_VERS_SYS_FIELD_NOT_HIDDEN
+alter table t without system versioning;
+alter table t drop column sys_trx_end;
+select * from t;
+
+alter table t without system versioning;
+show create table t;
+
 call verify_vtq;
 drop table t;
 

sql/handler.cc

@@ -6603,15 +6603,12 @@ bool Vers_parse_info::is_trx_end(const Create_field &f) const
   return f.flags & VERS_SYS_END_FLAG;
 }
 
-
-static bool vers_create_sys_field(THD *thd, const char *field_name,
-                                 Alter_info *alter_info,
-                                 int flags,
-                                 bool integer_fields)
+static Create_field *vers_init_sys_field(THD *thd, const char *field_name,
+                                         int flags, bool integer_fields)
 {
   Create_field *f= new (thd->mem_root) Create_field();
   if (!f)
-    return true;
+    return NULL;
 
   memset(f, 0, sizeof(*f));
   f->field_name= field_name;
@@ -6630,10 +6627,38 @@ static bool vers_create_sys_field(THD *thd, const char *field_name,
   }
 
   if (f->check(thd))
+    return NULL;
+
+  return f;
+}
+
+static bool vers_create_sys_field(THD *thd, const char *field_name,
+                                  Alter_info *alter_info, int flags,
+                                  bool integer_fields)
+{
+  Create_field *f= vers_init_sys_field(thd, field_name, flags, integer_fields);
+  if (!f)
     return true;
 
-  alter_info->create_list.push_back(f);
   alter_info->flags|= Alter_info::ALTER_ADD_COLUMN;
+  alter_info->create_list.push_back(f);
+
+  return false;
+}
+
+static bool vers_change_sys_field(THD *thd, const char *field_name,
+                                  Alter_info *alter_info, int flags,
+                                  bool integer_fields, const char *change)
+{
+  Create_field *f= vers_init_sys_field(thd, field_name, flags, integer_fields);
+  if (!f)
+    return true;
+
+  f->change= change;
+
+  alter_info->flags|= Alter_info::ALTER_CHANGE_COLUMN;
+  alter_info->create_list.push_back(f);
+
   return false;
 }
 
@@ -6814,6 +6839,7 @@ static bool add_field_to_drop_list(THD *thd, Alter_info *alter_info,
 {
   DBUG_ASSERT(field);
   DBUG_ASSERT(field->field_name);
+  alter_info->flags|= Alter_info::ALTER_DROP_COLUMN;
   Alter_drop *ad= new (thd->mem_root)
       Alter_drop(Alter_drop::COLUMN, field->field_name, false);
   return !ad || alter_info->drop_list.push_back(ad, thd->mem_root);
@@ -6838,11 +6864,23 @@ bool Vers_parse_info::check_and_fix_alter(THD *thd, Alter_info *alter_info,
       return true;
     }
 
+    if (!(share->vers_start_field()->flags & HIDDEN_FLAG))
+    {
+      my_error(ER_VERS_SYS_FIELD_NOT_HIDDEN, MYF(0),
+               share->vers_start_field()->field_name);
+      return true;
+    }
+    if (!(share->vers_end_field()->flags & HIDDEN_FLAG))
+    {
+      my_error(ER_VERS_SYS_FIELD_NOT_HIDDEN, MYF(0),
+               share->vers_end_field()->field_name);
+      return true;
+    }
+
     if (add_field_to_drop_list(thd, alter_info, share->vers_start_field()) ||
         add_field_to_drop_list(thd, alter_info, share->vers_end_field()))
       return true;
 
-    alter_info->flags|= Alter_info::ALTER_DROP_COLUMN;
     return false;
   }
 
@@ -6908,14 +6946,16 @@ bool Vers_parse_info::check_and_fix_alter(THD *thd, Alter_info *alter_info,
           return true;
         }
 
-        if (vers_create_sys_field(thd, name, alter_info,
+        if (vers_change_sys_field(thd, name, alter_info,
                                   f->flags &
                                       (VERS_SYS_START_FLAG | VERS_SYS_END_FLAG),
-                                  integer_fields))
+                                  integer_fields, name))
         {
           return true;
         }
 
+        it.remove();
+
         if (done_start && done_end)
           break;
       }

sql/share/errmsg-utf8.txt

@@ -7597,3 +7597,6 @@ ER_WRONG_TABLESPACE_NAME 42000
 
 ER_VERS_ALTER_SYSTEM_FIELD
         eng "Can not change system versioning field '%s'"
+
+ER_VERS_SYS_FIELD_NOT_HIDDEN
+	eng "System versioning field '%s' is not hidden"