Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(tianmu): support add integer/decimal column DDL with default value. (#1187) #1208

Closed
wants to merge 1 commit into from
Closed

feat(tianmu): support add integer/decimal column DDL with default value. (#1187) #1208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions mysql-test/suite/tianmu/r/issue1187.result
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
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
insert into t3(id,name) values (3,'CCC');
select * from t3;
id name age
1 AAA 66
2 BBB 66
3 CCC 66
alter table t3 alter column age set default 77;
insert into t3(id,name) values (4,'DDD');
select * from t3;
id name age
1 AAA 66
2 BBB 66
3 CCC 66
4 DDD 77
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;
45 changes: 45 additions & 0 deletions mysql-test/suite/tianmu/t/issue1187.test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
--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;
Copy link
Collaborator

@DandreChen DandreChen Jan 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when the type is varchar not null，what will happen？

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it is convenient，please fix it together when the type is varchar not null
as below case:

create table ttt(id int,name varchar(5));
insert into ttt values(1,'AAA'),(2,'BBB');
alter table ttt add column age varchar(5) not null;
select * from ttt;

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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pls, add some case for errors. such as using wrong default value.
Taking the following as an instance.
The column type is char but using integer as a default value, the type and default value is mismatch. or the default will lead to type overflow.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

F.I.Y: Just only change the default value.
ALTER TABLE table_name ALTER COLUMN column_name SET DEFAULT 'literal';

Copy link
Author

@lujiashun lujiashun Jan 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. ok, i will try to add some wrong default value; maybe it will be intercepted by sql layer, i will test it.(confirmed, it wll be intercepted by sql layer, better not to add this mtr);
  2. "Just only change the default value" is a related sceanrio, i will add this MTR.

select * from t3;
insert into t3(id,name) values (3,'CCC');
select * from t3;
alter table t3 alter column age set default 77;
insert into t3(id,name) values (4,'DDD');
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;
11 changes: 10 additions & 1 deletion storage/tianmu/core/engine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,16 @@ 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);
RingsC marked this conversation as resolved.
Show resolved Hide resolved
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!");
Expand Down
3 changes: 2 additions & 1 deletion storage/tianmu/core/engine.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
78 changes: 64 additions & 14 deletions storage/tianmu/core/tianmu_attr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not use AttributeTypeInfo::NotNull() to check whether the field is NULL? I guess int_not_null means has_default_value?

Copy link
Author

@lujiashun lujiashun Jan 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1.not null flag check is also a solution. in the scenario the alter table defined not null or null(same with tianmu), the
output is NULL. I just want to use GetValueFromField(it deals the default value logic) function to deal with the scenario, the std::variants store the value, it has the interface to decide whether it is null,if not null, what is the default value.
2 The current solution is also a proper way,not to consider the null flag (or it has been considered in GetValueFromField)
3 the std::variant in gcc8 has bugs, what we supported is gcc9.3, if we do not upgrade debian gcc to 9.3, another solution is to use boost:variant, but not recommended. It is better to upgrade the debian CI to gcc9.3 @wisehead

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()) {
Expand Down Expand Up @@ -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;
Expand All @@ -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();
Expand Down
7 changes: 7 additions & 0 deletions storage/tianmu/core/tianmu_attr_typeinfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,14 @@
#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"

namespace Tianmu {
namespace handler {
extern Tianmu::core::Value GetValueFromField(Field *f);
}
namespace core {
int ATI::TextSize(common::ColumnType attrt, uint precision, int scale, DTCollation collation) {
return common::TxtDataFormat::StaticExtrnalSize(attrt, precision, scale, &collation);
Expand All @@ -36,5 +40,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
4 changes: 4 additions & 0 deletions storage/tianmu/core/tianmu_attr_typeinfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <bitset>

#include "common/common_definitions.h"
#include "core/value.h"

namespace Tianmu {
namespace types {
Expand Down Expand Up @@ -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_;
Expand All @@ -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_;
};
Expand Down
81 changes: 62 additions & 19 deletions storage/tianmu/core/value.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,42 +18,85 @@
#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
RingsC marked this conversation as resolved.
Show resolved Hide resolved
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); }
#if defined(__GNUC__) && (__GNUC__ == 8)
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); }
#else
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_); }
#endif

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; }
#if defined(__GNUC__) && (__GNUC__ == 8)
int64_t &GetInt() { return boost::get<int64_t>(var_); }
const int64_t &GetInt() const { return boost::get<int64_t>(var_); }
#else
int64_t &GetInt() { return std::get<int64_t>(var_); }
const int64_t &GetInt() const { return std::get<int64_t>(var_); }
#endif

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; }
#if defined(__GNUC__) && (__GNUC__ == 8)
double &GetDouble() { return boost::get<double>(var_); }
const double &GetDouble() const { return boost::get<double>(var_); }
#else
double &GetDouble() { return std::get<double>(var_); }
const double &GetDouble() const { return std::get<double>(var_); }
#endif

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_.emplace<std::string>(s, n); }
#if defined(__GNUC__) && (__GNUC__ == 8)
std::string &GetString() { return boost::get<std::string>(var_); }
const std::string &GetString() const { return boost::get<std::string>(var_); }
#else
std::string &GetString() { return std::get<std::string>(var_); }
const std::string &GetString() const { return std::get<std::string>(var_); }
#endif

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); }
#if defined(__GNUC__) && (__GNUC__ == 8)
void SetStringView(char *s, uint n) { var_ = std::string_view(s, n); }
std::string_view &GetStringView() { return boost::get<std::string_view>(var_); }
RingsC marked this conversation as resolved.
Show resolved Hide resolved
const std::string_view &GetStringView() const { return boost::get<std::string_view>(var_); }
#else
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_); }
#endif

private:
std::variant<std::monostate, int64_t, double, std::string, std::string_view> var;
#if defined(__GNUC__) && (__GNUC__ == 8)
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
std::variant<std::monostate, int64_t, double, std::string, std::string_view> var_;
#endif
};

} // namespace core
} // namespace Tianmu

Expand Down
Loading