Skip to content

Commit db4c235

Browse files
alexey-milovidovalexey-milovidov
authored
Merge pull request #10569 from zhang2014/fix/ISSUES-10551
ISSUES-10551 add backward compatibility for create bloom filter index
2 parents 250a44a + 404452b commit db4c235

11 files changed

Lines changed: 64 additions & 28 deletions

‎src/Storages/MergeTree/MergeTreeData.cpp‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ MergeTreeData::MergeTreeData(
146146
throw Exception("MergeTree storages require data path", ErrorCodes::INCORRECT_FILE_NAME);
147147

148148
const auto settings = getSettings();
149-
setProperties(metadata);
149+
setProperties(metadata, /*only_check*/ false, attach);
150150

151151
/// NOTE: using the same columns list as is read when performing actual merges.
152152
merging_params.check(getColumns().getAllPhysical());
@@ -305,7 +305,7 @@ static void checkKeyExpression(const ExpressionActions & expr, const Block & sam
305305
}
306306
}
307307

308-
void MergeTreeData::setProperties(const StorageInMemoryMetadata & metadata, bool only_check)
308+
void MergeTreeData::setProperties(const StorageInMemoryMetadata & metadata, bool only_check, bool attach)
309309
{
310310
if (!metadata.order_by_ast)
311311
throw Exception("ORDER BY cannot be empty", ErrorCodes::BAD_ARGUMENTS);
@@ -432,8 +432,9 @@ void MergeTreeData::setProperties(const StorageInMemoryMetadata & metadata, bool
432432
new_indices.push_back(
433433
MergeTreeIndexFactory::instance().get(
434434
all_columns,
435-
std::dynamic_pointer_cast<ASTIndexDeclaration>(index_decl->clone()),
436-
global_context));
435+
std::dynamic_pointer_cast<ASTIndexDeclaration>(index_decl),
436+
global_context,
437+
attach));
437438

438439
if (indices_names.find(new_indices.back()->name) != indices_names.end())
439440
throw Exception(

‎src/Storages/MergeTree/MergeTreeData.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -848,7 +848,7 @@ class MergeTreeData : public IStorage
848848
/// The same for clearOldTemporaryDirectories.
849849
std::mutex clear_old_temporary_directories_mutex;
850850

851-
void setProperties(const StorageInMemoryMetadata & metadata, bool only_check = false);
851+
void setProperties(const StorageInMemoryMetadata & metadata, bool only_check = false, bool attach = false);
852852

853853
void initPartitionKey();
854854

‎src/Storages/MergeTree/MergeTreeIndexBloomFilter.cpp‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ static void assertIndexColumnsType(const Block & header)
8888
}
8989

