Skip to content

Commit 6c4f337

Browse files
Storage: Fix DataTypePtr is not shared as expected (#9939) (#10305)
close #9947 Storage: Fix DataTypePtr is not shared as expected * Introduce a class `DataTypePtrCache` and manage the shared cache of `DataTypePtr` instances. * Introduce `DataTypeFactory::getOrSet(const ASTPtr & ast)` and try to find the cache with data type name as "ast->range.first, ast->range.second" logging: Turn the logging level of "updateTableColumnInfo" into debug because that could cause lots of logging when restarting tiflash Signed-off-by: ti-chi-bot <[email protected]> Signed-off-by: JaySon-Huang <[email protected]> Co-authored-by: JaySon <[email protected]> Co-authored-by: JaySon-Huang <[email protected]>
1 parent bf1951c commit 6c4f337

File tree

7 files changed

+172
-51
lines changed

7 files changed

+172
-51
lines changed

dbms/src/DataTypes/DataTypeFactory.cpp

Lines changed: 58 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -34,46 +34,63 @@ extern const int DATA_TYPE_CANNOT_HAVE_ARGUMENTS;
3434
} // namespace ErrorCodes
3535

3636

37-
DataTypePtr DataTypeFactory::get(const String & full_name) const
37+
DataTypePtr DataTypePtrCache::get(const String & full_name) const
3838
{
39-
ParserIdentifierWithOptionalParameters parser;
40-
ASTPtr ast = parseQuery(parser, full_name.data(), full_name.data() + full_name.size(), "data type", 0);
41-
return get(ast);
39+
std::shared_lock lock(rw_lock);
40+
if (auto it = cached_types.find(full_name); //
41+
it != cached_types.end())
42+
return it->second;
43+
return nullptr;
4244
}
43-
// DataTypeFactory is a Singleton, so need to be protected by lock.
44-
DataTypePtr DataTypeFactory::getOrSet(const String & full_name)
45+
46+
void DataTypePtrCache::tryCache(const String & full_name, const DataTypePtr & datatype_ptr)
4547
{
46-
{
47-
std::shared_lock lock(rw_lock);
48-
auto it = fullname_types.find(full_name);
49-
if (it != fullname_types.end())
50-
{
51-
return it->second;
52-
}
53-
}
54-
ParserIdentifierWithOptionalParameters parser;
55-
ASTPtr ast = parseQuery(parser, full_name.data(), full_name.data() + full_name.size(), "data type", 0);
56-
DataTypePtr datatype_ptr = get(ast);
57-
// avoid big hashmap in rare cases.
48+
// It can not handle the situation that DataTypePtr sharing between
49+
// "Enum16('N' = 1, 'Y' = 2)" and "Enum16('Y' = 2, 'N' = 1)", but should
50+
// be good enough.
51+
// Avoid big hashmap in rare cases.
5852
std::unique_lock lock(rw_lock);
59-
if (fullname_types.size() < MAX_FULLNAME_TYPES)
53+
if (cached_types.size() < MAX_FULLNAME_TYPES)
6054
{
6155
// DataTypeEnum may generate too many full_name, so just skip inserting DataTypeEnum into fullname_types when
6256
// the capacity limit is almost reached, which ensures that most datatypes can be cached.
63-
if (fullname_types.size() > FULLNAME_TYPES_HIGH_WATER_MARK
57+
if (cached_types.size() > FULLNAME_TYPES_HIGH_WATER_MARK
6458
&& (datatype_ptr->getTypeId() == TypeIndex::Enum8 || datatype_ptr->getTypeId() == TypeIndex::Enum16))
6559
{
66-
return datatype_ptr;
60+
return;
6761
}
68-
fullname_types.emplace(full_name, datatype_ptr);
62+
cached_types.emplace(full_name, datatype_ptr);
6963
}
64+
}
65+
66+
DataTypePtr DataTypeFactory::get(const String & full_name) const
67+
{
68+
ParserIdentifierWithOptionalParameters parser;
69+
ASTPtr ast = parseQuery(parser, full_name.data(), full_name.data() + full_name.size(), "data type", 0);
70+
return get(ast);
71+
}
72+
73+
DataTypePtr DataTypeFactory::getOrSet(const ASTPtr & ast)
74+
{
75+
String owned_str_full_name(ast->range.first, ast->range.second);
76+
if (auto cached_ptr = fullname_types.get(owned_str_full_name); cached_ptr != nullptr)
77+
return cached_ptr;
78+
79+
auto datatype_ptr = get(ast);
80+
fullname_types.tryCache(owned_str_full_name, datatype_ptr);
7081
return datatype_ptr;
7182
}
7283

73-
size_t DataTypeFactory::getFullNameCacheSize() const
84+
DataTypePtr DataTypeFactory::getOrSet(const String & full_name)
7485
{
75-
std::shared_lock lock(rw_lock);
76-
return fullname_types.size();
86+
if (auto cached_ptr = fullname_types.get(full_name); cached_ptr != nullptr)
87+
return cached_ptr;
88+
89+
ParserIdentifierWithOptionalParameters parser;
90+
ASTPtr ast = parseQuery(parser, full_name.data(), full_name.data() + full_name.size(), "data type", 0);
91+
auto datatype_ptr = get(ast);
92+
fullname_types.tryCache(full_name, datatype_ptr);
93+
return datatype_ptr;
7794
}
7895

7996
DataTypePtr DataTypeFactory::get(const ASTPtr & ast) const
@@ -116,7 +133,7 @@ DataTypePtr DataTypeFactory::get(const String & family_name, const ASTPtr & para
116133
return it->second(parameters);
117134
}
118135

119-
throw Exception("Unknown data type family: " + family_name, ErrorCodes::UNKNOWN_TYPE);
136+
throw Exception(ErrorCodes::UNKNOWN_TYPE, "Unknown data type family: {}", family_name);
120137
}
121138

