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.

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

*: fix the origin_default_value issue in disaggregated mode (#9666) #9729

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
24 changes: 20 additions & 4 deletions dbms/src/Common/FieldVisitors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,14 @@

namespace DB
{
template <typename T>
static inline String format(const DecimalField<T> & x)
{
WriteBufferFromOwnString wb;
writeText(x.getValue(), x.getScale(), wb);
return wb.str();
}

template <typename T>
static inline String formatQuoted(T x)
{
Expand Down Expand Up @@ -170,19 +178,27 @@ String FieldVisitorToString::operator()(const String & x) const
}
String FieldVisitorToString::operator()(const DecimalField<Decimal32> & x) const
{
return formatQuoted(x);
if (isDecimalWithQuoted)
return formatQuoted(x);
return format(x);
}
String FieldVisitorToString::operator()(const DecimalField<Decimal64> & x) const
{
return formatQuoted(x);
if (isDecimalWithQuoted)
return formatQuoted(x);
return format(x);
}
String FieldVisitorToString::operator()(const DecimalField<Decimal128> & x) const
{
return formatQuoted(x);
if (isDecimalWithQuoted)
return formatQuoted(x);
return format(x);
}
String FieldVisitorToString::operator()(const DecimalField<Decimal256> & x) const
{
return formatQuoted(x);
if (isDecimalWithQuoted)
return formatQuoted(x);
return format(x);
}

String FieldVisitorToString::operator()(const Array & x) const
Expand Down
9 changes: 9 additions & 0 deletions dbms/src/Common/FieldVisitors.h
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,16 @@ typename std::decay_t<Visitor>::ResultType applyVisitor(Visitor && visitor, F1 &
/** Prints Field as literal in SQL query */
class FieldVisitorToString : public StaticVisitor<String>
{
private:
bool isDecimalWithQuoted;

public:
FieldVisitorToString()
: isDecimalWithQuoted(true){};

FieldVisitorToString(bool val)
: isDecimalWithQuoted(val){};

String operator()(const Null & x) const;
String operator()(const UInt64 & x) const;
String operator()(const Int64 & x) const;
Expand Down
102 changes: 82 additions & 20 deletions dbms/src/TiDB/Schema/TiDB.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,16 @@
#include <Common/MyTime.h>
#include <Core/Types.h>
#include <DataTypes/DataTypeDecimal.h>
#include <DataTypes/FieldToDataType.h>
#include <IO/Buffer/ReadBufferFromString.h>
#include <Poco/Base64Decoder.h>
#include <Poco/Dynamic/Var.h>
#include <Poco/MemoryStream.h>
#include <Poco/StreamCopier.h>
#include <Poco/StringTokenizer.h>
#include <Storages/MutableSupport.h>
#include <TiDB/Collation/Collator.h>
#include <TiDB/Decode/DatumCodec.h>
#include <TiDB/Decode/JsonBinary.h>
#include <TiDB/Decode/Vector.h>
#include <TiDB/Schema/SchemaNameMapper.h>
Expand All @@ -36,6 +39,7 @@
#include <algorithm>
#include <cmath>
#include <magic_enum.hpp>
#include <string>

namespace DB
{
Expand Down Expand Up @@ -191,7 +195,8 @@ ColumnInfo::ColumnInfo(Poco::JSON::Object::Ptr json)
Field ColumnInfo::defaultValueToField() const
{
const auto & value = origin_default_value;
if (value.isEmpty())
const auto & bit_value = origin_default_bit_value;
if (value.isEmpty() && bit_value.isEmpty())
{
if (hasNotNullFlag())
return DB::GenDefaultField(*this);
Expand Down Expand Up @@ -223,14 +228,12 @@ Field ColumnInfo::defaultValueToField() const
});
case TypeBit:
{
const auto & bit_value = origin_default_bit_value;
if (bit_value.isEmpty())
{
if (hasNotNullFlag())
return DB::GenDefaultField(*this);
return Field();
}
TRY_CATCH_DEFAULT_VALUE_TO_FIELD({ return getBitValue(bit_value.convert<String>()); });
TRY_CATCH_DEFAULT_VALUE_TO_FIELD({
// When we got bit_value from tipb, we have decoded it.
if (auto is_int = bit_value.isInteger(); is_int)
return bit_value.convert<UInt64>();
return getBitValue(bit_value.convert<String>());
});
}
// Floating type.
case TypeFloat:
Expand All @@ -239,7 +242,12 @@ Field ColumnInfo::defaultValueToField() const
case TypeDate:
case TypeDatetime:
case TypeTimestamp:
TRY_CATCH_DEFAULT_VALUE_TO_FIELD({ return DB::parseMyDateTime(value.convert<String>()); });
TRY_CATCH_DEFAULT_VALUE_TO_FIELD({
// When we got value from tipb, we have decoded it.
if (auto is_int = value.isInteger(); is_int)
return value.convert<UInt64>();
return DB::parseMyDateTime(value.convert<String>());
});
case TypeVarchar:
case TypeTinyBlob:
case TypeMediumBlob:
Expand Down Expand Up @@ -277,12 +285,22 @@ Field ColumnInfo::defaultValueToField() const
return getDecimalValue(text);
});
case TypeTime:
TRY_CATCH_DEFAULT_VALUE_TO_FIELD({ return getTimeValue(value.convert<String>()); });
TRY_CATCH_DEFAULT_VALUE_TO_FIELD({
// When we got value from tipb, we have decoded it.
if (auto is_int = value.isInteger(); is_int)
return value.convert<UInt64>();
return getTimeValue(value.convert<String>());
});
case TypeYear:
// Never throw exception here, do not use TRY_CATCH_DEFAULT_VALUE_TO_FIELD
return getYearValue(value.convert<String>());
case TypeSet:
TRY_CATCH_DEFAULT_VALUE_TO_FIELD({ return getSetValue(value.convert<String>()); });
TRY_CATCH_DEFAULT_VALUE_TO_FIELD({
// When we got value from tipb, we have decoded it.
if (auto is_int = value.isInteger(); is_int)
return value.convert<UInt64>();
return getSetValue(value.convert<String>());
});
case TypeTiDBVectorFloat32:
return genVectorFloat32Empty();
default:
Expand Down Expand Up @@ -1362,17 +1380,61 @@ ColumnInfo toTiDBColumnInfo(const tipb::ColumnInfo & tipb_column_info)
tidb_column_info.flag = tipb_column_info.flag();
tidb_column_info.flen = tipb_column_info.columnlen();
tidb_column_info.decimal = tipb_column_info.decimal();
// TiFlash get default value from origin_default_value, check `Field ColumnInfo::defaultValueToField() const`
// So we need to set origin_default_value to tipb_column_info.default_val()
// Related logic in tidb, https://github.com/pingcap/tidb/blob/45318da24d8e4c0c6aab836d291a33f949dd18bf/pkg/table/tables/tables.go#L2303-L2329
// For TypeBit, we need to set origin_default_bit_value to tipb_column_info.default_val().
if (tidb_column_info.tp == TypeBit)
tidb_column_info.origin_default_bit_value = tipb_column_info.default_val();
else
tidb_column_info.origin_default_value = tipb_column_info.default_val();
tidb_column_info.collate = tipb_column_info.collation();
for (int i = 0; i < tipb_column_info.elems_size(); ++i)
tidb_column_info.elems.emplace_back(tipb_column_info.elems(i), i + 1);
// TiFlash get default value from origin_default_value, check `Field ColumnInfo::defaultValueToField() const`
// So we need to set origin_default_value to tipb_column_info.default_val()
// Related logic in tidb, https://github.com/pingcap/tidb/blob/45318da24d8e4c0c6aab836d291a33f949dd18bf/pkg/table/tables/tables.go#L2303-L2329
// And, decode tipb_column_info.default_val.
{
// The default value is null.
if (tipb_column_info.default_val().empty())
{
Poco::Dynamic::Var empty_val;
tidb_column_info.origin_default_value = empty_val;
return tidb_column_info;
}
size_t cursor = 0;
auto val = DB::DecodeDatum(cursor, tipb_column_info.default_val());
if (val.getType() == DB::Field::Types::String)
{
tidb_column_info.origin_default_value = val.get<String>();
return tidb_column_info;
}
switch (tidb_column_info.tp)
{
case TypeDate:
case TypeDatetime:
case TypeTimestamp:
case TypeSet:
tidb_column_info.origin_default_value = val.get<UInt64>();
break;
case TypeTime:
tidb_column_info.origin_default_value = val.get<Int64>();
break;
case TypeBit:
// For TypeBit, we need to set origin_default_bit_value to tipb_column_info.default_val().
tidb_column_info.origin_default_bit_value = val.get<UInt64>();
break;
case TypeYear:
// If the year column has 'not null' option and the value is 0, this year value is '0000'.
if (val.get<Int64>() == 0)
{
Poco::Dynamic::Var empty_val;
tidb_column_info.origin_default_value = empty_val;
break;
}
// else fallthrough
// The above types will be processed again when defaultValueToField is called.
// By distinguishing the type of the string type(by synchronizing the default values in the schema, its type is string),
// we can distinguish whether it needs to be processed.
default:
auto str_val = DB::applyVisitor(DB::FieldVisitorToString(false), val);
tidb_column_info.origin_default_value = str_val;
}
}

return tidb_column_info;
}

Expand Down
26 changes: 26 additions & 0 deletions tests/fullstack-test2/ddl/alter_default_value.test
Original file line number Diff line number Diff line change
Expand Up @@ -103,4 +103,30 @@ mysql> set session tidb_isolation_read_engines='tiflash'; select /*+ read_from_s
| 1 | Unprioritized |
+------+---------------+

mysql> alter table test.t drop column e;

#############
# Alter table... add column with default value
mysql> alter table test.t add column col1 int default 30;
mysql> alter table test.t add column col2 year not null;
mysql> alter table test.t add column col3 decimal(6,2) not null;
mysql> alter table test.t add column col4 decimal(6,2) default "1.234";
mysql> alter table test.t add column col5 int default 123;
mysql> alter table test.t add column col6 varchar(255) default 'sss';
mysql> alter table test.t add column col7 timestamp default '2017-02-11';
mysql> alter table test.t add column col8 datetime default '2018-11-04 23:12:03';
mysql> alter table test.t add column col9 year default '2012';
mysql> alter table test.t add column col10 set('value1', 'value2', 'value3') default 'value3';
mysql> alter table test.t add column col11 enum('value1', 'value2', 'value3') default 'value1';
mysql> alter table test.t add column col12 datetime default '2018-11-04 23:12:03';
mysql> alter table test.t add column col13 time default '2017-02-23 12:18:30';
mysql> alter table test.t add column col14 time default null;
mysql> alter table test.t add column col15 bit(32) default b'000000010000001000000011';
mysql> set session tidb_isolation_read_engines='tiflash'; select col1, col2, col3, col4, col5, col6, col7, col8, col9, col10, col11, col12, col13, col14, hex(col15) from test.t;
+------+------+------+------+------+------+---------------------+---------------------+------+--------+--------+---------------------+----------+-------+------------+
| col1 | col2 | col3 | col4 | col5 | col6 | col7 | col8 | col9 | col10 | col11 | col12 | col13 | col14 | hex(col15) |
+------+------+------+------+------+------+---------------------+---------------------+------+--------+--------+---------------------+----------+-------+------------+
| 30 | 0000 | 0.00 | 1.23 | 123 | sss | 2017-02-11 00:00:00 | 2018-11-04 23:12:03 | 2012 | value3 | value1 | 2018-11-04 23:12:03 | 12:18:30 | NULL | 10203 |
| 30 | 0000 | 0.00 | 1.23 | 123 | sss | 2017-02-11 00:00:00 | 2018-11-04 23:12:03 | 2012 | value3 | value1 | 2018-11-04 23:12:03 | 12:18:30 | NULL | 10203 |
+------+------+------+------+------+------+---------------------+---------------------+------+--------+--------+---------------------+----------+-------+------------+
mysql> drop table if exists test.t