fix(tianmu): support add integer/decimal column DDL with default valu…

…e. (stoneatom#1187) [summary] check the default value of field,please see tianmu_attr.cpp for details: 1. if field is integer type but not real type; 2. if field is integer type and also real type; 3. if field is not integer type;
lujiashun · Jan 12, 2023 · 62cd214 · 62cd214
1 parent 3fa2aab
commit 62cd214
Show file tree

Hide file tree

Showing 10 changed files with 244 additions and 38 deletions.
diff --git a/mysql-test/suite/tianmu/r/issue1187.result b/mysql-test/suite/tianmu/r/issue1187.result
@@ -0,0 +1,46 @@
+DROP DATABASE IF EXISTS issuse1187_test;
+CREATE DATABASE issuse1187_test;
+USE issuse1187_test;
+create table t1(id int,name varchar(5)) ;
+insert into t1 values(1,'AAA'),(2,'BBB');
+alter table t1 add column age int null;
+select * from t1;
+id	name	age
+1	AAA	NULL
+2	BBB	NULL
+create table t2(id int,name varchar(5)) ;
+insert into t2 values(1,'AAA'),(2,'BBB');
+alter table t2 add column age int not null;
+select * from t2;
+id	name	age
+1	AAA	0
+2	BBB	0
+create table t3(id int,name varchar(5)) ;
+insert into t3 values(1,'AAA'),(2,'BBB');
+alter table t3 add column age int not null default 66;
+select * from t3;
+id	name	age
+1	AAA	66
+2	BBB	66
+create table t4(id int,name varchar(5)) ;
+insert into t4 values(1,'AAA'),(2,'BBB');
+alter table t4 add column age DECIMAL(17,9);
+select * from t4;
+id	name	age
+1	AAA	NULL
+2	BBB	NULL
+create table t5(id int,name varchar(5)) ;
+insert into t5 values(1,'AAA'),(2,'BBB');
+alter table t5 add column age DECIMAL(17,9) not null;
+select * from t5;
+id	name	age
+1	AAA	0.000000000
+2	BBB	0.000000000
+create table t6(id int,name varchar(5)) ;
+insert into t6 values(1,'AAA'),(2,'BBB');
+alter table t6 add column age DECIMAL(17,9) not null default '800.0024';
+select * from t6;
+id	name	age
+1	AAA	800.002400000
+2	BBB	800.002400000
+DROP DATABASE issuse1187_test;
diff --git a/mysql-test/suite/tianmu/t/issue1187.test b/mysql-test/suite/tianmu/t/issue1187.test
@@ -0,0 +1,40 @@
+--source include/have_tianmu.inc
+
+--disable_warnings
+DROP DATABASE IF EXISTS issuse1187_test;
+--enable_warnings
+
+CREATE DATABASE issuse1187_test;
+USE issuse1187_test;
+
+create table t1(id int,name varchar(5)) ;
+insert into t1 values(1,'AAA'),(2,'BBB');
+alter table t1 add column age int null;
+select * from t1;
+
+create table t2(id int,name varchar(5)) ;
+insert into t2 values(1,'AAA'),(2,'BBB');
+alter table t2 add column age int not null;
+select * from t2;
+
+create table t3(id int,name varchar(5)) ;
+insert into t3 values(1,'AAA'),(2,'BBB');
+alter table t3 add column age int not null default 66;
+select * from t3;
+
+create table t4(id int,name varchar(5)) ;
+insert into t4 values(1,'AAA'),(2,'BBB');
+alter table t4 add column age DECIMAL(17,9);
+select * from t4;
+
+create table t5(id int,name varchar(5)) ;
+insert into t5 values(1,'AAA'),(2,'BBB');
+alter table t5 add column age DECIMAL(17,9) not null;
+select * from t5;
+
+create table t6(id int,name varchar(5)) ;
+insert into t6 values(1,'AAA'),(2,'BBB');
+alter table t6 add column age DECIMAL(17,9) not null default '800.0024';
+select * from t6;
+
+DROP DATABASE issuse1187_test;
diff --git a/storage/tianmu/core/engine.cpp b/storage/tianmu/core/engine.cpp
@@ -635,7 +635,15 @@ std::shared_ptr<TableOption> Engine::GetTableOption(const std::string &table, TA
 
 void Engine::CreateTable(const std::string &table, TABLE *form) { TianmuTable::CreateNew(GetTableOption(table, form)); }
 
-AttributeTypeInfo Engine::GetAttrTypeInfo(const Field &field) {
+AttributeTypeInfo Engine::GetAttrTypeInfo(Field &field) {
+  AttributeTypeInfo ati = GetAttrTypeInfoInternal(field);
+  bitmap_set_bit(field.table->write_set, field.field_index);
+  bitmap_set_bit(field.table->read_set, field.field_index);
+  field.set_default();
+  ati.SetDefaultValue(&field);
+  return ati;
+}
+AttributeTypeInfo Engine::GetAttrTypeInfoInternal(const Field &field) {
   bool auto_inc = field.flags & AUTO_INCREMENT_FLAG;
   if (auto_inc && field.part_of_key.to_ulonglong() == 0) {
     throw common::AutoIncException("AUTO_INCREMENT can be only declared on primary key column!");

diff --git a/storage/tianmu/core/engine.h b/storage/tianmu/core/engine.h
@@ -148,7 +148,8 @@ class Engine final {
  public:
   static common::ColumnType GetCorrespondingType(const Field &field);
   static AttributeTypeInfo GetCorrespondingATI(Field &field);
-  static AttributeTypeInfo GetAttrTypeInfo(const Field &field);
+  static AttributeTypeInfo GetAttrTypeInfo(Field &field);
+  static AttributeTypeInfo GetAttrTypeInfoInternal(const Field &field);
   static common::ColumnType GetCorrespondingType(const enum_field_types &eft);
   static bool IsTianmuTable(TABLE *table);
   static bool ConvertToField(Field *field, types::TianmuDataType &tianmu_item, std::vector<uchar> *blob_buf);

diff --git a/storage/tianmu/core/tianmu_attr.cpp b/storage/tianmu/core/tianmu_attr.cpp
@@ -87,19 +87,42 @@ void TianmuAttr::Create(const fs::path &dir, const AttributeTypeInfo &ati, uint8
   fmeta.WriteExact(&meta, sizeof(meta));
   fmeta.Flush();
 
+  size_t no_nulls{no_rows};
+  int64_t min{0};
+  int64_t max{0};
+  bool int_not_null{false};
+  bool is_real{false};
+  core::Value value = ati.GetDefaultValue();
+  if (ati.GetPackType() == common::PackType::INT && value.HasValue()) {
+    int_not_null = true;
+  }
+  if (ATI::IsRealType(ati.Type())) {
+    is_real = true;
+  }
+
+  if (int_not_null) {
+    no_nulls = 0;
+    if (is_real) {
+      common::double_int_t t(value.GetDouble());
+      min = max = t.i;
+    } else {
+      min = max = value.GetInt();
+    }
+  }
+
   COL_VER_HDR hdr{
-      no_rows,  // no_obj
-      no_rows,  // no_nulls
-      no_pack,  // no of packs
-      0,        // no of deleted
-      0,        // auto_inc next
-      0,        // min
-      0,        // max
-      0,        // dict file version name. 0 means n/a
-      0,        // is unique?
-      0,        // is unique_updated?
-      0,        // natural size
-      0,        // compressed size
+      no_rows,   // no_obj
+      no_nulls,  // no_nulls
+      no_pack,   // no of packs
+      0,         // no of deleted
+      0,         // auto_inc next
+      min,       // min
+      max,       // max
+      0,         // dict file version name. 0 means n/a
+      0,         // is unique?
+      0,         // is unique_updated?
+      0,         // natural size
+      0,         // compressed size
   };
 
   if (ati.Lookup()) {
@@ -128,7 +151,20 @@ void TianmuAttr::Create(const fs::path &dir, const AttributeTypeInfo &ati, uint8
     DPN dpn;
     dpn.reset();
     dpn.used = 1;
-    dpn.numOfNulls = 1 << pss;
+    if (int_not_null) {
+      dpn.numOfNulls = 0;
+      if (is_real) {
+        dpn.max_d = value.GetDouble();
+        dpn.min_d = value.GetDouble();
+        dpn.sum_d = value.GetDouble() * (1 << pss);
+      } else {
+        dpn.max_i = value.GetInt();
+        dpn.min_i = value.GetInt();
+        dpn.sum_i = value.GetInt() * (1 << pss);
+      }
+    } else {
+      dpn.numOfNulls = 1 << pss;
+    }
     dpn.numOfRecords = 1 << pss;
     dpn.xmax = common::MAX_XID;
     dpn.dataAddress = DPN_INVALID_ADDR;
@@ -141,7 +177,21 @@ void TianmuAttr::Create(const fs::path &dir, const AttributeTypeInfo &ati, uint8
     auto left = no_rows % (1 << pss);
     if (left != 0) {
       dpn.numOfRecords = left;
-      dpn.numOfNulls = left;
+
+      if (int_not_null) {
+        dpn.numOfNulls = 0;
+        if (is_real) {
+          dpn.max_d = value.GetDouble();
+          dpn.min_d = value.GetDouble();
+          dpn.sum_d = value.GetDouble() * left;
+        } else {
+          dpn.max_i = value.GetInt();
+          dpn.min_i = value.GetInt();
+          dpn.sum_i = value.GetInt() * left;
+        }
+      } else {
+        dpn.numOfNulls = left;
+      }
     }
     fdn.WriteExact(&dpn, sizeof(dpn));
     fdn.Flush();

diff --git a/storage/tianmu/core/tianmu_attr_typeinfo.cpp b/storage/tianmu/core/tianmu_attr_typeinfo.cpp
@@ -19,6 +19,7 @@
 #include "common/data_format.h"
 #include "common/txt_data_format.h"
 #include "core/tianmu_attr.h"
+#include "handler/ha_tianmu.h"
 #include "types/tianmu_data_types.h"
 #include "types/tianmu_num.h"
 
@@ -36,5 +37,8 @@ const types::TianmuDataType &AttributeTypeInfo::ValuePrototype() const {
   DEBUG_ASSERT(ATI::IsDateTimeType(attrt_));
   return types::TianmuDateTime::NullValue();
 }
+
+void AttributeTypeInfo::SetDefaultValue(Field *field) { value_ = Tianmu::handler::GetValueFromField(field); }
+
 }  // namespace core
 }  // namespace Tianmu
diff --git a/storage/tianmu/core/tianmu_attr_typeinfo.h b/storage/tianmu/core/tianmu_attr_typeinfo.h
@@ -21,6 +21,7 @@
 #include <bitset>
 
 #include "common/common_definitions.h"
+#include "core/value.h"
 
 namespace Tianmu {
 namespace types {
@@ -124,6 +125,8 @@ class AttributeTypeInfo {
   bool Lookup() const { return fmt_ == common::PackFmt::LOOKUP; }
   unsigned char Flag() const { return flag_.to_ulong(); }
   void SetFlag(unsigned char v) { flag_ = std::bitset<std::numeric_limits<unsigned char>::digits>(v); }
+  void SetDefaultValue(Field *field);
+  core::Value GetDefaultValue() const { return value_; }
 
  private:
   common::ColumnType attrt_;
@@ -132,6 +135,7 @@ class AttributeTypeInfo {
   int scale_;
   DTCollation collation_;
   std::string field_name_;
+  core::Value value_;
 
   std::bitset<std::numeric_limits<unsigned char>::digits> flag_;
 };

diff --git a/storage/tianmu/core/value.h b/storage/tianmu/core/value.h
@@ -18,42 +18,87 @@
 #define TIANMU_CORE_VALUE_H_
 #pragma once
 
+#include <boost/variant.hpp>
 #include <string>
 #include <variant>
 
 namespace Tianmu {
 namespace core {
+// std::variant has some bug in gcc8
+#if defined(__GNUC__) && (__GNUC__ == 8)
 class Value final {
  public:
   Value() = default;
-  explicit Value(int64_t i) { var = i; }
+  explicit Value(int64_t i) { var_ = i; }
   ~Value() = default;
 
-  bool HasValue() const { return !std::holds_alternative<std::monostate>(var); }
-  bool IsInt() const { return std::holds_alternative<int64_t>(var); }
-  bool IsDouble() const { return std::holds_alternative<double>(var); }
-  bool IsString() const { return std::holds_alternative<std::string>(var); }
-  bool IsStringView() const { return std::holds_alternative<std::string_view>(var); }
+  bool HasValue() const { return var_.which() != static_cast<int>(ValueType::kValueNull); }
+  bool IsInt() const { return var_.which() == static_cast<int>(ValueType::kValueInt); }
+  bool IsDouble() const { return var_.which() == static_cast<int>(ValueType::kValueDouble); }
+  bool IsString() const { return var_.which() == static_cast<int>(ValueType::kVauleString); }
+  bool IsStringView() const { return var_.which() == static_cast<int>(ValueType::kValueStringView); }
 
-  void SetInt(int64_t v) { var = v; }
-  int64_t &GetInt() { return std::get<int64_t>(var); }
-  const int64_t &GetInt() const { return std::get<int64_t>(var); }
+  void SetInt(int64_t v) { var_ = v; }
+  int64_t &GetInt() { return boost::get<int64_t>(var_); }
+  const int64_t &GetInt() const { return boost::get<int64_t>(var_); }
 
-  void SetDouble(double v) { var = v; }
-  double &GetDouble() { return std::get<double>(var); }
-  const double &GetDouble() const { return std::get<double>(var); }
+  void SetDouble(double v) { var_ = v; }
+  double &GetDouble() { return boost::get<double>(var_); }
+  const double &GetDouble() const { return boost::get<double>(var_); }
 
-  void SetString(char *s, uint n) { var.emplace<std::string>(s, n); }
-  std::string &GetString() { return std::get<std::string>(var); }
-  const std::string &GetString() const { return std::get<std::string>(var); }
+  void SetString(char *s, uint n) { var_ = std::string(s, n); }
+  std::string &GetString() { return boost::get<std::string>(var_); }
+  const std::string &GetString() const { return boost::get<std::string>(var_); }
 
-  void SetStringView(char *s, uint n) { var.emplace<std::string_view>(s, n); }
-  std::string_view &GetStringView() { return std::get<std::string_view>(var); }
-  const std::string_view &GetStringView() const { return std::get<std::string_view>(var); }
+  void SetStringView(char *s, uint n) { var_ = std::string_view(s, n); }
+  std::string_view &GetStringView() { return boost::get<std::string_view>(var_); }
+  const std::string_view &GetStringView() const { return boost::get<std::string_view>(var_); }
 
  private:
-  std::variant<std::monostate, int64_t, double, std::string, std::string_view> var;
+  enum class ValueType {
+    kValueNull = 0,
+    kValueInt = 1,
+    kValueDouble = 2,
+    kVauleString = 3,
+    kValueStringView = 4,
+  };
+  boost::variant<boost::blank, int64_t, double, std::string, std::string_view> var_;
 };
+
+#else
+
+class Value final {
+ public:
+  Value() = default;
+  explicit Value(int64_t i) { var_ = i; }
+  ~Value() = default;
+
+  bool HasValue() const { return !std::holds_alternative<std::monostate>(var_); }
+  bool IsInt() const { return std::holds_alternative<int64_t>(var_); }
+  bool IsDouble() const { return std::holds_alternative<double>(var_); }
+  bool IsString() const { return std::holds_alternative<std::string>(var_); }
+  bool IsStringView() const { return std::holds_alternative<std::string_view>(var_); }
+
+  void SetInt(int64_t v) { var_ = v; }
+  int64_t &GetInt() { return std::get<int64_t>(var_); }
+  const int64_t &GetInt() const { return std::get<int64_t>(var_); }
+
+  void SetDouble(double v) { var_ = v; }
+  double &GetDouble() { return std::get<double>(var_); }
+  const double &GetDouble() const { return std::get<double>(var_); }
+
+  void SetString(char *s, uint n) { var_.emplace<std::string>(s, n); }
+  std::string &GetString() { return std::get<std::string>(var_); }
+  const std::string &GetString() const { return std::get<std::string>(var_); }
+
+  void SetStringView(char *s, uint n) { var_.emplace<std::string_view>(s, n); }
+  std::string_view &GetStringView() { return std::get<std::string_view>(var_); }
+  const std::string_view &GetStringView() const { return std::get<std::string_view>(var_); }
+
+ private:
+  std::variant<std::monostate, int64_t, double, std::string, std::string_view> var_;
+};
+#endif
 }  // namespace core
 }  // namespace Tianmu
 

diff --git a/storage/tianmu/handler/ha_tianmu.cpp b/storage/tianmu/handler/ha_tianmu.cpp
@@ -80,7 +80,7 @@ my_bool rcbase_query_caching_of_table_permitted(THD *thd, [[maybe_unused]] char
   return ((my_bool)FALSE);
 }
 
-static core::Value GetValueFromField(Field *f) {
+core::Value GetValueFromField(Field *f) {
   core::Value v;
 
   if (f->is_null())
@@ -112,7 +112,8 @@ static core::Value GetValueFromField(Field *f) {
       // convert to UTC
       if (!common::IsTimeStampZero(my_time)) {
         my_bool myb;
-        my_time_t secs_utc = current_txn_->Thd()->variables.time_zone->TIME_to_gmt_sec(&my_time, &myb);
+        // in some senario,current_txn_ is empty, so use f->table->in_use instead to get THD.
+        my_time_t secs_utc = f->table->in_use->variables.time_zone->TIME_to_gmt_sec(&my_time, &myb);
         common::GMTSec2GMTTime(&my_time, secs_utc);
       }
       types::DT dt = {};
@@ -1634,7 +1635,13 @@ bool ha_tianmu::inplace_alter_table(TABLE *altered_table, Alter_inplace_info *ha
         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);
+
+      Field_iterator_table it_field;
+      std::vector<Field *> v_new;
+      for (it_field.set_table(altered_table); !it_field.end_of_fields(); it_field.next()) {
+        v_new.push_back(it_field.field());
+      }
+
       ha_tianmu_engine_->PrepareAlterTable(table_name_, v_new, v_old, ha_thd());
       DBUG_RETURN(false);
     } else if (ha_alter_info->handler_flags == TIANMU_SUPPORTED_ALTER_COLUMN_NAME) {