9090
std::unique_ptr<IMergeTreeIndex> bloomFilterIndexCreatorNew(
91-
const NamesAndTypesList & columns, std::shared_ptr<ASTIndexDeclaration> node, const Context & context)
91+
const NamesAndTypesList & columns, std::shared_ptr<ASTIndexDeclaration> node, const Context & context, bool attach)
9292
{
9393
if (node->name.empty())
9494
throw Exception("Index must have unique name.", ErrorCodes::INCORRECT_QUERY);
@@ -105,19 +105,29 @@ std::unique_ptr<IMergeTreeIndex> bloomFilterIndexCreatorNew(
105105
const auto & arguments = node->type->arguments;
106106

107107
if (arguments && arguments->children.size() > 1)
108-
throw Exception("BloomFilter index cannot have more than one parameter.", ErrorCodes::NUMBER_OF_ARGUMENTS_DOESNT_MATCH);
108+
{
109+
if (!attach) /// This is for backward compatibility.
110+
throw Exception("BloomFilter index cannot have more than one parameter.", ErrorCodes::NUMBER_OF_ARGUMENTS_DOESNT_MATCH);
111+
112+
arguments->children.erase(++arguments->children.begin(), arguments->children.end());
113+
}
114+
109115

110116
if (arguments && !arguments->children.empty())
111117
{
112-
const auto & argument = arguments->children[0]->as<ASTLiteral>();
118+
auto * argument = arguments->children[0]->as<ASTLiteral>();
113119

114120
if (!argument || (argument->value.safeGet<Float64>() < 0 || argument->value.safeGet<Float64>() > 1))
115-
throw Exception("The BloomFilter false positive must be a double number between 0 and 1.", ErrorCodes::BAD_ARGUMENTS);
121+
{
122+
if (!attach || !argument) /// This is for backward compatibility.
123+
throw Exception("The BloomFilter false positive must be a double number between 0 and 1.", ErrorCodes::BAD_ARGUMENTS);
124+
125+
argument->value = Field(std::min(Float64(1), std::max(argument->value.safeGet<Float64>(), Float64(0))));
126+
}
116127

117128
max_conflict_probability = argument->value.safeGet<Float64>();
118129
}
119130

120-
121131
const auto & bits_per_row_and_size_of_hash_functions = BloomFilterHash::calculationBestPractices(max_conflict_probability);
122132

123133
return std::make_unique<MergeTreeIndexBloomFilter>(

‎src/Storages/MergeTree/MergeTreeIndexFullText.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -748,7 +748,8 @@ bool SplitTokenExtractor::nextLike(const String & str, size_t * pos, String & to
748748
std::unique_ptr<IMergeTreeIndex> bloomFilterIndexCreator(
749749
const NamesAndTypesList & new_columns,
750750
std::shared_ptr<ASTIndexDeclaration> node,
751-
const Context & context)
751+
const Context & context,
752+
bool /*attach*/)
752753
{
753754
if (node->name.empty())
754755
throw Exception("Index must have unique name", ErrorCodes::INCORRECT_QUERY);

‎src/Storages/MergeTree/MergeTreeIndexMinMax.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,8 @@ bool MergeTreeIndexMinMax::mayBenefitFromIndexForIn(const ASTPtr & node) const
183183
std::unique_ptr<IMergeTreeIndex> minmaxIndexCreator(
184184
const NamesAndTypesList & new_columns,
185185
std::shared_ptr<ASTIndexDeclaration> node,
186-
const Context & context)
186+
const Context & context,
187+
bool /*attach*/)
187188
{
188189
if (node->name.empty())
189190
throw Exception("Index must have unique name", ErrorCodes::INCORRECT_QUERY);

‎src/Storages/MergeTree/MergeTreeIndexSet.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,8 @@ bool MergeTreeIndexSet::mayBenefitFromIndexForIn(const ASTPtr &) const
458458
std::unique_ptr<IMergeTreeIndex> setIndexCreator(
459459
const NamesAndTypesList & new_columns,
460460
std::shared_ptr<ASTIndexDeclaration> node,
461-
const Context & context)
461+
const Context & context,
462+
bool /*attach*/)
462463
{
463464
if (node->name.empty())
464465
throw Exception("Index must have unique name", ErrorCodes::INCORRECT_QUERY);

‎src/Storages/MergeTree/MergeTreeIndices.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,14 @@ void MergeTreeIndexFactory::registerIndex(const std::string & name, Creator crea
2828
std::unique_ptr<IMergeTreeIndex> MergeTreeIndexFactory::get(
2929
const NamesAndTypesList & columns,
3030
std::shared_ptr<ASTIndexDeclaration> node,
31-
const Context & context) const
31+
const Context & context,
32+
bool attach) const
3233
{
3334
if (!node->type)
34-
throw Exception(
35-
"for index TYPE is required", ErrorCodes::INCORRECT_QUERY);
35+
throw Exception("TYPE is required for index", ErrorCodes::INCORRECT_QUERY);
36+
3637
if (node->type->parameters && !node->type->parameters->children.empty())
37-
throw Exception(
38-
"Index type can not have parameters", ErrorCodes::INCORRECT_QUERY);
38+
throw Exception("Index type cannot have parameters", ErrorCodes::INCORRECT_QUERY);
3939

4040
boost::algorithm::to_lower(node->type->name);
4141
auto it = indexes.find(node->type->name);
@@ -52,7 +52,7 @@ std::unique_ptr<IMergeTreeIndex> MergeTreeIndexFactory::get(
5252
}),
5353
ErrorCodes::INCORRECT_QUERY);
5454

55-
return it->second(columns, node, context);
55+
return it->second(columns, node, context, attach);
5656
}
5757

5858
MergeTreeIndexFactory::MergeTreeIndexFactory()

‎src/Storages/MergeTree/MergeTreeIndices.h‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -137,12 +137,14 @@ class MergeTreeIndexFactory : private boost::noncopyable
137137
std::unique_ptr<IMergeTreeIndex>(
138138
const NamesAndTypesList & columns,
139139
std::shared_ptr<ASTIndexDeclaration> node,
140-
const Context & context)>;
140+
const Context & context,
141+
bool attach)>;
141142

142143
std::unique_ptr<IMergeTreeIndex> get(
143144
const NamesAndTypesList & columns,
144145
std::shared_ptr<ASTIndexDeclaration> node,
145-
const Context & context) const;
146+
const Context & context,
147+
bool attach) const;
146148