122139

@@ -127,21 +144,23 @@ void DataTypeFactory::registerDataType(
127144
{
128145
if (creator == nullptr)
129146
throw Exception(
130-
"DataTypeFactory: the data type family " + family_name + " has been provided a null constructor",
131-
ErrorCodes::LOGICAL_ERROR);
147+
ErrorCodes::LOGICAL_ERROR,
148+
"DataTypeFactory: the data type family {} has been provided a null constructor",
149+
family_name);
132150

133151
if (!data_types.emplace(family_name, creator).second)
134152
throw Exception(
135-
"DataTypeFactory: the data type family name '" + family_name + "' is not unique",
136-
ErrorCodes::LOGICAL_ERROR);
153+
ErrorCodes::LOGICAL_ERROR,
154+
"DataTypeFactory: the data type family name '{}' is not unique",
155+
family_name);
137156

138157
String family_name_lowercase = Poco::toLower(family_name);
139-
140158
if (case_sensitiveness == CaseInsensitive
141159
&& !case_insensitive_data_types.emplace(family_name_lowercase, creator).second)
142160
throw Exception(
143-
"DataTypeFactory: the case insensitive data type family name '" + family_name + "' is not unique",
144-
ErrorCodes::LOGICAL_ERROR);
161+
ErrorCodes::LOGICAL_ERROR,
162+
"DataTypeFactory: the case insensitive data type family name '{}' is not unique",
163+
family_name);
145164
}
146165

147166

