MDEV-14798 Add, drop system versioning semantic and syntax

SQL: DROP PERIOD FOR SYSTEM_TIME syntax

and remove ER_VERS_SYS_FIELD_EXISTS

originally by: Eugene Kosov

8 files changed, 118 insertions, 29 deletions

diff --git a/mysql-test/suite/versioning/r/alter.result b/mysql-test/suite/versioning/r/alter.result
index 4057f410856..8693f24591c 100644
--- a/mysql-test/suite/versioning/r/alter.result
+++ b/mysql-test/suite/versioning/r/alter.result
@@ -65,7 +65,7 @@ t	CREATE TABLE `t` (
   PERIOD FOR SYSTEM_TIME (`trx_start`, `trx_end`)
 ) ENGINE=MyISAM DEFAULT CHARSET=latin1 WITH SYSTEM VERSIONING
 alter table t drop system versioning;
-ERROR HY000: System versioning field `trx_start` exists
+ERROR HY000: Wrong parameters for `t`: missing 'DROP COLUMN `trx_start`'
 alter table t drop column trx_start, drop column trx_end;
 select row_start from t;
 row_start
@@ -477,7 +477,44 @@ set statement system_versioning_alter_history=keep for
 alter table t1 add column row_start timestamp(6);
 ERROR 42S21: Duplicate column name 'row_start'
 # MDEV-14798 Add, drop system versioning semantic and syntax
-create or replace table t (a int) with system versioning;
+create or replace table t (
+a int,
+row_start timestamp(6) generated always as row start,
+row_end timestamp(6) generated always as row end,
+period for system_time(row_start, row_end)
+) with system versioning;
+show create table t;
+Table	Create Table
+t	CREATE TABLE `t` (
+  `a` int(11) DEFAULT NULL,
+  `row_start` timestamp(6) GENERATED ALWAYS AS ROW START,
+  `row_end` timestamp(6) GENERATED ALWAYS AS ROW END,
+  PERIOD FOR SYSTEM_TIME (`row_start`, `row_end`)
+) ENGINE=MyISAM DEFAULT CHARSET=latin1 WITH SYSTEM VERSIONING
+alter table t
+drop column row_start,
+drop column row_end,
+drop period for system_time,
+drop system versioning;
+show create table t;
+Table	Create Table
+t	CREATE TABLE `t` (
+  `a` int(11) DEFAULT NULL
+) ENGINE=MyISAM DEFAULT CHARSET=latin1
+alter table t drop period for system_time;
+ERROR HY000: Table `t` is not system-versioned
+create or replace table t (
+a int,
+row_start timestamp(6) generated always as row start,
+row_end timestamp(6) generated always as row end,
+period for system_time(row_start, row_end)
+) with system versioning;
+alter table t drop period for system_time;
+ERROR HY000: Wrong parameters for `t`: missing 'DROP COLUMN `row_start`, DROP COLUMN `row_end`'
+alter table t drop column sys_trx_start, drop period for system_time;
+ERROR HY000: Wrong parameters for `t`: missing 'DROP COLUMN `row_start`, DROP COLUMN `row_end`'
+alter table t drop column sys_trx_end, drop period for system_time;
+ERROR HY000: Wrong parameters for `t`: missing 'DROP COLUMN `row_start`, DROP COLUMN `row_end`'
 alter table t add period for system_time(sys_trx_start, sys_trx_end);
 ERROR HY000: Table `t` is already system-versioned
 drop database test;
diff --git a/mysql-test/suite/versioning/t/alter.test b/mysql-test/suite/versioning/t/alter.test
index d0fef1e8fcd..128397b6cc8 100644
--- a/mysql-test/suite/versioning/t/alter.test
+++ b/mysql-test/suite/versioning/t/alter.test
@@ -53,7 +53,7 @@ alter table t
   add system versioning;
 show create table t;
 
---error ER_VERS_SYS_FIELD_EXISTS
+--error ER_MISSING
 alter table t drop system versioning;
 
 alter table t drop column trx_start, drop column trx_end;
@@ -405,7 +405,36 @@ set statement system_versioning_alter_history=keep for
 alter table t1 add column row_start timestamp(6);
 
 --echo # MDEV-14798 Add, drop system versioning semantic and syntax
-create or replace table t (a int) with system versioning;
+create or replace table t (
+  a int,
+  row_start timestamp(6) generated always as row start,
+  row_end timestamp(6) generated always as row end,
+  period for system_time(row_start, row_end)
+) with system versioning;
+show create table t;
+
+alter table t
+  drop column row_start,
+  drop column row_end,
+  drop period for system_time,
+  drop system versioning;
+show create table t;
+
+--error ER_VERS_NOT_VERSIONED
+alter table t drop period for system_time;
+
+create or replace table t (
+  a int,
+  row_start timestamp(6) generated always as row start,
+  row_end timestamp(6) generated always as row end,
+  period for system_time(row_start, row_end)
+) with system versioning;
+--error ER_MISSING
+alter table t drop period for system_time;
+--error ER_MISSING
+alter table t drop column sys_trx_start, drop period for system_time;
+--error ER_MISSING
+alter table t drop column sys_trx_end, drop period for system_time;
 
 --error ER_VERS_ALREADY_VERSIONED
 alter table t add period for system_time(sys_trx_start, sys_trx_end);