147149
void registerIndex(const std::string & name, Creator creator);
148150

@@ -159,21 +161,25 @@ class MergeTreeIndexFactory : private boost::noncopyable
159161
std::unique_ptr<IMergeTreeIndex> minmaxIndexCreator(
160162
const NamesAndTypesList & columns,
161163
std::shared_ptr<ASTIndexDeclaration> node,
162-
const Context & context);
164+
const Context & context,
165+
bool attach);
163166

164167
std::unique_ptr<IMergeTreeIndex> setIndexCreator(
165168
const NamesAndTypesList & columns,
166169
std::shared_ptr<ASTIndexDeclaration> node,
167-
const Context & context);
170+
const Context & context,
171+
bool attach);
168172

169173
std::unique_ptr<IMergeTreeIndex> bloomFilterIndexCreator(
170174
const NamesAndTypesList & columns,
171175
std::shared_ptr<ASTIndexDeclaration> node,
172-
const Context & context);
176+
const Context & context,
177+
bool attach);
173178

174179
std::unique_ptr<IMergeTreeIndex> bloomFilterIndexCreatorNew(
175180
const NamesAndTypesList & columns,
176181
std::shared_ptr<ASTIndexDeclaration> node,
177-
const Context & context);
182+
const Context & context,
183+
bool attach);
178184

179185
}

‎src/Storages/MergeTree/registerStorageMergeTree.cpp‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -604,9 +604,8 @@ static StoragePtr create(const StorageFactory::Arguments & args)
604604

605605

606606
if (args.query.columns_list && args.query.columns_list->indices)
607-
for (const auto & index : args.query.columns_list->indices->children)
608-
indices_description.indices.push_back(
609-
std::dynamic_pointer_cast<ASTIndexDeclaration>(index->clone()));
607+
for (auto & index : args.query.columns_list->indices->children)
608+
indices_description.indices.push_back(std::dynamic_pointer_cast<ASTIndexDeclaration>(index));
610609

611610
storage_settings->loadFromQuery(*args.storage_def);
612611

‎tests/queries/0_stateless/01249_bad_arguments_for_bloom_filter.reference‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
CREATE TABLE default.bloom_filter_idx_good\n(\n `u64` UInt64, \n `i32` Int32, \n `f64` Float64, \n `d` Decimal(10, 2), \n `s` String, \n `e` Enum8(\'a\' = 1, \'b\' = 2, \'c\' = 3), \n `dt` Date, \n INDEX bloom_filter_a i32 TYPE bloom_filter(0.) GRANULARITY 1\n)\nENGINE = MergeTree()\nORDER BY u64\nSETTINGS index_granularity = 8192
2+
CREATE TABLE default.bloom_filter_idx_good\n(\n `u64` UInt64, \n `i32` Int32, \n `f64` Float64, \n `d` Decimal(10, 2), \n `s` String, \n `e` Enum8(\'a\' = 1, \'b\' = 2, \'c\' = 3), \n `dt` Date, \n INDEX bloom_filter_a i32 TYPE bloom_filter(0.) GRANULARITY 1\n)\nENGINE = MergeTree()\nORDER BY u64\nSETTINGS index_granularity = 8192
3+
CREATE TABLE default.bloom_filter_idx_good\n(\n `u64` UInt64, \n `i32` Int32, \n `f64` Float64, \n `d` Decimal(10, 2), \n `s` String, \n `e` Enum8(\'a\' = 1, \'b\' = 2, \'c\' = 3), \n `dt` Date, \n INDEX bloom_filter_a i32 TYPE bloom_filter(1.) GRANULARITY 1\n)\nENGINE = MergeTree()\nORDER BY u64\nSETTINGS index_granularity = 8192

0 commit comments

Comments
 (0)