Skip to content

Commit

Permalink
*: fix the origin_default_value issue in disaggregated mode (#9666)
Browse files Browse the repository at this point in the history 
close #9665

Since the TiDB add new column operation does not actually write the new column data, it simply logs it to `origin_default_value`.
In disaggregated mode, the default value may be read using tipb passing the default value. The original logic is to pass the default value of tipb to the default value of ColumnInfo (calling defaultValueToField).

After obtaining the default value of tipb, decode it, and then use the defaultValueToField value for processing.

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
  • Loading branch information
@zimulala @ti-chi-bot
zimulala and ti-chi-bot[bot] authored Nov 25, 2024
1 parent b335225 commit e3e6780
Show file tree
Hide file tree
Showing 4 changed files with 137 additions and 24 deletions.
24 changes: 20 additions & 4 deletions dbms/src/Common/FieldVisitors.cpp
View file
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
View file
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
View file
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
View file
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

0 comments on commit e3e6780

Please sign in to comment.