@@ -152,16 +171,18 @@ void DataTypeFactory::registerSimpleDataType(
152171
{
153172
if (creator == nullptr)
154173
throw Exception(
155-
"DataTypeFactory: the data type " + name + " has been provided a null constructor",
156-
ErrorCodes::LOGICAL_ERROR);
174+
ErrorCodes::LOGICAL_ERROR,
175+
"DataTypeFactory: the data type {} has been provided a null constructor",
176+
name);
157177

158178
registerDataType(
159179
name,
160180
[name, creator](const ASTPtr & ast) {
161181
if (ast)
162182
throw Exception(
163-
"Data type " + name + " cannot have arguments",
164-
ErrorCodes::DATA_TYPE_CANNOT_HAVE_ARGUMENTS);
183+
ErrorCodes::DATA_TYPE_CANNOT_HAVE_ARGUMENTS,
184+
"Data type {} cannot have arguments",
185+
name);
165186
return creator();
166187
},
167188
case_sensitiveness);

dbms/src/DataTypes/DataTypeFactory.h

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,29 @@ using DataTypePtr = std::shared_ptr<const IDataType>;
3030
class IAST;
3131
using ASTPtr = std::shared_ptr<IAST>;
3232

33+
// When there are lots of tables created, the DataTypePtr could consume lots of
34+
// memory (tiflash#9947), or lots of CPU time to parse the string into IAST and
35+
// create the datatype (tiflash#6395). So we make a cache of FullName to
36+
// DataTypePtr in order to reuse the same DataTypePtr.
37+
class DataTypePtrCache
38+
{
39+
public:
40+
DataTypePtr get(const String & full_name) const;
41+
void tryCache(const String & full_name, const DataTypePtr & datatype_ptr);
42+
size_t getFullNameCacheSize() const
43+
{
44+
std::shared_lock lock(rw_lock);
45+
return cached_types.size();
46+
}
47+
48+
private:
49+
// full_name -> DataTypePtr
50+
using FullnameTypes = std::unordered_map<String, DataTypePtr>;
51+
static constexpr int MAX_FULLNAME_TYPES = 50000;
52+
static constexpr int FULLNAME_TYPES_HIGH_WATER_MARK = 49000;
53+
mutable std::shared_mutex rw_lock;
54+
FullnameTypes cached_types;
55+
};
3356

3457
/** Creates a data type by name of data type family and parameters.
3558
*/
@@ -40,16 +63,15 @@ class DataTypeFactory final : public ext::Singleton<DataTypeFactory>
4063
using SimpleCreator = std::function<DataTypePtr()>;
4164
// family_name -> Creator
4265
using DataTypesDictionary = std::unordered_map<String, Creator>;
43-
// full_name -> DataTypePtr
44-
using FullnameTypes = std::unordered_map<String, DataTypePtr>;
4566

4667
public:
4768
DataTypePtr get(const String & full_name) const;
4869
// In order to optimize the speed of generating data type instances, this will cache the full_name -> DataTypePtr.
4970
DataTypePtr getOrSet(const String & full_name);
71+
DataTypePtr getOrSet(const ASTPtr & ast);
5072
DataTypePtr get(const String & family_name, const ASTPtr & parameters) const;
5173
DataTypePtr get(const ASTPtr & ast) const;
52-
size_t getFullNameCacheSize() const;
74+
size_t getFullNameCacheSize() const { return fullname_types.getFullNameCacheSize(); }
5375

5476
/// For compatibility with SQL, it's possible to specify that certain data type name is case insensitive.
5577
enum CaseSensitiveness
@@ -76,10 +98,8 @@ class DataTypeFactory final : public ext::Singleton<DataTypeFactory>
7698
/// Case insensitive data types will be additionally added here with lowercased name.
7799
DataTypesDictionary case_insensitive_data_types;
78100

79-
static constexpr int MAX_FULLNAME_TYPES = 50000;
80-
static constexpr int FULLNAME_TYPES_HIGH_WATER_MARK = 49000;
81-
mutable std::shared_mutex rw_lock;
82-
FullnameTypes fullname_types;
101+
DataTypePtrCache fullname_types;
102+
83103
DataTypeFactory();
84104
friend class ext::Singleton<DataTypeFactory>;
85105
};

