From 5ed4b4b770b1b884d8b0d8569e4d3854fd4cbf96 Mon Sep 17 00:00:00 2001 From: lujiashun Date: Tue, 10 Jan 2023 21:09:07 +0800 Subject: [PATCH] fix(tianmu): support add integer/decimal column DDL with default value. (#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; --- mysql-test/suite/tianmu/r/issue1187.result | 46 +++++++++++ mysql-test/suite/tianmu/t/issue1187.test | 40 ++++++++++ storage/tianmu/core/engine.cpp | 11 ++- storage/tianmu/core/engine.h | 3 +- storage/tianmu/core/tianmu_attr.cpp | 78 ++++++++++++++---- storage/tianmu/core/tianmu_attr_typeinfo.cpp | 4 + storage/tianmu/core/tianmu_attr_typeinfo.h | 4 + storage/tianmu/core/value.h | 83 +++++++++++++++----- storage/tianmu/handler/ha_tianmu.cpp | 13 ++- storage/tianmu/handler/ha_tianmu.h | 1 + 10 files changed, 245 insertions(+), 38 deletions(-) create mode 100644 mysql-test/suite/tianmu/r/issue1187.result create mode 100644 mysql-test/suite/tianmu/t/issue1187.test diff --git a/mysql-test/suite/tianmu/r/issue1187.result b/mysql-test/suite/tianmu/r/issue1187.result new file mode 100644 index 0000000000..71ab495717 --- /dev/null +++ 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 new file mode 100644 index 0000000000..dc179cdca3 --- /dev/null +++ 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; \ No newline at end of file diff --git a/storage/tianmu/core/engine.cpp b/storage/tianmu/core/engine.cpp index 1cfc03f6bc..af0b18caf3 100644 --- a/storage/tianmu/core/engine.cpp +++ b/storage/tianmu/core/engine.cpp @@ -635,7 +635,16 @@ std::shared_ptr 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 index b718ce7b9a..27f53ffc83 100644 --- 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 *blob_buf); diff --git a/storage/tianmu/core/tianmu_attr.cpp b/storage/tianmu/core/tianmu_attr.cpp index 6de8f59612..69e271bd38 100644 --- 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 index 4ba59ac720..858650d55f 100644 --- 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 index 26f7d32126..26d19801ad 100644 --- a/storage/tianmu/core/tianmu_attr_typeinfo.h +++ b/storage/tianmu/core/tianmu_attr_typeinfo.h @@ -21,6 +21,7 @@ #include #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::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::digits> flag_; }; diff --git a/storage/tianmu/core/value.h b/storage/tianmu/core/value.h index c58b799c82..52f2efb854 100644 --- a/storage/tianmu/core/value.h +++ b/storage/tianmu/core/value.h @@ -18,42 +18,87 @@ #define TIANMU_CORE_VALUE_H_ #pragma once +#include #include #include 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(var); } - bool IsInt() const { return std::holds_alternative(var); } - bool IsDouble() const { return std::holds_alternative(var); } - bool IsString() const { return std::holds_alternative(var); } - bool IsStringView() const { return std::holds_alternative(var); } + bool HasValue() const { return var_.which() != static_cast(ValueType::kValueNull); } + bool IsInt() const { return var_.which() == static_cast(ValueType::kValueInt); } + bool IsDouble() const { return var_.which() == static_cast(ValueType::kValueDouble); } + bool IsString() const { return var_.which() == static_cast(ValueType::kVauleString); } + bool IsStringView() const { return var_.which() == static_cast(ValueType::kValueStringView); } - void SetInt(int64_t v) { var = v; } - int64_t &GetInt() { return std::get(var); } - const int64_t &GetInt() const { return std::get(var); } + void SetInt(int64_t v) { var_ = v; } + int64_t &GetInt() { return boost::get(var_); } + const int64_t &GetInt() const { return boost::get(var_); } - void SetDouble(double v) { var = v; } - double &GetDouble() { return std::get(var); } - const double &GetDouble() const { return std::get(var); } + void SetDouble(double v) { var_ = v; } + double &GetDouble() { return boost::get(var_); } + const double &GetDouble() const { return boost::get(var_); } - void SetString(char *s, uint n) { var.emplace(s, n); } - std::string &GetString() { return std::get(var); } - const std::string &GetString() const { return std::get(var); } + void SetString(char *s, uint n) { var_ = std::string(s, n); } + std::string &GetString() { return boost::get(var_); } + const std::string &GetString() const { return boost::get(var_); } - void SetStringView(char *s, uint n) { var.emplace(s, n); } - std::string_view &GetStringView() { return std::get(var); } - const std::string_view &GetStringView() const { return std::get(var); } + void SetStringView(char *s, uint n) { var_ = std::string_view(s, n); } + std::string_view &GetStringView() { return boost::get(var_); } + const std::string_view &GetStringView() const { return boost::get(var_); } private: - std::variant var; + enum class ValueType { + kValueNull = 0, + kValueInt = 1, + kValueDouble = 2, + kVauleString = 3, + kValueStringView = 4, + }; + boost::variant var_; }; + +#else + +class Value final { + public: + Value() = default; + explicit Value(int64_t i) { var_ = i; } + ~Value() = default; + + bool HasValue() const { return !std::holds_alternative(var_); } + bool IsInt() const { return std::holds_alternative(var_); } + bool IsDouble() const { return std::holds_alternative(var_); } + bool IsString() const { return std::holds_alternative(var_); } + bool IsStringView() const { return std::holds_alternative(var_); } + + void SetInt(int64_t v) { var_ = v; } + int64_t &GetInt() { return std::get(var_); } + const int64_t &GetInt() const { return std::get(var_); } + + void SetDouble(double v) { var_ = v; } + double &GetDouble() { return std::get(var_); } + const double &GetDouble() const { return std::get(var_); } + + void SetString(char *s, uint n) { var_.emplace(s, n); } + std::string &GetString() { return std::get(var_); } + const std::string &GetString() const { return std::get(var_); } + + void SetStringView(char *s, uint n) { var_.emplace(s, n); } + std::string_view &GetStringView() { return std::get(var_); } + const std::string_view &GetStringView() const { return std::get(var_); } + + private: + std::variant var_; +}; +#endif } // namespace core } // namespace Tianmu diff --git a/storage/tianmu/handler/ha_tianmu.cpp b/storage/tianmu/handler/ha_tianmu.cpp index 56d2431633..06d6686d32 100644 --- 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 v_old(table_share->field, table_share->field + table_share->fields); - std::vector v_new(altered_table->s->field, altered_table->s->field + altered_table->s->fields); + + Field_iterator_table it_field; + std::vector 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) { diff --git a/storage/tianmu/handler/ha_tianmu.h b/storage/tianmu/handler/ha_tianmu.h index 396ebe2922..d70f43aeaa 100644 --- a/storage/tianmu/handler/ha_tianmu.h +++ b/storage/tianmu/handler/ha_tianmu.h @@ -23,6 +23,7 @@ namespace Tianmu { namespace handler { +extern Tianmu::core::Value GetValueFromField(Field *f); // Class definition for the storage engine class ha_tianmu final : public handler {