From 0c571f8c4ee6235327253d5eba58a63f03f12627 Mon Sep 17 00:00:00 2001
From: Aleksey Midenkov <midenok@gmail.com>
Date: Fri, 24 Nov 2017 19:15:10 +0300
Subject: [PATCH] SQL: versioning_alter_history ERROR mode [closes #350]

Disabled DROP and SURVIVE modes.
---
 mysql-test/r/mysqlcheck.result                |  6 +-
 mysql-test/r/mysqld--help.result              |  7 +--
 .../sys_vars/r/sysvars_server_embedded.result | 10 ++--
 .../r/sysvars_server_notembedded.result       | 10 ++--
 mysql-test/suite/versioning/r/alter.result    | 60 ++++---------------
 mysql-test/suite/versioning/t/alter.test      | 23 ++++++-
 sql/mysqld.h                                  |  3 +-
 sql/share/errmsg-utf8.txt                     |  4 +-
 sql/sql_alter.h                               | 13 ++--
 sql/sql_table.cc                              | 30 +++++++---
 sql/sys_vars.cc                               | 11 ++--
 11 files changed, 89 insertions(+), 88 deletions(-)

diff --git a/mysql-test/r/mysqlcheck.result b/mysql-test/r/mysqlcheck.result
index d1785626fc5..c6ce2b465da 100644
--- a/mysql-test/r/mysqlcheck.result
+++ b/mysql-test/r/mysqlcheck.result
@@ -73,7 +73,8 @@ status   : OK
 mysql.user                                         OK
 mysql.vtmd_template
 note     : Table does not support optimize, doing recreate + analyze instead
-status   : OK
+error    : Not allowed for versioned `mysql.vtmd_template`. Change `versioning_alter_history` to proceed with ALTER.
+status   : Operation failed
 mysql.column_stats                                 OK
 mysql.columns_priv                                 OK
 mysql.db                                           OK
@@ -141,7 +142,8 @@ status   : OK
 mysql.user                                         Table is already up to date
 mysql.vtmd_template
 note     : Table does not support optimize, doing recreate + analyze instead
-status   : OK
+error    : Not allowed for versioned `mysql.vtmd_template`. Change `versioning_alter_history` to proceed with ALTER.
+status   : Operation failed
 create table t1 (a int) engine=myisam;
 create view v1 as select * from t1;
 test.t1                                            OK
diff --git a/mysql-test/r/mysqld--help.result b/mysql-test/r/mysqld--help.result
index 145193338fd..e31571e93a8 100644
--- a/mysql-test/r/mysqld--help.result
+++ b/mysql-test/r/mysqld--help.result
@@ -1293,10 +1293,9 @@ The following options may be given as the first argument:
  -V, --version[=name] 
  Output version information and exit.
  --versioning-alter-history=name 
- Versioning ALTER TABLE mode. KEEP: leave historical
- system rows as is on ALTER TABLE; SURVIVE: use DDL
- survival feature; DROP: delete historical system rows on
- ALTER TABLE
+ Versioning ALTER TABLE mode. ERROR: Fail ALTER with
+ error; KEEP: Keep historical system rows and subject them
+ to ALTER; 
  --versioning-force  Force system versioning for all created tables
  --versioning-hide=name 
  Hide system versioning from being displayed in table
diff --git a/mysql-test/suite/sys_vars/r/sysvars_server_embedded.result b/mysql-test/suite/sys_vars/r/sysvars_server_embedded.result
index ba3e2900624..385ecb4e4e6 100644
--- a/mysql-test/suite/sys_vars/r/sysvars_server_embedded.result
+++ b/mysql-test/suite/sys_vars/r/sysvars_server_embedded.result
@@ -4463,17 +4463,17 @@ ENUM_VALUE_LIST	NEVER,COMPLEMENTARY,PREFERABLY
 READ_ONLY	NO
 COMMAND_LINE_ARGUMENT	REQUIRED
 VARIABLE_NAME	VERSIONING_ALTER_HISTORY
-SESSION_VALUE	KEEP
-GLOBAL_VALUE	KEEP
+SESSION_VALUE	ERROR
+GLOBAL_VALUE	ERROR
 GLOBAL_VALUE_ORIGIN	COMPILE-TIME
-DEFAULT_VALUE	KEEP
+DEFAULT_VALUE	ERROR
 VARIABLE_SCOPE	SESSION
 VARIABLE_TYPE	ENUM
-VARIABLE_COMMENT	Versioning ALTER TABLE mode. KEEP: leave historical system rows as is on ALTER TABLE; SURVIVE: use DDL survival feature; DROP: delete historical system rows on ALTER TABLE
+VARIABLE_COMMENT	Versioning ALTER TABLE mode. ERROR: Fail ALTER with error; KEEP: Keep historical system rows and subject them to ALTER; 
 NUMERIC_MIN_VALUE	NULL
 NUMERIC_MAX_VALUE	NULL
 NUMERIC_BLOCK_SIZE	NULL
-ENUM_VALUE_LIST	KEEP,SURVIVE,DROP
+ENUM_VALUE_LIST	ERROR,KEEP
 READ_ONLY	NO
 COMMAND_LINE_ARGUMENT	REQUIRED
 VARIABLE_NAME	VERSIONING_ASOF_TIMESTAMP
diff --git a/mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result b/mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result
index 95f73c4e89a..0e6e87619e7 100644
--- a/mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result
+++ b/mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result
@@ -5359,17 +5359,17 @@ ENUM_VALUE_LIST	NEVER,COMPLEMENTARY,PREFERABLY
 READ_ONLY	NO
 COMMAND_LINE_ARGUMENT	REQUIRED
 VARIABLE_NAME	VERSIONING_ALTER_HISTORY
-SESSION_VALUE	KEEP
-GLOBAL_VALUE	KEEP
+SESSION_VALUE	ERROR
+GLOBAL_VALUE	ERROR
 GLOBAL_VALUE_ORIGIN	COMPILE-TIME
-DEFAULT_VALUE	KEEP
+DEFAULT_VALUE	ERROR
 VARIABLE_SCOPE	SESSION
 VARIABLE_TYPE	ENUM
-VARIABLE_COMMENT	Versioning ALTER TABLE mode. KEEP: leave historical system rows as is on ALTER TABLE; SURVIVE: use DDL survival feature; DROP: delete historical system rows on ALTER TABLE
+VARIABLE_COMMENT	Versioning ALTER TABLE mode. ERROR: Fail ALTER with error; KEEP: Keep historical system rows and subject them to ALTER; 
 NUMERIC_MIN_VALUE	NULL
 NUMERIC_MAX_VALUE	NULL
 NUMERIC_BLOCK_SIZE	NULL
-ENUM_VALUE_LIST	KEEP,SURVIVE,DROP
+ENUM_VALUE_LIST	ERROR,KEEP
 READ_ONLY	NO
 COMMAND_LINE_ARGUMENT	REQUIRED
 VARIABLE_NAME	VERSIONING_ASOF_TIMESTAMP
diff --git a/mysql-test/suite/versioning/r/alter.result b/mysql-test/suite/versioning/r/alter.result
index fdceb98ce99..0a3fb3d43e1 100644
--- a/mysql-test/suite/versioning/r/alter.result
+++ b/mysql-test/suite/versioning/r/alter.result
@@ -1,3 +1,6 @@
+select @@versioning_alter_history;
+@@versioning_alter_history
+ERROR
 create table t(
 a int
 );
@@ -17,12 +20,15 @@ t	CREATE TABLE `t` (
   `sys_trx_end` timestamp(6) GENERATED ALWAYS AS ROW END,
   PERIOD FOR SYSTEM_TIME (`sys_trx_start`, `sys_trx_end`)
 ) ENGINE=MyISAM DEFAULT CHARSET=latin1 WITH SYSTEM VERSIONING
+alter table t add column y int;
+ERROR HY000: Not allowed for versioned `test.t`. Change `versioning_alter_history` to proceed with ALTER.
 alter table t drop system versioning;
 show create table t;
 Table	Create Table
 t	CREATE TABLE `t` (
   `a` int(11) DEFAULT NULL
 ) ENGINE=MyISAM DEFAULT CHARSET=latin1
+set versioning_alter_history= keep;
 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,
@@ -587,51 +593,9 @@ Table	Create Table
 t	CREATE TABLE `t` (
   `a` int(11) DEFAULT NULL
 ) ENGINE=MyISAM DEFAULT CHARSET=latin1
-set versioning_alter_history = DROP;
-create or replace table t (a int) with system versioning engine innodb;
-insert into t values (1);
-update t set a = 2;
-select * from t for system_time all;
-a
-2
-1
-alter table t add column b int;
-select * from t for system_time all;
-a	b
-2	NULL
-create or replace table t (a int) with system versioning engine myisam;
-insert into t values (1);
-update t set a = 2;
-select * from t for system_time all;
-a
-2
-1
-alter table t add column b int;
-select * from t for system_time all;
-a	b
-2	NULL
-create or replace table non_empty (
-a int,
-sys_trx_start bigint(20) unsigned,
-sys_trx_end bigint(20) unsigned
-) engine innodb;
-insert into non_empty values (1, 100, 200);
-alter table non_empty
-change column sys_trx_start sys_trx_start bigint(20) unsigned generated always as row start;
-ERROR HY000: Can not modify column `sys_trx_start` to GENERATED ALWAYS AS ROW START/END for non-empty table
-alter table non_empty
-change column sys_trx_end sys_trx_end bigint(20) unsigned generated always as row end;
-ERROR HY000: Can not modify column `sys_trx_end` to GENERATED ALWAYS AS ROW START/END for non-empty table
-drop table non_empty;
-call verify_vtq;
-No	A	B	C	D
-1	1	1	1	1
-2	1	1	1	1
-drop table t;
-drop procedure verify_vtq;
-drop procedure innodb_verify_vtq;
-drop function default_engine;
-drop function sys_commit_ts;
-drop function sys_datatype;
-drop procedure concat_exec2;
-drop procedure concat_exec3;
+set versioning_alter_history= SURVIVE;
+ERROR 42000: Variable 'versioning_alter_history' can't be set to the value of 'SURVIVE'
+set versioning_alter_history= DROP;
+ERROR 42000: Variable 'versioning_alter_history' can't be set to the value of 'DROP'
+drop database test;
+create database test;
diff --git a/mysql-test/suite/versioning/t/alter.test b/mysql-test/suite/versioning/t/alter.test
index 817549168bc..71b40c73a2f 100644
--- a/mysql-test/suite/versioning/t/alter.test
+++ b/mysql-test/suite/versioning/t/alter.test
@@ -1,3 +1,5 @@
+select @@versioning_alter_history;
+
 create table t(
   a int
 );
@@ -8,9 +10,14 @@ alter table t drop system versioning;
 alter table t add system versioning;
 show create table t;
 
+--error ER_VERS_ALTER_NOT_ALLOWED
+alter table t add column y int;
+
 alter table t drop system versioning;
 show create table t;
 
+set versioning_alter_history= keep;
+
 --error ER_VERS_FIELD_WRONG_TYPE
 alter table t
   add column trx_start bigint(20) unsigned generated always as row start,
@@ -273,8 +280,17 @@ select * from t;
 alter table t drop system versioning;
 show create table t;
 
-set versioning_alter_history = DROP;
 
+## These experimental options are now disabled
+
+--error ER_WRONG_VALUE_FOR_VAR
+set versioning_alter_history= SURVIVE;
+
+--error ER_WRONG_VALUE_FOR_VAR
+set versioning_alter_history= DROP;
+
+if (0)
+{
 create or replace table t (a int) with system versioning engine innodb;
 insert into t values (1);
 update t set a = 2;
@@ -305,6 +321,7 @@ alter table non_empty
 drop table non_empty;
 
 call verify_vtq;
-drop table t;
+}
 
--- source suite/versioning/common_finish.inc
+drop database test;
+create database test;
diff --git a/sql/mysqld.h b/sql/mysqld.h
index 0e42788acad..4aaadcc618e 100644
--- a/sql/mysqld.h
+++ b/sql/mysqld.h
@@ -210,7 +210,8 @@ enum vers_hide_enum
 
 enum vers_alter_history_enum
 {
-  VERS_ALTER_HISTORY_KEEP= 0,
+  VERS_ALTER_HISTORY_ERROR= 0,
+  VERS_ALTER_HISTORY_KEEP,
   VERS_ALTER_HISTORY_SURVIVE,
   VERS_ALTER_HISTORY_DROP
 };
diff --git a/sql/share/errmsg-utf8.txt b/sql/share/errmsg-utf8.txt
index 33dd0b9d0c6..fb30fe89c95 100644
--- a/sql/share/errmsg-utf8.txt
+++ b/sql/share/errmsg-utf8.txt
@@ -7845,8 +7845,8 @@ WARN_VERS_TRX_MISSING
 WARN_VERS_PART_NON_HISTORICAL
         eng "Partition %`s contains non-historical data"
 
-ER_VERS_NOT_ALLOWED
-        eng "%`s is not allowed for versioned table"
+ER_VERS_ALTER_NOT_ALLOWED
+        eng "Not allowed for versioned `%s.%s`. Change `versioning_alter_history` to proceed with ALTER."
 
 ER_VERS_RANGE_PROHIBITED
         eng "SYSTEM_TIME range selector is prohibited"
diff --git a/sql/sql_alter.h b/sql/sql_alter.h
index 39458c45b80..5c651b0c91b 100644
--- a/sql/sql_alter.h
+++ b/sql/sql_alter.h
@@ -100,20 +100,25 @@ public:
 
   enum enum_enable_or_disable { LEAVE_AS_IS, ENABLE, DISABLE };
 
-  bool vers_data_modifying() const
+  bool data_modifying() const
   {
     return flags & (
       ALTER_ADD_COLUMN |
       ALTER_DROP_COLUMN |
       ALTER_CHANGE_COLUMN |
+      ALTER_COLUMN_ORDER);
+  }
+
+  bool partition_modifying() const
+  {
+    return flags & (
       ALTER_DROP_PARTITION |
       ALTER_COALESCE_PARTITION |
       ALTER_REORGANIZE_PARTITION |
-      ALTER_TABLE_REORG |
       ALTER_REMOVE_PARTITIONING |
+      ALTER_TABLE_REORG |
       ALTER_EXCHANGE_PARTITION |
-      ALTER_TRUNCATE_PARTITION |
-      ALTER_COLUMN_ORDER);
+      ALTER_TRUNCATE_PARTITION);
   }
 
   /**
diff --git a/sql/sql_table.cc b/sql/sql_table.cc
index 300a75ffe77..b04c9acbc38 100644
--- a/sql/sql_table.cc
+++ b/sql/sql_table.cc
@@ -8843,11 +8843,23 @@ bool mysql_alter_table(THD *thd, const char *new_db, const char *new_name,
                           &alter_prelocking_strategy);
   thd->open_options&= ~HA_OPEN_FOR_ALTER;
   bool versioned= table_list->table && table_list->table->versioned();
-  bool vers_data_mod= versioned &&
-    thd->variables.vers_alter_history == VERS_ALTER_HISTORY_SURVIVE &&
-    alter_info->vers_data_modifying();
+  bool vers_survival_mod= false;
 
-  if (vers_data_mod)
+  if (versioned)
+  {
+    bool vers_data_mod= alter_info->data_modifying();
+    if (thd->variables.vers_alter_history == VERS_ALTER_HISTORY_SURVIVE)
+    {
+      vers_survival_mod= alter_info->data_modifying() || alter_info->partition_modifying();
+    }
+    else if (vers_data_mod && thd->variables.vers_alter_history == VERS_ALTER_HISTORY_ERROR)
+    {
+      my_error(ER_VERS_ALTER_NOT_ALLOWED, MYF(0), table_list->db, table_list->table_name);
+      DBUG_RETURN(true);
+    }
+  }
+
+  if (vers_survival_mod)
   {
     table_list->set_lock_type(thd, TL_WRITE);
     if (thd->mdl_context.upgrade_shared_lock(table_list->table->mdl_ticket,
@@ -9178,7 +9190,7 @@ bool mysql_alter_table(THD *thd, const char *new_db, const char *new_name,
       Upgrade from MDL_SHARED_UPGRADABLE to MDL_SHARED_NO_WRITE.
       Afterwards it's safe to take the table level lock.
     */