diff --git a/sql/handler.cc b/sql/handler.cc
index 06abd149aa5..33342e4f5f3 100644
--- a/sql/handler.cc
+++ b/sql/handler.cc
@@ -7183,23 +7183,21 @@ bool Vers_parse_info::fix_alter_info(THD *thd, Alter_info *alter_info,
     }
   }
 
-  if ((versioned_fields || unversioned_fields) && !share->versioned)
+  if ((alter_info->flags & Alter_info::ALTER_DROP_PERIOD ||
+       versioned_fields || unversioned_fields) && !share->versioned)
   {
     my_error(ER_VERS_NOT_VERSIONED, MYF(0), table_name);
     return true;
   }
 
-  if (add_period)
+  if (share->versioned)
   {
-    if (share->versioned)
+    if (alter_info->flags & Alter_info::ALTER_ADD_PERIOD)
     {
       my_error(ER_VERS_ALREADY_VERSIONED, MYF(0), table_name);
       return true;
     }
-  }
 
-  if (share->versioned)
-  {
     // copy info from existing table
     create_info->options|= HA_VERSIONED_TABLE;
 
@@ -7305,7 +7303,9 @@ Vers_parse_info::fix_create_like(Alter_info &alter_info, HA_CREATE_INFO &create_
 
 bool Vers_parse_info::need_check(const Alter_info *alter_info) const
 {
-  return versioned_fields || unversioned_fields || add_period ||
+  return versioned_fields || unversioned_fields ||
+         alter_info->flags & Alter_info::ALTER_ADD_PERIOD ||
+         alter_info->flags & Alter_info::ALTER_DROP_PERIOD ||
          alter_info->flags & Alter_info::ALTER_ADD_SYSTEM_VERSIONING ||
          alter_info->flags & Alter_info::ALTER_DROP_SYSTEM_VERSIONING || *this;
 }
diff --git a/sql/handler.h b/sql/handler.h
index a96e98c2f84..4bb57ecd7dd 100644
--- a/sql/handler.h
+++ b/sql/handler.h
@@ -1726,8 +1726,7 @@ struct Vers_parse_info
 {
   Vers_parse_info() :
     versioned_fields(false),
-    unversioned_fields(false),
-    add_period(false)
+    unversioned_fields(false)
   {}
 
   struct start_end_t
@@ -1791,7 +1790,6 @@ public:
   */
   bool versioned_fields : 1;
   bool unversioned_fields : 1;
-  bool add_period : 1;  // ADD PERIOD FOR SYSTEM_TIME was specified
 };
 
 /**
diff --git a/sql/share/errmsg-utf8.txt b/sql/share/errmsg-utf8.txt
index c860b580a26..f34d707e096 100644
--- a/sql/share/errmsg-utf8.txt
+++ b/sql/share/errmsg-utf8.txt
@@ -7882,8 +7882,8 @@ ER_VERS_NO_TRX_ID
 ER_VERS_ALTER_SYSTEM_FIELD
         eng "Can not change system versioning field %`s"
 
-ER_VERS_SYS_FIELD_EXISTS
-        eng "System versioning field %`s exists"
+ER_UNUSED_24
+        eng "You should never see it"
 
 ER_NOT_LOG_TABLE
         eng "Table %`s.%`s is not a log table"
diff --git a/sql/sql_alter.h b/sql/sql_alter.h
index c7cc98aacc4..4314d662d3d 100644
--- a/sql/sql_alter.h
+++ b/sql/sql_alter.h
@@ -98,6 +98,8 @@ public:
   static const ulonglong ALTER_COLUMN_UNVERSIONED    = 1ULL << 30;
   static const ulonglong ALTER_ADD_SYSTEM_VERSIONING = 1ULL << 31;
   static const ulonglong ALTER_DROP_SYSTEM_VERSIONING= 1ULL << 32;
+  static const ulonglong ALTER_ADD_PERIOD            = 1ULL << 33;
+  static const ulonglong ALTER_DROP_PERIOD           = 1ULL << 34;
 
   enum enum_enable_or_disable { LEAVE_AS_IS, ENABLE, DISABLE };
 
diff --git a/sql/sql_table.cc b/sql/sql_table.cc
index 64359a585e3..e20208012d5 100644
--- a/sql/sql_table.cc
+++ b/sql/sql_table.cc
@@ -7806,6 +7806,19 @@ blob_length_by_type(enum_field_types type)
 }
 
 
+static void append_drop_column(THD *thd, bool dont, String *str,
+                               Field *field)
+{
+  if (!dont)
+  {
+    if (str->length())
+      str->append(STRING_WITH_LEN(", "));
+    str->append(STRING_WITH_LEN("DROP COLUMN "));
+    append_identifier(thd, str, &field->field_name);
+  }
+}
+
+
 /**
   Prepare column and key definitions for CREATE TABLE in ALTER TABLE.
 
@@ -7976,6 +7989,7 @@ mysql_prepare_alter_table(THD *thd, TABLE *table,
       }
       if (table->s->tmp_table == NO_TMP_TABLE)
         (void) delete_statistics_for_column(thd, table, field);
+      dropped_sys_vers_fields|= field->flags;
       drop_it.remove();
       dropped_fields= &table->tmp_set;
       bitmap_set_bit(dropped_fields, field->field_index);
@@ -8048,7 +8062,9 @@ mysql_prepare_alter_table(THD *thd, TABLE *table,
              field->flags & VERS_SYSTEM_FIELD &&
              field->invisible < INVISIBLE_SYSTEM)
     {
-      my_error(ER_VERS_SYS_FIELD_EXISTS, MYF(0), field->field_name.str);
+      StringBuffer<NAME_LEN*3> tmp;
+      append_drop_column(thd, false, &tmp, field);
+      my_error(ER_MISSING, MYF(0), table->s->table_name.str, tmp.c_ptr());
       goto err;
     }
     else if (drop && field->invisible < INVISIBLE_SYSTEM &&
@@ -8058,13 +8074,13 @@ mysql_prepare_alter_table(THD *thd, TABLE *table,
       /* "dropping" a versioning field only hides it from the user */
       def= new (thd->mem_root) Create_field(thd, field, field);
       def->invisible= INVISIBLE_SYSTEM;
-      dropped_sys_vers_fields|= field->flags;
       alter_info->flags|= Alter_info::ALTER_CHANGE_COLUMN;
       if (field->flags & VERS_SYS_START_FLAG)
         create_info->vers_info.as_row.start= def->field_name= Vers_parse_info::default_start;
       else
         create_info->vers_info.as_row.end= def->field_name= Vers_parse_info::default_end;
       new_create_list.push_back(def, thd->mem_root);
+      dropped_sys_vers_fields|= field->flags;
       drop_it.remove();
     }
     else
@@ -8093,18 +8109,21 @@ mysql_prepare_alter_table(THD *thd, TABLE *table,
       }
     }
   }