dbms/src/DataTypes/tests/gtest_data_type_get_common_type.cpp

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,13 @@
1616
#include <DataTypes/getLeastSupertype.h>
1717
#include <DataTypes/getMostSubtype.h>
1818
#include <DataTypes/isSupportedDataTypeCast.h>
19+
#include <Interpreters/InterpreterCreateQuery.h>
20+
#include <Parsers/ASTCreateQuery.h>
21+
#include <Parsers/ParserCreateQuery.h>
22+
#include <Parsers/parseQuery.h>
23+
#include <Storages/ColumnsDescription.h>
1924
#include <TestUtils/TiFlashTestBasic.h>
25+
#include <TestUtils/TiFlashTestEnv.h>
2026

2127
#include <sstream>
2228

@@ -333,5 +339,78 @@ try
333339
}
334340
CATCH
335341

342+
TEST(DataTypeTest, ParseColumns)
343+
{
344+
std::string table_def = R"(ATTACH TABLE ks_133501_t_10
345+
(
346+
s_0 String,
347+
s_1 String,
348+
s_2 String,
349+
e_0 Enum16('N' = 1, 'Y' = 2),
350+
e_1 Enum16('N' = 1, 'Y' = 2),
351+
e_2 Enum16('N' = 1, 'Y' = 2),
352+
e_3 Enum16('N' = 1, 'Y' = 2),
353+
e_4 Enum16('N' = 1, 'Y' = 2),
354+
e_5 Enum16('N' = 1, 'Y' = 2),
355+
e_6 Enum16('N' = 1, 'Y' = 2),
356+
e_7 Enum16('N' = 1, 'Y' = 2),
357+
e_8 Enum16('N' = 1, 'Y' = 2),
358+
e_9 Enum16('N' = 1, 'Y' = 2),
359+
e_10 Enum16('N' = 1, 'Y' = 2),
360+
e_11 Enum16('N' = 1, 'Y' = 2),
361+
e_12 Enum16('N' = 1, 'Y' = 2),
362+
e_13 Enum16('N' = 1, 'Y' = 2),
363+
e_14 Enum16('N' = 1, 'Y' = 2),
364+
e_15 Enum16('N' = 1, 'Y' = 2),
365+
e_16 Enum16('N' = 1, 'Y' = 2),
366+
e_17 Enum16('N' = 1, 'Y' = 2),
367+
e_18 Enum16('N' = 1, 'Y' = 2),
368+
_tidb_rowid Int64
369+
)
370+
ENGINE = DeltaMerge(_tidb_rowid, '{"cols":[{"id":1,"name":{"L":"s_0","O":"s_0"},"offset":0,"state":5,"type":{"Charset":"utf8mb4","Collate":"utf8mb4_bin","Decimal":0,"Flag":4099,"Flen":255,"Tp":254}},{"id":2,"name":{"L":"s_1","O":"s_1"},"offset":1,"state":5,"type":{"Charset":"utf8mb4","Collate":"utf8mb4_bin","Decimal":0,"Flag":4099,"Flen":64,"Tp":254}},{"id":3,"name":{"L":"s_2","O":"s_2"},"offset":2,"state":5,"type":{"Charset":"utf8mb4","Collate":"utf8mb4_bin","Decimal":0,"Flag":4099,"Flen":32,"Tp":254}},{"default":"N","default_bit":null,"id":4,"name":{"L":"e_0","O":"e_0"},"offset":3,"state":5,"type":{"Charset":"utf8mb4","Collate":"utf8mb4_bin","Decimal":0,"Elems":["N","Y"],"Flag":1,"Flen":1,"Tp":247}},{"default":"N","default_bit":null,"id":5,"name":{"L":"e_1","O":"e_1"},"offset":4,"state":5,"type":{"Charset":"utf8mb4","Collate":"utf8mb4_bin","Decimal":0,"Elems":["N","Y"],"Flag":1,"Flen":1,"Tp":247}},{"default":"N","default_bit":null,"id":6,"name":{"L":"e_2","O":"e_2"},"offset":5,"state":5,"type":{"Charset":"utf8mb4","Collate":"utf8mb4_bin","Decimal":0,"Elems":["N","Y"],"Flag":1,"Flen":1,"Tp":247}},{"default":"N","default_bit":null,"id":7,"name":{"L":"e_3","O":"e_3"},"offset":6,"state":5,"type":{"Charset":"utf8mb4","Collate":"utf8mb4_bin","Decimal":0,"Elems":["N","Y"],"Flag":1,"Flen":1,"Tp":247}},{"default":"N","default_bit":null,"id":8,"name":{"L":"e_4","O":"e_4"},"offset":7,"state":5,"type":{"Charset":"utf8mb4","Collate":"utf8mb4_bin","Decimal":0,"Elems":["N","Y"],"Flag":1,"Flen":1,"Tp":247}},{"default":"N","default_bit":null,"id":9,"name":{"L":"e_5","O":"e_5"},"offset":8,"state":5,"type":{"Charset":"utf8mb4","Collate":"utf8mb4_bin","Decimal":0,"Elems":["N","Y"],"Flag":1,"Flen":1,"Tp":247}},{"default":"N","default_bit":null,"id":10,"name":{"L":"e_6","O":"e_6"},"offset":9,"state":5,"type":{"Charset":"utf8mb4","Collate":"utf8mb4_bin","Decimal":0,"Elems":["N","Y"],"Flag":1,"Flen":1,"Tp":247}},{"default":"N","default_bit":null,"id":11,"name":{"L":"e_7","O":"e_7"},"offset":10,"state":5,"type":{"Charset":"utf8mb4","Collate":"utf8mb4_bin","Decimal":0,"Elems":["N","Y"],"Flag":1,"Flen":1,"Tp":247}},{"default":"N","default_bit":null,"id":12,"name":{"L":"e_8","O":"e_8"},"offset":11,"state":5,"type":{"Charset":"utf8mb4","Collate":"utf8mb4_bin","Decimal":0,"Elems":["N","Y"],"Flag":1,"Flen":1,"Tp":247}},{"default":"N","default_bit":null,"id":13,"name":{"L":"e_9","O":"e_9"},"offset":12,"state":5,"type":{"Charset":"utf8mb4","Collate":"utf8mb4_bin","Decimal":0,"Elems":["N","Y"],"Flag":1,"Flen":1,"Tp":247}},{"default":"N","default_bit":null,"id":14,"name":{"L":"e_10","O":"e_10"},"offset":13,"state":5,"type":{"Charset":"utf8mb4","Collate":"utf8mb4_bin","Decimal":0,"Elems":["N","Y"],"Flag":1,"Flen":1,"Tp":247}},{"default":"N","default_bit":null,"id":15,"name":{"L":"e_11","O":"e_11"},"offset":14,"state":5,"type":{"Charset":"utf8mb4","Collate":"utf8mb4_bin","Decimal":0,"Elems":["N","Y"],"Flag":1,"Flen":1,"Tp":247}},{"default":"N","default_bit":null,"id":16,"name":{"L":"e_12","O":"e_12"},"offset":15,"state":5,"type":{"Charset":"utf8mb4","Collate":"utf8mb4_bin","Decimal":0,"Elems":["N","Y"],"Flag":1,"Flen":1,"Tp":247}},{"default":"N","default_bit":null,"id":17,"name":{"L":"e_13","O":"e_13"},"offset":16,"state":5,"type":{"Charset":"utf8mb4","Collate":"utf8mb4_bin","Decimal":0,"Elems":["N","Y"],"Flag":1,"Flen":1,"Tp":247}},{"default":"N","default_bit":null,"id":18,"name":{"L":"e_14","O":"e_14"},"offset":17,"state":5,"type":{"Charset":"utf8mb4","Collate":"utf8mb4_bin","Decimal":0,"Elems":["N","Y"],"Flag":1,"Flen":1,"Tp":247}},{"default":"N","default_bit":null,"id":19,"name":{"L":"e_15","O":"e_15"},"offset":18,"state":5,"type":{"Charset":"utf8mb4","Collate":"utf8mb4_bin","Decimal":0,"Elems":["N","Y"],"Flag":1,"Flen":1,"Tp":247}},{"default":"N","default_bit":null,"id":20,"name":{"L":"e_16","O":"e_16"},"offset":19,"state":5,"type":{"Charset":"utf8mb4","Collate":"utf8mb4_bin","Decimal":0,"Elems":["N","Y"],"Flag":1,"Flen":1,"Tp":247}},{"default":"N","default_bit":null,"id":21,"name":{"L":"e_17","O":"e_17"},"offset":20,"state":5,"type":{"Charset":"utf8mb4","Collate":"utf8mb4_bin","Decimal":0,"Elems":["N","Y"],"Flag":1,"Flen":1,"Tp":247}},{"default":"N","default_bit":null,"id":22,"name":{"L":"e_18","O":"e_18"},"offset":21,"state":5,"type":{"Charset":"utf8mb4","Collate":"utf8mb4_bin","Decimal":0,"Elems":["N","Y"],"Flag":1,"Flen":1,"Tp":247}}],"id":10,"index_info":[{"id":1,"idx_cols":[{"length":-1,"name":{"L":"s_0","O":"s_0"},"offset":0},{"length":-1,"name":{"L":"s_1","O":"s_1"},"offset":1},{"length":-1,"name":{"L":"s_2","O":"s_2"},"offset":2}],"idx_name":{"L":"primary","O":"primary"},"index_type":1,"is_global":false,"is_invisible":false,"is_primary":true,"is_unique":true,"state":5}],"is_common_handle":false,"keyspace_id":133501,"name":{"L":"s_1","O":"s_1"},"partition":null,"pk_is_handle":false,"schema_version":-1,"state":5,"tiflash_replica":{"Count":0},"update_timestamp":456193504796999692}', 0))";
371+
372+
auto & global_context = TiFlashTestEnv::getGlobalContext();
373+
374+
std::vector<ColumnsDescription> parsed_columns;
375+
for (size_t i = 0; i < 1000; ++i)
376+
{
377+
ParserCreateQuery parser;
378+
ASTPtr ast = parseQuery(parser, table_def.data(), table_def.data() + table_def.size(), "", 0);
379+
ASTCreateQuery & ast_create_query = typeid_cast<ASTCreateQuery &>(*ast);
380+
ColumnsDescription columns
381+
= InterpreterCreateQuery::getColumnsDescription(*ast_create_query.columns, global_context);
382+
parsed_columns.emplace_back(columns);
383+
}
384+
ASSERT_EQ(parsed_columns.size(), 1000);
385+
386+
auto str_type_ptr = parsed_columns[0].getAll().filter(Names{"s_0"}).begin()->type;
387+
auto enum_type_ptr = parsed_columns[0].getAll().filter(Names{"e_0"}).begin()->type;
388+
for (size_t i = 1; i < parsed_columns.size(); ++i)
389+
{
390+
auto enum_cols = parsed_columns[i].getAll().filter(Names{
391+
"e_0", "e_1", "e_2", "e_3", "e_4", "e_5", "e_6", "e_7", "e_8", "e_9",
392+
"e_10", "e_11", "e_12", "e_13", "e_14", "e_15", "e_16", "e_17", "e_18",
393+
});
394+
ASSERT_EQ(enum_cols.size(), 19);
395+
for (const auto & col : enum_cols)
396+
{
397+
ASSERT_TRUE(col.type->equals(*enum_type_ptr));
398+
// they must share the same ptr, otherwise the memory consumption is too high when there are
399+
// many columns with the same type
400+
ASSERT_EQ(col.type.get(), enum_type_ptr.get());
401+
}
402+
403+
auto str_cols = parsed_columns[i].getAll().filter(Names{"s_0", "s_1", "s_2"});
404+
ASSERT_EQ(str_cols.size(), 3);
405+
for (const auto & col : str_cols)
406+
{
407+
ASSERT_TRUE(col.type->equals(*str_type_ptr));
408+
// they must share the same ptr, otherwise the memory consumption is too high when there are
409+
// many columns with the same type
410+
ASSERT_EQ(col.type.get(), str_type_ptr.get());
411+
}
412+
}
413+
}
414+
336415
} // namespace tests
337416
} // namespace DB