-    if ((!vers_data_mod &&
+    if ((!vers_survival_mod &&
          thd->mdl_context.upgrade_shared_lock(
              mdl_ticket, MDL_SHARED_NO_WRITE,
              thd->variables.lock_wait_timeout)) ||
@@ -9609,7 +9621,7 @@ bool mysql_alter_table(THD *thd, const char *new_db, const char *new_name,
                                  alter_info->keys_onoff,
                                  &alter_ctx))
     {
-      if (vers_data_mod && new_versioned && table->versioned_by_sql())
+      if (vers_survival_mod && new_versioned && table->versioned_by_sql())
       {
         // Failure of this function may result in corruption of an original table.
         vers_reset_alter_copy(thd, table);
@@ -9711,7 +9723,7 @@ bool mysql_alter_table(THD *thd, const char *new_db, const char *new_name,
     anything goes wrong while renaming the new table.
   */
   char backup_name[FN_LEN];
-  if (vers_data_mod)
+  if (vers_survival_mod)
     VTMD_table::archive_name(thd, alter_ctx.table_name, backup_name,
                          sizeof(backup_name));
   else
@@ -9745,7 +9757,7 @@ bool mysql_alter_table(THD *thd, const char *new_db, const char *new_name,
     goto err_with_mdl;
   }
 
-  if (vers_data_mod && new_versioned)
+  if (vers_survival_mod && new_versioned)
   {
     DBUG_ASSERT(alter_info && table_list);
     VTMD_rename vtmd(*table_list);
@@ -9781,7 +9793,7 @@ err_after_rename:
   }
 
   // ALTER TABLE succeeded, delete the backup of the old table.
-  if (!(vers_data_mod && new_versioned) &&
+  if (!(vers_survival_mod && new_versioned) &&
       quick_rm_table(thd, old_db_type, alter_ctx.db, backup_name, FN_IS_TMP))
   {
     /*
diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc
index fd5409ab26d..8afd07bd97a 100644
--- a/sql/sys_vars.cc
+++ b/sql/sys_vars.cc
@@ -413,14 +413,15 @@ static Sys_var_mybool Sys_vers_innodb_algorithm_simple(
        SESSION_VAR(vers_innodb_algorithm_simple), CMD_LINE(OPT_ARG),
        DEFAULT(TRUE));
 
-static const char *vers_alter_history_keywords[]= {"KEEP", "SURVIVE", "DROP", NULL};
+static const char *vers_alter_history_keywords[]= {"ERROR", "KEEP",/* "SURVIVE", "DROP",*/ NULL};
 static Sys_var_enum Sys_vers_alter_history(
        "versioning_alter_history", "Versioning ALTER TABLE mode. "
-       "KEEP: leave historical system rows as is on ALTER TABLE; "
-       "SURVIVE: use DDL survival feature; "
-       "DROP: delete historical system rows on ALTER TABLE",
+       "ERROR: Fail ALTER with error; " /* TODO: fail only when history non-empty */
+       "KEEP: Keep historical system rows and subject them to ALTER; "
+       /*"SURVIVE: Keep historical system rows intact; "
+       "DROP: Drop historical system rows while processing ALTER"*/,
        SESSION_VAR(vers_alter_history), CMD_LINE(REQUIRED_ARG),
-       vers_alter_history_keywords, DEFAULT(VERS_ALTER_HISTORY_KEEP));
+       vers_alter_history_keywords, DEFAULT(VERS_ALTER_HISTORY_ERROR));
 
 static Sys_var_mybool Sys_transaction_registry(
        "transaction_registry",