fix(hander): support alter talbe with charset stoneatom#1670

[summary] 1. backport code 2. fix no txn version for table not commited by transaction.
hustjieke · May 9, 2023 · b666d64 · b666d64
1 parent 56acbb8
commit b666d64
Show file tree

Hide file tree

Showing 5 changed files with 133 additions and 10 deletions.
diff --git a/mysql-test/suite/tianmu/r/issue848.result b/mysql-test/suite/tianmu/r/issue848.result
@@ -0,0 +1,70 @@
+create database test_issue848;
+use test_issue848;
+CREATE TABLE `t1` (
+`c_char` char(10) DEFAULT NULL COMMENT 'char',
+`c_varchar` varchar(10) DEFAULT NULL COMMENT 'varchar',
+`c_text` text COMMENT 'text') 
+ENGINE=TIANMU DEFAULT CHARSET=GBK;
+show create table t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `c_char` char(10) DEFAULT NULL COMMENT 'char',
+  `c_varchar` varchar(10) DEFAULT NULL COMMENT 'varchar',
+  `c_text` text COMMENT 'text'
+) ENGINE=TIANMU DEFAULT CHARSET=gbk
+insert into t1(c_char,c_varchar,c_text) values(x'D6D0B9FA',x'D5E3BDAD',x'BABCD6DD');
+ALTER TABLE t1 DEFAULT CHARACTER SET gbk;
+show create table t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `c_char` char(10) DEFAULT NULL COMMENT 'char',
+  `c_varchar` varchar(10) DEFAULT NULL COMMENT 'varchar',
+  `c_text` text COMMENT 'text'
+) ENGINE=TIANMU DEFAULT CHARSET=gbk
+ALTER TABLE t1 CHARACTER SET latin1;
+show create table t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `c_char` char(10) CHARACTER SET gbk DEFAULT NULL COMMENT 'char',
+  `c_varchar` varchar(10) CHARACTER SET gbk DEFAULT NULL COMMENT 'varchar',
+  `c_text` text CHARACTER SET gbk COMMENT 'text'
+) ENGINE=TIANMU DEFAULT CHARSET=latin1
+alter table t1  add column ex_column char(30);
+show create table t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `c_char` char(10) CHARACTER SET gbk DEFAULT NULL COMMENT 'char',
+  `c_varchar` varchar(10) CHARACTER SET gbk DEFAULT NULL COMMENT 'varchar',
+  `c_text` text CHARACTER SET gbk COMMENT 'text',
+  `ex_column` char(30) DEFAULT NULL
+) ENGINE=TIANMU DEFAULT CHARSET=latin1
+ALTER TABLE t1 MODIFY c_char char(10) CHARACTER SET UTF8MB4;
+ALTER TABLE t1 MODIFY c_varchar char(10) CHARACTER SET UTF8MB4;
+ALTER TABLE t1 MODIFY c_text char(10) CHARACTER SET UTF8MB4;
+show create table t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `c_char` char(10) CHARACTER SET utf8mb4 COLLATE utf8mb4_0900_ai_ci DEFAULT NULL,
+  `c_varchar` char(10) CHARACTER SET utf8mb4 COLLATE utf8mb4_0900_ai_ci DEFAULT NULL,
+  `c_text` char(10) CHARACTER SET utf8mb4 COLLATE utf8mb4_0900_ai_ci DEFAULT NULL,
+  `ex_column` char(30) DEFAULT NULL
+) ENGINE=TIANMU DEFAULT CHARSET=latin1
+select hex(c_char),hex(c_varchar),hex(c_text) from t1;
+hex(c_char)	hex(c_varchar)	hex(c_text)
+E4B8ADE59BBD	E6B599E6B19F	E69DADE5B79E
+ALTER TABLE t1 CHANGE c_char c_char char(10) CHARACTER SET GBK;
+ALTER TABLE t1 CHANGE c_varchar c_varchar char(10) CHARACTER SET GBK;
+ALTER TABLE t1 CHANGE c_text c_text char(10) CHARACTER SET GBK;
+select hex(c_char),hex(c_varchar),hex(c_text) from t1;
+hex(c_char)	hex(c_varchar)	hex(c_text)
+D6D0B9FA	D5E3BDAD	BABCD6DD
+show create table t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `c_char` char(10) CHARACTER SET gbk COLLATE gbk_chinese_ci DEFAULT NULL,
+  `c_varchar` char(10) CHARACTER SET gbk COLLATE gbk_chinese_ci DEFAULT NULL,
+  `c_text` char(10) CHARACTER SET gbk COLLATE gbk_chinese_ci DEFAULT NULL,
+  `ex_column` char(30) DEFAULT NULL
+) ENGINE=TIANMU DEFAULT CHARSET=latin1
+drop table t1;
+drop database test_issue848;
diff --git a/mysql-test/suite/tianmu/t/issue848.test b/mysql-test/suite/tianmu/t/issue848.test
@@ -0,0 +1,34 @@
+--source include/have_tianmu.inc
+create database test_issue848;
+use test_issue848;
+CREATE TABLE `t1` (
+  `c_char` char(10) DEFAULT NULL COMMENT 'char',
+  `c_varchar` varchar(10) DEFAULT NULL COMMENT 'varchar',
+  `c_text` text COMMENT 'text') 
+ENGINE=TIANMU DEFAULT CHARSET=GBK;
+show create table t1;
+
+insert into t1(c_char,c_varchar,c_text) values(x'D6D0B9FA',x'D5E3BDAD',x'BABCD6DD');
+ALTER TABLE t1 DEFAULT CHARACTER SET gbk;
+show create table t1;
+
+ALTER TABLE t1 CHARACTER SET latin1;
+show create table t1;
+
+alter table t1  add column ex_column char(30);
+show create table t1;
+
+ALTER TABLE t1 MODIFY c_char char(10) CHARACTER SET UTF8MB4;
+ALTER TABLE t1 MODIFY c_varchar char(10) CHARACTER SET UTF8MB4;
+ALTER TABLE t1 MODIFY c_text char(10) CHARACTER SET UTF8MB4;
+show create table t1;
+select hex(c_char),hex(c_varchar),hex(c_text) from t1;
+
+ALTER TABLE t1 CHANGE c_char c_char char(10) CHARACTER SET GBK;
+ALTER TABLE t1 CHANGE c_varchar c_varchar char(10) CHARACTER SET GBK;
+ALTER TABLE t1 CHANGE c_text c_text char(10) CHARACTER SET GBK;
+select hex(c_char),hex(c_varchar),hex(c_text) from t1;
+show create table t1;
+
+drop table t1;
+drop database test_issue848;
diff --git a/storage/tianmu/core/engine.cpp b/storage/tianmu/core/engine.cpp
@@ -758,7 +758,9 @@ void Engine::CommitTx(THD *thd, bool all) {
   // skip for first time, called in trans_commit_stmt(), thd->server_status = SERVER_STATUS_IN_TRANS and all = false
   // then called in trans_commit_implicit(), thd->server_status will be set "~SERVER_STATUS_IN_TRANS", and all = true
   // tianmu truncate table in 5.7 is ok, 8.0 has this problem.
-  if ((thd->lex->sql_command == SQLCOM_TRUNCATE) && (thd->server_status & SERVER_STATUS_IN_TRANS)) {
+  // Same problem with alter table ,see: https://github.com/stoneatom/stonedb/issues/1670
+  if ((thd->lex->sql_command == SQLCOM_TRUNCATE || thd->lex->sql_command == SQLCOM_ALTER_TABLE) &&
+      (thd->server_status & SERVER_STATUS_IN_TRANS)) {
     return;
   }
   if (all || !thd_test_options(thd, OPTION_NOT_AUTOCOMMIT)) {

diff --git a/storage/tianmu/handler/ha_tianmu.cpp b/storage/tianmu/handler/ha_tianmu.cpp
@@ -61,6 +61,8 @@ const Alter_inplace_info::HA_ALTER_FLAGS ha_tianmu::TIANMU_SUPPORTED_ALTER_ADD_D
     Alter_inplace_info::ADD_COLUMN | Alter_inplace_info::DROP_COLUMN | Alter_inplace_info::ALTER_STORED_COLUMN_ORDER;
 const Alter_inplace_info::HA_ALTER_FLAGS ha_tianmu::TIANMU_SUPPORTED_ALTER_COLUMN_NAME =
     Alter_inplace_info::ALTER_COLUMN_DEFAULT | Alter_inplace_info::ALTER_COLUMN_NAME;
+const Alter_inplace_info::HA_ALTER_FLAGS ha_tianmu::TIANMU_SUPPORTED_ALTER_TABLE_OPTIONS =
+    Alter_inplace_info::CHANGE_CREATE_OPTION;
 /////////////////////////////////////////////////////////////////////
 //
 // NOTICE: ALL EXCEPTIONS SHOULD BE CAUGHT in the handler API!!!
@@ -1511,6 +1513,11 @@ int ha_tianmu::reset() {
 enum_alter_inplace_result ha_tianmu::check_if_supported_inplace_alter([[maybe_unused]] TABLE *altered_table,
                                                                       Alter_inplace_info *ha_alter_info) {
   DBUG_ENTER(__PRETTY_FUNCTION__);
+  if ((ha_alter_info->handler_flags & TIANMU_SUPPORTED_ALTER_TABLE_OPTIONS) &&
+      (ha_alter_info->create_info->used_fields & HA_CREATE_USED_DEFAULT_CHARSET)) {
+    DBUG_RETURN(HA_ALTER_INPLACE_EXCLUSIVE_LOCK);
+  }
+
   if ((ha_alter_info->handler_flags & ~TIANMU_SUPPORTED_ALTER_ADD_DROP_ORDER) &&
       (ha_alter_info->handler_flags != TIANMU_SUPPORTED_ALTER_COLUMN_NAME)) {
     // support alter table column type
@@ -1532,14 +1539,18 @@ bool ha_tianmu::inplace_alter_table(TABLE *altered_table [[maybe_unused]],
                                     Alter_inplace_info *ha_alter_info [[maybe_unused]],
                                     const dd::Table *old_table_def [[maybe_unused]],
                                     dd::Table *new_table_def [[maybe_unused]]) {  // stonedb8 TODO
+  DBUG_ENTER(__PRETTY_FUNCTION__);
   try {
-    if (!(ha_alter_info->handler_flags & ~TIANMU_SUPPORTED_ALTER_ADD_DROP_ORDER)) {
+    if ((ha_alter_info->handler_flags & TIANMU_SUPPORTED_ALTER_TABLE_OPTIONS) &&
+        (ha_alter_info->create_info->used_fields & HA_CREATE_USED_DEFAULT_CHARSET)) {
+      DBUG_RETURN(false);
+    } else if (!(ha_alter_info->handler_flags & ~TIANMU_SUPPORTED_ALTER_ADD_DROP_ORDER)) {
       std::vector<Field *> v_old(table_share->field, table_share->field + table_share->fields);
       std::vector<Field *> v_new(altered_table->s->field, altered_table->s->field + altered_table->s->fields);
       ha_rcengine_->PrepareAlterTable(table_name_, v_new, v_old, ha_thd());
-      return false;
+      DBUG_RETURN(false);
     } else if (ha_alter_info->handler_flags == TIANMU_SUPPORTED_ALTER_COLUMN_NAME) {
-      return false;
+      DBUG_RETURN(false);
     }
   } catch (std::exception &e) {
     TIANMU_LOG(LogCtl_Level::ERROR, "An exception is caught: %s", e.what());
@@ -1549,24 +1560,29 @@ bool ha_tianmu::inplace_alter_table(TABLE *altered_table [[maybe_unused]],
 
   my_message(static_cast<int>(common::ErrorCode::UNKNOWN_ERROR), "Unable to inplace alter table", MYF(0));
 
-  return true;
+  DBUG_RETURN(true);
 }
 
 bool ha_tianmu::commit_inplace_alter_table(TABLE *altered_table [[maybe_unused]],
                                            Alter_inplace_info *ha_alter_info [[maybe_unused]],
                                            bool commit [[maybe_unused]],
                                            const dd::Table *old_table_def [[maybe_unused]],
                                            dd::Table *new_table_def [[maybe_unused]]) {  // stonedb8 TODO
+  DBUG_ENTER(__PRETTY_FUNCTION__);
   if (!commit) {
     TIANMU_LOG(LogCtl_Level::INFO, "Alter table failed : %s%s", table_name_.c_str(), " rollback");
-    return true;
+    DBUG_RETURN(true);
+  }
+  if ((ha_alter_info->handler_flags & TIANMU_SUPPORTED_ALTER_TABLE_OPTIONS) &&
+      (ha_alter_info->create_info->used_fields & HA_CREATE_USED_DEFAULT_CHARSET)) {
+    DBUG_RETURN(false);
   }
   if (ha_alter_info->handler_flags == TIANMU_SUPPORTED_ALTER_COLUMN_NAME) {
-    return false;
+    DBUG_RETURN(false);
   }
   if ((ha_alter_info->handler_flags & ~TIANMU_SUPPORTED_ALTER_ADD_DROP_ORDER)) {
     TIANMU_LOG(LogCtl_Level::INFO, "Altered table not support type %llu", ha_alter_info->handler_flags);
-    return true;
+    DBUG_RETURN(true);
   }
   fs::path tmp_dir(table_name_ + ".tmp");
   fs::path tab_dir(table_name_ + common::TIANMU_EXT);
@@ -1599,9 +1615,9 @@ bool ha_tianmu::commit_inplace_alter_table(TABLE *altered_table [[maybe_unused]]
     TIANMU_LOG(LogCtl_Level::ERROR, "file system error: %s %s|%s", e.what(), e.path1().string().c_str(),
                e.path2().string().c_str());
     my_message(static_cast<int>(common::ErrorCode::UNKNOWN_ERROR), "Failed to commit alter table", MYF(0));
-    return true;
+    DBUG_RETURN(true);
   }
-  return false;
+  DBUG_RETURN(false);
 }
 /*
  key: mysql format, may be union key, need changed to kvstore key format

diff --git a/storage/tianmu/handler/ha_tianmu.h b/storage/tianmu/handler/ha_tianmu.h
@@ -149,6 +149,7 @@ class ha_tianmu final : public handler {
  public:
   static const Alter_inplace_info::HA_ALTER_FLAGS TIANMU_SUPPORTED_ALTER_ADD_DROP_ORDER;
   static const Alter_inplace_info::HA_ALTER_FLAGS TIANMU_SUPPORTED_ALTER_COLUMN_NAME;
+  static const Alter_inplace_info::HA_ALTER_FLAGS TIANMU_SUPPORTED_ALTER_TABLE_OPTIONS;
 
  protected:
   int set_cond_iter();