dbms/src/Interpreters/InterpreterCreateQuery.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ static ColumnsAndDefaults parseColumns(const ASTExpressionList & column_list_ast
219219

220220
if (col_decl.type)
221221
{
222-
columns.emplace_back(col_decl.name, DataTypeFactory::instance().get(col_decl.type));
222+
columns.emplace_back(col_decl.name, DataTypeFactory::instance().getOrSet(col_decl.type));
223223
}
224224
else
225225
/// we're creating dummy DataTypeUInt8 in order to prevent the NullPointerException in ExpressionActions
@@ -295,7 +295,8 @@ static ColumnsAndDefaults parseColumns(const ASTExpressionList & column_list_ast
295295
column_name,
296296
ColumnDefault{
297297
columnDefaultKindFromString(col_decl_ptr->default_specifier),
298-
col_decl_ptr->default_expression});
298+
col_decl_ptr->default_expression,
299+
});
299300
}
300301
}
301302

dbms/src/Parsers/StringRange.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
#include <Core/Types.h>
1818
#include <Parsers/TokenIterator.h>
1919

20-
#include <map>
2120
#include <memory>
2221

2322

@@ -29,7 +28,7 @@ struct StringRange
2928
const char * first = nullptr;
3029
const char * second = nullptr;
3130

32-
StringRange() {}
31+
StringRange() = default;
3332
StringRange(const char * begin, const char * end)
3433
: first(begin)
3534
, second(end)

dbms/src/Storages/DeltaMerge/tests/gtest_key_range.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,11 +173,11 @@ TEST(RowKey, ToNextKeyIntHandle)
173173
EXPECT_EQ(0, compare(next.toRowKeyValueRef(), range.getEnd()));
174174
EXPECT_EQ(range.getEnd().toDebugString(), "21");
175175
}
176-
// any suffix will be regarded as Key=21 in RowKeyRange::fromRegionRange.
176+
// any non-empty suffix will be regarded as Key=21 in RowKeyRange::fromRegionRange.
177177
{
178178
auto key_end = RecordKVFormat::genRawKey(table_id, 20);
179179
std::mt19937_64 rand_gen(std::random_device{}());
180-
size_t rand_length = rand_gen() % 255;
180+
size_t rand_length = std::min(1, rand_gen() % 255); // ensure rand_length is at least 1
181181
auto rand_suffix = DB::random::randomString(rand_length);
182182
key_end.insert(key_end.end(), rand_suffix.begin(), rand_suffix.end());
183183
LOG_INFO(

dbms/src/Storages/StorageDeltaMerge.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ void StorageDeltaMerge::updateTableColumnInfo()
126126
{
127127
const ColumnsDescription & columns = getColumns();
128128

129-
LOG_INFO(
129+
LOG_DEBUG(
130130
log,
131131
"updateTableColumnInfo, table_name={} ordinary=\"{}\" materialized=\"{}\"",
132132
table_column_info->table_name,
@@ -231,6 +231,7 @@ void StorageDeltaMerge::updateTableColumnInfo()
231231
}
232232
table_column_defines.push_back(col_def);
233233
}
234+
table_column_defines.shrink_to_fit();
234235

235236
if (!new_columns.materialized.contains(VERSION_COLUMN_NAME))
236237
{

0 commit comments

Comments
 (0)