-  if (dropped_sys_vers_fields &&
-      ((dropped_sys_vers_fields & VERS_SYSTEM_FIELD) != VERS_SYSTEM_FIELD))
+  dropped_sys_vers_fields &= VERS_SYSTEM_FIELD;
+  if ((dropped_sys_vers_fields ||
+       alter_info->flags & Alter_info::ALTER_DROP_PERIOD) &&
+      dropped_sys_vers_fields != VERS_SYSTEM_FIELD)
   {
-    StringBuffer<NAME_LEN*2> tmp;
-    tmp.append(STRING_WITH_LEN("DROP COLUMN "));
-    if (dropped_sys_vers_fields & VERS_SYS_START_FLAG)
-      append_identifier(thd, &tmp, &table->vers_end_field()->field_name);
-    else
-      append_identifier(thd, &tmp, &table->vers_start_field()->field_name);
+    StringBuffer<NAME_LEN*3> tmp;
+    append_drop_column(thd, dropped_sys_vers_fields & VERS_SYS_START_FLAG,
+                       &tmp, table->vers_start_field());
+    append_drop_column(thd, dropped_sys_vers_fields & VERS_SYS_END_FLAG,
+                       &tmp, table->vers_end_field());
     my_error(ER_MISSING, MYF(0), table->s->table_name.str, tmp.c_ptr());
     goto err;
   }
+  alter_info->flags &=
+    ~(Alter_info::ALTER_DROP_PERIOD | Alter_info::ALTER_ADD_PERIOD);
   def_it.rewind();
   while ((def=def_it++))			// Add new columns
   {
diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy
index c773c8da506..523b01a61ec 100644
--- a/sql/sql_yacc.yy
+++ b/sql/sql_yacc.yy
@@ -904,10 +904,10 @@ bool my_yyoverflow(short **a, YYSTYPE **b, size_t *yystacksize);
 %parse-param { THD *thd }
 %lex-param { THD *thd }
 /*
-  Currently there are 123 shift/reduce conflicts.
+  Currently there are 125 shift/reduce conflicts.
   We should not introduce new conflicts any more.
 */
-%expect 123
+%expect 125
 
 /*
    Comments for TOKENS.
@@ -8121,7 +8121,7 @@ alter_list_item:
           }
         | ADD period_for_system_time
           {
-            Lex->vers_get_info().add_period= true;
+            Lex->alter_info.flags|= Alter_info::ALTER_ADD_PERIOD;
           }
         | add_column '(' create_field_list ')'
           {
@@ -8287,6 +8287,10 @@ alter_list_item:
           {
             Lex->alter_info.flags|= Alter_info::ALTER_DROP_SYSTEM_VERSIONING;
           }
+        | DROP PERIOD_SYM FOR_SYSTEM_TIME_SYM
+          {
+            Lex->alter_info.flags|= Alter_info::ALTER_DROP_PERIOD;
+          }
         ;
 
 opt_index_lock_algorithm: