From 6b0498cd93bdb62f19d18052f6df271d4930b53a Mon Sep 17 00:00:00 2001 From: alesapin Date: Wed, 3 Jul 2019 11:49:52 +0300 Subject: [PATCH 01/62] First version of check cmd --- dbms/src/Storages/StorageMergeTree.cpp | 9 +++++++++ dbms/src/Storages/StorageMergeTree.h | 2 ++ dbms/src/Storages/StorageReplicatedMergeTree.cpp | 2 +- 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/dbms/src/Storages/StorageMergeTree.cpp b/dbms/src/Storages/StorageMergeTree.cpp index 87526e3f39d..bc6bb4d1080 100644 --- a/dbms/src/Storages/StorageMergeTree.cpp +++ b/dbms/src/Storages/StorageMergeTree.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -1116,4 +1117,12 @@ ActionLock StorageMergeTree::getActionLock(StorageActionBlockType action_type) return {}; } +bool StorageMergeTree::checkData() const +{ + DataPartsVector data_parts = getDataPartsVector(); + for (auto & part : data_parts) + checkDataPart(part, true, primary_key_data_types, skip_indices); + return true; +} + } diff --git a/dbms/src/Storages/StorageMergeTree.h b/dbms/src/Storages/StorageMergeTree.h index b5156ce7137..90b02c29994 100644 --- a/dbms/src/Storages/StorageMergeTree.h +++ b/dbms/src/Storages/StorageMergeTree.h @@ -70,6 +70,8 @@ public: String getDataPath() const override { return full_path; } + bool checkData() const override; + private: String path; diff --git a/dbms/src/Storages/StorageReplicatedMergeTree.cpp b/dbms/src/Storages/StorageReplicatedMergeTree.cpp index 86298df3f19..576857f98da 100644 --- a/dbms/src/Storages/StorageReplicatedMergeTree.cpp +++ b/dbms/src/Storages/StorageReplicatedMergeTree.cpp @@ -2380,7 +2380,7 @@ void StorageReplicatedMergeTree::removePartAndEnqueueFetch(const String & part_n auto results = zookeeper->multi(ops); - String path_created = dynamic_cast(*results[0]).path_created; + String path_created = dynamic_cast(*results.back()).path_created; log_entry->znode_name = path_created.substr(path_created.find_last_of('/') + 1); queue.insert(zookeeper, log_entry); } From 3925a3bd13a79f405daa167c63d9c2a2204eb426 Mon Sep 17 00:00:00 2001 From: alesapin Date: Wed, 3 Jul 2019 16:17:19 +0300 Subject: [PATCH 02/62] Generalize check query --- dbms/src/Common/FileChecker.cpp | 18 ++++--- dbms/src/Common/FileChecker.h | 3 +- .../Interpreters/InterpreterCheckQuery.cpp | 44 +++++++++++++--- dbms/src/Parsers/ASTCheckQuery.h | 3 ++ dbms/src/Parsers/ParserCheckQuery.cpp | 15 ++++-- dbms/src/Storages/IStorage.h | 3 +- .../ReplicatedMergeTreePartCheckThread.cpp | 15 ++++-- .../ReplicatedMergeTreePartCheckThread.h | 5 +- dbms/src/Storages/StorageLog.cpp | 2 +- dbms/src/Storages/StorageLog.h | 2 +- dbms/src/Storages/StorageMergeTree.cpp | 52 +++++++++++++++++-- dbms/src/Storages/StorageMergeTree.h | 2 +- .../Storages/StorageReplicatedMergeTree.cpp | 28 ++++++++++ .../src/Storages/StorageReplicatedMergeTree.h | 2 + dbms/src/Storages/StorageStripeLog.cpp | 2 +- dbms/src/Storages/StorageStripeLog.h | 2 +- dbms/src/Storages/StorageTinyLog.cpp | 3 +- dbms/src/Storages/StorageTinyLog.h | 2 +- 18 files changed, 166 insertions(+), 37 deletions(-) diff --git a/dbms/src/Common/FileChecker.cpp b/dbms/src/Common/FileChecker.cpp index d196c703e36..8a372aea9a0 100644 --- a/dbms/src/Common/FileChecker.cpp +++ b/dbms/src/Common/FileChecker.cpp @@ -42,35 +42,39 @@ void FileChecker::update(const Files::const_iterator & begin, const Files::const save(); } -bool FileChecker::check() const +CheckResults FileChecker::check() const { /** Read the files again every time you call `check` - so as not to violate the constancy. * `check` method is rarely called. */ + + CheckResults results; Map local_map; load(local_map, files_info_path); if (local_map.empty()) - return true; + return {}; for (const auto & name_size : local_map) { Poco::File file(Poco::Path(files_info_path).parent().toString() + "/" + name_size.first); if (!file.exists()) { - LOG_ERROR(log, "File " << file.path() << " doesn't exist"); - return false; + results.emplace_back(file.path(), false, "File " + file.path() + " doesn't exist"); + break; } + size_t real_size = file.getSize(); if (real_size != name_size.second) { - LOG_ERROR(log, "Size of " << file.path() << " is wrong. Size is " << real_size << " but should be " << name_size.second); - return false; + results.emplace_back(file.path(), false, "Size of " + file.path() + " is wrong. Size is " + toString(real_size) + " but should be " + toString(name_size.second)); + break; } + results.emplace_back(file.path(), true, ""); } - return true; + return results; } void FileChecker::initialize() diff --git a/dbms/src/Common/FileChecker.h b/dbms/src/Common/FileChecker.h index 26167826888..bd69867b11c 100644 --- a/dbms/src/Common/FileChecker.h +++ b/dbms/src/Common/FileChecker.h @@ -3,6 +3,7 @@ #include #include #include +#include namespace DB @@ -24,7 +25,7 @@ public: void update(const Files::const_iterator & begin, const Files::const_iterator & end); /// Check the files whose parameters are specified in sizes.json - bool check() const; + CheckResults check() const; private: void initialize(); diff --git a/dbms/src/Interpreters/InterpreterCheckQuery.cpp b/dbms/src/Interpreters/InterpreterCheckQuery.cpp index c99c74fa33a..3edcc12fedc 100644 --- a/dbms/src/Interpreters/InterpreterCheckQuery.cpp +++ b/dbms/src/Interpreters/InterpreterCheckQuery.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include @@ -11,6 +12,21 @@ namespace DB { +namespace +{ + +NamesAndTypes getBlockStructure() +{ + return { + {"part_path", std::make_shared()}, + {"is_passed", std::make_shared()}, + {"message", std::make_shared()}, + }; +} + +} + + InterpreterCheckQuery::InterpreterCheckQuery(const ASTPtr & query_ptr_, const Context & context_) : query_ptr(query_ptr_), context(context_) { @@ -19,18 +35,32 @@ InterpreterCheckQuery::InterpreterCheckQuery(const ASTPtr & query_ptr_, const Co BlockIO InterpreterCheckQuery::execute() { - const auto & alter = query_ptr->as(); - const String & table_name = alter.table; - String database_name = alter.database.empty() ? context.getCurrentDatabase() : alter.database; + const auto & check = query_ptr->as(); + const String & table_name = check.table; + String database_name = check.database.empty() ? context.getCurrentDatabase() : check.database; StoragePtr table = context.getTable(database_name, table_name); + auto check_results = table->checkData(query_ptr, context); - auto column = ColumnUInt8::create(); - column->insertValue(UInt64(table->checkData())); - result = Block{{ std::move(column), std::make_shared(), "result" }}; + auto block_structure = getBlockStructure(); + auto path_column = block_structure[0].type->createColumn(); + auto is_passed_column = block_structure[1].type->createColumn(); + auto message_column = block_structure[2].type->createColumn(); + + for (const auto & check_result : check_results) + { + path_column->insert(check_result.fs_path); + is_passed_column->insert(static_cast(check_result.success)); + message_column->insert(check_result.failure_message); + } + + Block block({ + {std::move(path_column), block_structure[0].type, block_structure[0].name}, + {std::move(is_passed_column), block_structure[1].type, block_structure[1].name}, + {std::move(message_column), block_structure[2].type, block_structure[2].name}}); BlockIO res; - res.in = std::make_shared(result); + res.in = std::make_shared(block); return res; } diff --git a/dbms/src/Parsers/ASTCheckQuery.h b/dbms/src/Parsers/ASTCheckQuery.h index 595b6c2ecb6..93a69afc368 100644 --- a/dbms/src/Parsers/ASTCheckQuery.h +++ b/dbms/src/Parsers/ASTCheckQuery.h @@ -7,6 +7,9 @@ namespace DB struct ASTCheckQuery : public ASTQueryWithTableAndOutput { + + ASTPtr partition; + /** Get the text that identifies this element. */ String getID(char delim) const override { return "CheckQuery" + (delim + database) + delim + table; } diff --git a/dbms/src/Parsers/ParserCheckQuery.cpp b/dbms/src/Parsers/ParserCheckQuery.cpp index cd25e60b887..5ba8119571d 100644 --- a/dbms/src/Parsers/ParserCheckQuery.cpp +++ b/dbms/src/Parsers/ParserCheckQuery.cpp @@ -3,6 +3,7 @@ #include #include #include +#include namespace DB @@ -11,9 +12,11 @@ namespace DB bool ParserCheckQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) { ParserKeyword s_check_table("CHECK TABLE"); + ParserKeyword s_partition("PARTITION"); ParserToken s_dot(TokenType::Dot); ParserIdentifier table_parser; + ParserPartition partition_parser; ASTPtr table; ASTPtr database; @@ -23,24 +26,28 @@ bool ParserCheckQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) if (!table_parser.parse(pos, database, expected)) return false; + auto query = std::make_shared(); if (s_dot.ignore(pos)) { if (!table_parser.parse(pos, table, expected)) return false; - auto query = std::make_shared(); getIdentifierName(database, query->database); getIdentifierName(table, query->table); - node = query; } else { table = database; - auto query = std::make_shared(); getIdentifierName(table, query->table); - node = query; } + if (s_partition.ignore(pos, expected)) + { + if (!partition_parser.parse(pos, query->partition, expected)) + return false; + } + + node = query; return true; } diff --git a/dbms/src/Storages/IStorage.h b/dbms/src/Storages/IStorage.h index 5bfd8224372..da32b7aa74a 100644 --- a/dbms/src/Storages/IStorage.h +++ b/dbms/src/Storages/IStorage.h @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -285,7 +286,7 @@ public: virtual bool mayBenefitFromIndexForIn(const ASTPtr & /* left_in_operand */, const Context & /* query_context */) const { return false; } /// Checks validity of the data - virtual bool checkData() const { throw Exception("Check query is not supported for " + getName() + " storage", ErrorCodes::NOT_IMPLEMENTED); } + virtual CheckResults checkData(const ASTPtr & /* query */, const Context & /* context */) { throw Exception("Check query is not supported for " + getName() + " storage", ErrorCodes::NOT_IMPLEMENTED); } /// Checks that table could be dropped right now /// Otherwise - throws an exception with detailed information. diff --git a/dbms/src/Storages/MergeTree/ReplicatedMergeTreePartCheckThread.cpp b/dbms/src/Storages/MergeTree/ReplicatedMergeTreePartCheckThread.cpp index b06dea30052..045aa8a6461 100644 --- a/dbms/src/Storages/MergeTree/ReplicatedMergeTreePartCheckThread.cpp +++ b/dbms/src/Storages/MergeTree/ReplicatedMergeTreePartCheckThread.cpp @@ -181,7 +181,7 @@ void ReplicatedMergeTreePartCheckThread::searchForMissingPart(const String & par } -void ReplicatedMergeTreePartCheckThread::checkPart(const String & part_name) +CheckResult ReplicatedMergeTreePartCheckThread::checkPart(const String & part_name) { LOG_WARNING(log, "Checking part " << part_name); ProfileEvents::increment(ProfileEvents::ReplicatedPartChecks); @@ -197,6 +197,7 @@ void ReplicatedMergeTreePartCheckThread::checkPart(const String & part_name) if (!part) { searchForMissingPart(part_name); + return {part_name, false, "Part is missing, will search for it"}; } /// We have this part, and it's active. We will check whether we need this part and whether it has the right data. else if (part->name == part_name) @@ -242,7 +243,7 @@ void ReplicatedMergeTreePartCheckThread::checkPart(const String & part_name) if (need_stop) { LOG_INFO(log, "Checking part was cancelled."); - return; + return {part_name, false, "Checking part was cancelled"}; } LOG_INFO(log, "Part " << part_name << " looks good."); @@ -253,13 +254,15 @@ void ReplicatedMergeTreePartCheckThread::checkPart(const String & part_name) tryLogCurrentException(log, __PRETTY_FUNCTION__); - LOG_ERROR(log, "Part " << part_name << " looks broken. Removing it and queueing a fetch."); + String message = "Part " + part_name + " looks broken. Removing it and queueing a fetch."; + LOG_ERROR(log, message); ProfileEvents::increment(ProfileEvents::ReplicatedPartChecksFailed); storage.removePartAndEnqueueFetch(part_name); /// Delete part locally. storage.forgetPartAndMoveToDetached(part, "broken"); + return {part_name, false, message}; } } else if (part->modification_time + MAX_AGE_OF_LOCAL_PART_THAT_WASNT_ADDED_TO_ZOOKEEPER < time(nullptr)) @@ -269,8 +272,10 @@ void ReplicatedMergeTreePartCheckThread::checkPart(const String & part_name) /// Therefore, delete only if the part is old (not very reliable). ProfileEvents::increment(ProfileEvents::ReplicatedPartChecksFailed); - LOG_ERROR(log, "Unexpected part " << part_name << " in filesystem. Removing."); + String message = "Unexpected part " + part_name + " in filesystem. Removing."; + LOG_ERROR(log, message); storage.forgetPartAndMoveToDetached(part, "unexpected"); + return {part_name, false, message}; } else { @@ -290,6 +295,8 @@ void ReplicatedMergeTreePartCheckThread::checkPart(const String & part_name) /// In the worst case, errors will still appear `old_parts_lifetime` seconds in error log until the part is removed as the old one. LOG_WARNING(log, "We have part " << part->name << " covering part " << part_name); } + + return {part_name, true, ""}; } diff --git a/dbms/src/Storages/MergeTree/ReplicatedMergeTreePartCheckThread.h b/dbms/src/Storages/MergeTree/ReplicatedMergeTreePartCheckThread.h index 432fb0f4bb6..322ee593c46 100644 --- a/dbms/src/Storages/MergeTree/ReplicatedMergeTreePartCheckThread.h +++ b/dbms/src/Storages/MergeTree/ReplicatedMergeTreePartCheckThread.h @@ -11,6 +11,7 @@ #include #include #include +#include namespace DB { @@ -66,12 +67,12 @@ public: /// Get the number of parts in the queue for check. size_t size() const; - + /// Check part by name + CheckResult checkPart(const String & part_name); private: void run(); - void checkPart(const String & part_name); void searchForMissingPart(const String & part_name); StorageReplicatedMergeTree & storage; diff --git a/dbms/src/Storages/StorageLog.cpp b/dbms/src/Storages/StorageLog.cpp index afef7af57e9..d297353135d 100644 --- a/dbms/src/Storages/StorageLog.cpp +++ b/dbms/src/Storages/StorageLog.cpp @@ -625,7 +625,7 @@ BlockOutputStreamPtr StorageLog::write( return std::make_shared(*this); } -bool StorageLog::checkData() const +CheckResults StorageLog::checkData(const ASTPtr & /* query */, const Context & /* context */) { std::shared_lock lock(rwlock); return file_checker.check(); diff --git a/dbms/src/Storages/StorageLog.h b/dbms/src/Storages/StorageLog.h index cf0d07a3bfe..7d58f4e4a5d 100644 --- a/dbms/src/Storages/StorageLog.h +++ b/dbms/src/Storages/StorageLog.h @@ -38,7 +38,7 @@ public: void rename(const String & new_path_to_db, const String & new_database_name, const String & new_table_name) override; - bool checkData() const override; + CheckResults checkData(const ASTPtr & /* query */, const Context & /* context */) override; void truncate(const ASTPtr &, const Context &) override; diff --git a/dbms/src/Storages/StorageMergeTree.cpp b/dbms/src/Storages/StorageMergeTree.cpp index bc6bb4d1080..baa498b9f6f 100644 --- a/dbms/src/Storages/StorageMergeTree.cpp +++ b/dbms/src/Storages/StorageMergeTree.cpp @@ -7,8 +7,10 @@ #include #include #include +#include #include #include +#include #include #include #include @@ -1117,12 +1119,54 @@ ActionLock StorageMergeTree::getActionLock(StorageActionBlockType action_type) return {}; } -bool StorageMergeTree::checkData() const +CheckResults StorageMergeTree::checkData(const ASTPtr & query, const Context & context) { - DataPartsVector data_parts = getDataPartsVector(); + CheckResults results; + DataPartsVector data_parts; + if (const auto & check_query = query->as(); check_query.partition) + { + String partition_id = getPartitionIDFromQuery(check_query.partition, context); + data_parts = getDataPartsVectorInPartition(MergeTreeDataPartState::Committed, partition_id); + } + else + data_parts = getDataPartsVector(); + for (auto & part : data_parts) - checkDataPart(part, true, primary_key_data_types, skip_indices); - return true; + { + String full_part_path = part->getFullPath(); + /// If the checksums file is not present, calculate the checksums and write them to disk. + String checksums_path = full_part_path + "checksums.txt"; + if (!Poco::File(checksums_path).exists()) + { + auto counted_checksums = checkDataPart(part, false, primary_key_data_types, skip_indices); + try + { + counted_checksums.checkEqual(part->checksums, true); + WriteBufferFromFile out(full_part_path + "checksums.txt.tmp", 4096); + part->checksums.write(out); + Poco::File(full_part_path + "checksums.txt.tmp").renameTo(full_part_path + "checksums.txt"); + results.emplace_back(part->name, true, "Checksums recounted and written to disk"); + } + catch (Exception & ex) + { + results.emplace_back(part->name, false, + "Checksums file absent and counted doesn't equal to checksums in memory. Error: '" + ex.message() + "'"); + } + } + else + { + try + { + checkDataPart(part, true, primary_key_data_types, skip_indices); + results.emplace_back(part->name, true, ""); + } + catch (Exception & ex) + { + results.emplace_back(part->name, false, ex.message()); + } + } + } + return results; } } diff --git a/dbms/src/Storages/StorageMergeTree.h b/dbms/src/Storages/StorageMergeTree.h index 90b02c29994..0de9618d915 100644 --- a/dbms/src/Storages/StorageMergeTree.h +++ b/dbms/src/Storages/StorageMergeTree.h @@ -70,7 +70,7 @@ public: String getDataPath() const override { return full_path; } - bool checkData() const override; + CheckResults checkData(const ASTPtr & query, const Context & context) override; private: String path; diff --git a/dbms/src/Storages/StorageReplicatedMergeTree.cpp b/dbms/src/Storages/StorageReplicatedMergeTree.cpp index 576857f98da..f385fb374b7 100644 --- a/dbms/src/Storages/StorageReplicatedMergeTree.cpp +++ b/dbms/src/Storages/StorageReplicatedMergeTree.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include #include @@ -5101,4 +5102,31 @@ bool StorageReplicatedMergeTree::dropPartsInPartition( return true; } + +CheckResults StorageReplicatedMergeTree::checkData(const ASTPtr & query, const Context & context) +{ + CheckResults results; + DataPartsVector data_parts; + if (const auto & check_query = query->as(); check_query.partition) + { + String partition_id = getPartitionIDFromQuery(check_query.partition, context); + data_parts = getDataPartsVectorInPartition(MergeTreeDataPartState::Committed, partition_id); + } + else + data_parts = getDataPartsVector(); + + for (auto & part : data_parts) + { + try + { + results.push_back(part_check_thread.checkPart(part->name)); + } + catch(Exception & ex) + { + results.emplace_back(part->name, false, "Error during check:" + ex.message()); + } + } + return results; +} + } diff --git a/dbms/src/Storages/StorageReplicatedMergeTree.h b/dbms/src/Storages/StorageReplicatedMergeTree.h index eba0511e15e..59afb96a523 100644 --- a/dbms/src/Storages/StorageReplicatedMergeTree.h +++ b/dbms/src/Storages/StorageReplicatedMergeTree.h @@ -170,6 +170,8 @@ public: String getDataPath() const override { return full_path; } + CheckResults checkData(const ASTPtr & query, const Context & context) override; + private: /// Delete old parts from disk and from ZooKeeper. void clearOldPartsAndRemoveFromZK(); diff --git a/dbms/src/Storages/StorageStripeLog.cpp b/dbms/src/Storages/StorageStripeLog.cpp index dba2e64a88f..28f8616bbaa 100644 --- a/dbms/src/Storages/StorageStripeLog.cpp +++ b/dbms/src/Storages/StorageStripeLog.cpp @@ -282,7 +282,7 @@ BlockOutputStreamPtr StorageStripeLog::write( } -bool StorageStripeLog::checkData() const +CheckResults StorageStripeLog::checkData(const ASTPtr & /* query */, const Context & /* context */) { std::shared_lock lock(rwlock); return file_checker.check(); diff --git a/dbms/src/Storages/StorageStripeLog.h b/dbms/src/Storages/StorageStripeLog.h index 6489c82873e..31b681e5800 100644 --- a/dbms/src/Storages/StorageStripeLog.h +++ b/dbms/src/Storages/StorageStripeLog.h @@ -40,7 +40,7 @@ public: void rename(const String & new_path_to_db, const String & new_database_name, const String & new_table_name) override; - bool checkData() const override; + CheckResults checkData(const ASTPtr & /* query */, const Context & /* context */) override; /// Data of the file. struct ColumnData diff --git a/dbms/src/Storages/StorageTinyLog.cpp b/dbms/src/Storages/StorageTinyLog.cpp index 4690ab925e8..eacca1c6413 100644 --- a/dbms/src/Storages/StorageTinyLog.cpp +++ b/dbms/src/Storages/StorageTinyLog.cpp @@ -32,6 +32,7 @@ #include #include +#include #include @@ -404,7 +405,7 @@ BlockOutputStreamPtr StorageTinyLog::write( } -bool StorageTinyLog::checkData() const +CheckResults StorageTinyLog::checkData(const ASTPtr & /* query */, const Context & /* context */) { return file_checker.check(); } diff --git a/dbms/src/Storages/StorageTinyLog.h b/dbms/src/Storages/StorageTinyLog.h index 5b8e4bc90ac..111df9baf90 100644 --- a/dbms/src/Storages/StorageTinyLog.h +++ b/dbms/src/Storages/StorageTinyLog.h @@ -39,7 +39,7 @@ public: void rename(const String & new_path_to_db, const String & new_database_name, const String & new_table_name) override; - bool checkData() const override; + CheckResults checkData(const ASTPtr & /* query */, const Context & /* context */) override; /// Column data struct ColumnData From c880e00ffa4c1652da8ea8393d98306ecdbd24da Mon Sep 17 00:00:00 2001 From: alesapin Date: Wed, 3 Jul 2019 19:00:24 +0300 Subject: [PATCH 03/62] Start writing tests --- dbms/src/Common/FileChecker.cpp | 9 +++++---- dbms/src/Parsers/ASTCheckQuery.h | 10 +++++++++- dbms/src/Parsers/ParserCheckQuery.cpp | 2 ++ dbms/src/Parsers/ParserQueryWithOutput.cpp | 3 +++ dbms/src/Parsers/tests/select_parser.cpp | 21 +++++++++++---------- dbms/tests/clickhouse-client.xml | 1 + 6 files changed, 31 insertions(+), 15 deletions(-) diff --git a/dbms/src/Common/FileChecker.cpp b/dbms/src/Common/FileChecker.cpp index 8a372aea9a0..5256b683653 100644 --- a/dbms/src/Common/FileChecker.cpp +++ b/dbms/src/Common/FileChecker.cpp @@ -57,10 +57,11 @@ CheckResults FileChecker::check() const for (const auto & name_size : local_map) { - Poco::File file(Poco::Path(files_info_path).parent().toString() + "/" + name_size.first); + Poco::Path path = Poco::Path(files_info_path).parent().toString() + "/" + name_size.first; + Poco::File file(path); if (!file.exists()) { - results.emplace_back(file.path(), false, "File " + file.path() + " doesn't exist"); + results.emplace_back(path.getFileName(), false, "File " + file.path() + " doesn't exist"); break; } @@ -68,10 +69,10 @@ CheckResults FileChecker::check() const size_t real_size = file.getSize(); if (real_size != name_size.second) { - results.emplace_back(file.path(), false, "Size of " + file.path() + " is wrong. Size is " + toString(real_size) + " but should be " + toString(name_size.second)); + results.emplace_back(path.getFileName(), false, "Size of " + file.path() + " is wrong. Size is " + toString(real_size) + " but should be " + toString(name_size.second)); break; } - results.emplace_back(file.path(), true, ""); + results.emplace_back(path.getFileName(), true, ""); } return results; diff --git a/dbms/src/Parsers/ASTCheckQuery.h b/dbms/src/Parsers/ASTCheckQuery.h index 93a69afc368..89b9035ae9c 100644 --- a/dbms/src/Parsers/ASTCheckQuery.h +++ b/dbms/src/Parsers/ASTCheckQuery.h @@ -1,6 +1,8 @@ #pragma once #include +#include +#include namespace DB { @@ -22,7 +24,7 @@ struct ASTCheckQuery : public ASTQueryWithTableAndOutput } protected: - void formatQueryImpl(const FormatSettings & settings, FormatState &, FormatStateStacked frame) const override + void formatQueryImpl(const FormatSettings & settings, FormatState & state, FormatStateStacked frame) const override { std::string nl_or_nothing = settings.one_line ? "" : "\n"; @@ -40,6 +42,12 @@ protected: } settings.ostr << (settings.hilite ? hilite_keyword : "") << indent_str << backQuoteIfNeed(table) << (settings.hilite ? hilite_none : ""); } + + if (partition) + { + settings.ostr << (settings.hilite ? hilite_keyword : "") << indent_str << " PARTITION " << (settings.hilite ? hilite_none : ""); + partition->formatImpl(settings, state, frame); + } } }; diff --git a/dbms/src/Parsers/ParserCheckQuery.cpp b/dbms/src/Parsers/ParserCheckQuery.cpp index 5ba8119571d..1ae0b0322ca 100644 --- a/dbms/src/Parsers/ParserCheckQuery.cpp +++ b/dbms/src/Parsers/ParserCheckQuery.cpp @@ -4,6 +4,8 @@ #include #include #include +#include +#include namespace DB diff --git a/dbms/src/Parsers/ParserQueryWithOutput.cpp b/dbms/src/Parsers/ParserQueryWithOutput.cpp index c41e0946a96..a5582bb4b4e 100644 --- a/dbms/src/Parsers/ParserQueryWithOutput.cpp +++ b/dbms/src/Parsers/ParserQueryWithOutput.cpp @@ -13,6 +13,9 @@ #include #include #include +#include +#include + namespace DB diff --git a/dbms/src/Parsers/tests/select_parser.cpp b/dbms/src/Parsers/tests/select_parser.cpp index f5d94746aa1..a5e0b84fe33 100644 --- a/dbms/src/Parsers/tests/select_parser.cpp +++ b/dbms/src/Parsers/tests/select_parser.cpp @@ -10,16 +10,17 @@ try { using namespace DB; - std::string input = - " SELECT 18446744073709551615, f(1), '\\\\', [a, b, c], (a, b, c), 1 + 2 * -3, a = b OR c > d.1 + 2 * -g[0] AND NOT e < f * (x + y)" - " FROM default.hits" - " WHERE CounterID = 101500 AND UniqID % 3 = 0" - " GROUP BY UniqID" - " HAVING SUM(Refresh) > 100" - " ORDER BY Visits, PageViews" - " LIMIT LENGTH('STRING OF 20 SYMBOLS') - 20 + 1000, 10.05 / 5.025 * 5" - " INTO OUTFILE 'test.out'" - " FORMAT TabSeparated"; + //std::string input = + // " SELECT 18446744073709551615, f(1), '\\\\', [a, b, c], (a, b, c), 1 + 2 * -3, a = b OR c > d.1 + 2 * -g[0] AND NOT e < f * (x + y)" + // " FROM default.hits" + // " WHERE CounterID = 101500 AND UniqID % 3 = 0" + // " GROUP BY UniqID" + // " HAVING SUM(Refresh) > 100" + // " ORDER BY Visits, PageViews" + // " LIMIT LENGTH('STRING OF 20 SYMBOLS') - 20 + 1000, 10.05 / 5.025 * 5" + // " INTO OUTFILE 'test.out'" + // " FORMAT TabSeparated"; + std::string input = "CHECK TABLE txxx PARTITION 201910"; ParserQueryWithOutput parser; ASTPtr ast = parseQuery(parser, input.data(), input.data() + input.size(), "", 0); diff --git a/dbms/tests/clickhouse-client.xml b/dbms/tests/clickhouse-client.xml index b6003ca2d09..6524397fab3 100644 --- a/dbms/tests/clickhouse-client.xml +++ b/dbms/tests/clickhouse-client.xml @@ -1,3 +1,4 @@ 100000 +HELLO From 477a7450fb0ca79520f395f411041383e325ad39 Mon Sep 17 00:00:00 2001 From: alesapin Date: Wed, 3 Jul 2019 23:51:13 +0300 Subject: [PATCH 04/62] Add test for data corruption --- dbms/src/Storages/StorageMergeTree.cpp | 6 +- .../integration/test_check_table/__init__.py | 0 .../integration/test_check_table/test.py | 122 ++++++++++++++++++ .../00077_log_tinylog_stripelog.reference | 19 ++- 4 files changed, 138 insertions(+), 9 deletions(-) create mode 100644 dbms/tests/integration/test_check_table/__init__.py create mode 100644 dbms/tests/integration/test_check_table/test.py diff --git a/dbms/src/Storages/StorageMergeTree.cpp b/dbms/src/Storages/StorageMergeTree.cpp index baa498b9f6f..dc0a544c4a0 100644 --- a/dbms/src/Storages/StorageMergeTree.cpp +++ b/dbms/src/Storages/StorageMergeTree.cpp @@ -1138,19 +1138,19 @@ CheckResults StorageMergeTree::checkData(const ASTPtr & query, const Context & c String checksums_path = full_part_path + "checksums.txt"; if (!Poco::File(checksums_path).exists()) { - auto counted_checksums = checkDataPart(part, false, primary_key_data_types, skip_indices); try { + auto counted_checksums = checkDataPart(part, false, primary_key_data_types, skip_indices); counted_checksums.checkEqual(part->checksums, true); WriteBufferFromFile out(full_part_path + "checksums.txt.tmp", 4096); part->checksums.write(out); Poco::File(full_part_path + "checksums.txt.tmp").renameTo(full_part_path + "checksums.txt"); - results.emplace_back(part->name, true, "Checksums recounted and written to disk"); + results.emplace_back(part->name, true, "Checksums recounted and written to disk."); } catch (Exception & ex) { results.emplace_back(part->name, false, - "Checksums file absent and counted doesn't equal to checksums in memory. Error: '" + ex.message() + "'"); + "Check of part finished with error: '" + ex.message() + "'"); } } else diff --git a/dbms/tests/integration/test_check_table/__init__.py b/dbms/tests/integration/test_check_table/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/dbms/tests/integration/test_check_table/test.py b/dbms/tests/integration/test_check_table/test.py new file mode 100644 index 00000000000..e788b13f21e --- /dev/null +++ b/dbms/tests/integration/test_check_table/test.py @@ -0,0 +1,122 @@ +import time + +import pytest + +from helpers.cluster import ClickHouseCluster +from helpers.test_tools import assert_eq_with_retry + +cluster = ClickHouseCluster(__file__) + +node1 = cluster.add_instance('node1', with_zookeeper=True) +node2 = cluster.add_instance('node2', with_zookeeper=True) + + +@pytest.fixture(scope="module") +def started_cluster(): + try: + cluster.start() + + for node in [node1, node2]: + node.query(''' + CREATE TABLE replicated_mt(date Date, id UInt32, value Int32) + ENGINE = ReplicatedMergeTree('/clickhouse/tables/replicated_mt', '{replica}') PARTITION BY toYYYYMM(date) ORDER BY id; + '''.format(replica=node.name)) + + node1.query(''' + CREATE TABLE non_replicated_mt(date Date, id UInt32, value Int32) + ENGINE = MergeTree() PARTITION BY toYYYYMM(date) ORDER BY id; + ''') + + yield cluster + + finally: + cluster.shutdown() + + +def corrupt_data_part_on_disk(node, table, part_name): + part_path = node.query("SELECT path FROM system.parts WHERE table = '{}' and name = '{}'".format(table, part_name)).strip() + node.exec_in_container(['bash', '-c', 'cd {p} && ls *.bin | head -n 1 | xargs -I{{}} sh -c \'echo "1" >> $1\' -- {{}}'.format(p=part_path)], privileged=True) + +def remove_checksums_on_disk(node, table, part_name): + part_path = node.query("SELECT path FROM system.parts WHERE table = '{}' and name = '{}'".format(table, part_name)).strip() + node.exec_in_container(['bash', '-c', 'rm -r {p}/checksums.txt'.format(p=part_path)], privileged=True) + +def remove_part_from_disk(node, table, part_name): + part_path = node.query("SELECT path FROM system.parts WHERE table = '{}' and name = '{}'".format(table, part_name)).strip() + node.exec_in_container(['bash', '-c', 'rm -r {p}/*'.format(p=part_path)], privileged=True) + + +def test_check_normal_table_corruption(started_cluster): + node1.query("INSERT INTO non_replicated_mt VALUES (toDate('2019-02-01'), 1, 10), (toDate('2019-02-01'), 2, 12)") + assert node1.query("CHECK TABLE non_replicated_mt PARTITION 201902") == "201902_1_1_0\t1\t\n" + + remove_checksums_on_disk(node1, "non_replicated_mt", "201902_1_1_0") + + assert node1.query("CHECK TABLE non_replicated_mt").strip() == "201902_1_1_0\t1\tChecksums recounted and written to disk." + + assert node1.query("SELECT COUNT() FROM non_replicated_mt") == "2\n" + + remove_checksums_on_disk(node1, "non_replicated_mt", "201902_1_1_0") + + assert node1.query("CHECK TABLE non_replicated_mt PARTITION 201902").strip() == "201902_1_1_0\t1\tChecksums recounted and written to disk." + + assert node1.query("SELECT COUNT() FROM non_replicated_mt") == "2\n" + + corrupt_data_part_on_disk(node1, "non_replicated_mt", "201902_1_1_0") + + assert node1.query("CHECK TABLE non_replicated_mt").strip() == "201902_1_1_0\t0\tCannot read all data. Bytes read: 2. Bytes expected: 16." + + assert node1.query("CHECK TABLE non_replicated_mt").strip() == "201902_1_1_0\t0\tCannot read all data. Bytes read: 2. Bytes expected: 16." + + node1.query("INSERT INTO non_replicated_mt VALUES (toDate('2019-01-01'), 1, 10), (toDate('2019-01-01'), 2, 12)") + + assert node1.query("CHECK TABLE non_replicated_mt PARTITION 201901") == "201901_2_2_0\t1\t\n" + + corrupt_data_part_on_disk(node1, "non_replicated_mt", "201901_2_2_0") + + remove_checksums_on_disk(node1, "non_replicated_mt", "201901_2_2_0") + + assert node1.query("CHECK TABLE non_replicated_mt PARTITION 201901") == "201901_2_2_0\t0\tCheck of part finished with error: \\'Cannot read all data. Bytes read: 2. Bytes expected: 16.\\'\n" + + +def test_check_replicated_table_simple(started_cluster): + node1.query("INSERT INTO replicated_mt VALUES (toDate('2019-02-01'), 1, 10), (toDate('2019-02-01'), 2, 12)") + node2.query("SYSTEM SYNC REPLICA replicated_mt") + + assert node1.query("SELECT count() from replicated_mt") == "2\n" + assert node2.query("SELECT count() from replicated_mt") == "2\n" + + assert node1.query("CHECK TABLE replicated_mt") == "201902_0_0_0\t1\t\n" + assert node2.query("CHECK TABLE replicated_mt") == "201902_0_0_0\t1\t\n" + + node2.query("INSERT INTO replicated_mt VALUES (toDate('2019-01-02'), 3, 10), (toDate('2019-01-02'), 4, 12)") + node1.query("SYSTEM SYNC REPLICA replicated_mt") + assert node1.query("SELECT count() from replicated_mt") == "4\n" + assert node2.query("SELECT count() from replicated_mt") == "4\n" + + assert node1.query("CHECK TABLE replicated_mt PARTITION 201901") == "201901_0_0_0\t1\t\n" + assert node2.query("CHECK TABLE replicated_mt PARTITION 201901") == "201901_0_0_0\t1\t\n" + + +def test_check_replicated_table_corruption(started_cluster): + node1.query("TRUNCATE TABLE replicated_mt") + node1.query("INSERT INTO replicated_mt VALUES (toDate('2019-02-01'), 1, 10), (toDate('2019-02-01'), 2, 12)") + node1.query("INSERT INTO replicated_mt VALUES (toDate('2019-01-02'), 3, 10), (toDate('2019-01-02'), 4, 12)") + node2.query("SYSTEM SYNC REPLICA replicated_mt") + + assert node1.query("SELECT count() from replicated_mt") == "4\n" + assert node2.query("SELECT count() from replicated_mt") == "4\n" + + corrupt_data_part_on_disk(node1, "replicated_mt", "201901_2_2_0") + assert node1.query("CHECK TABLE replicated_mt PARTITION 201901") == "201901_2_2_0\t0\tPart 201901_2_2_0 looks broken. Removing it and queueing a fetch.\n" + + node1.query("SYSTEM SYNC REPLICA replicated_mt") + assert node1.query("CHECK TABLE replicated_mt PARTITION 201901") == "201901_2_2_0\t1\t\n" + assert node1.query("SELECT count() from replicated_mt") == "4\n" + + remove_part_from_disk(node2, "replicated_mt", "201901_2_2_0") + assert node2.query("CHECK TABLE replicated_mt PARTITION 201901") == "201901_2_2_0\t0\tPart 201901_2_2_0 looks broken. Removing it and queueing a fetch.\n" + + node1.query("SYSTEM SYNC REPLICA replicated_mt") + assert node1.query("CHECK TABLE replicated_mt PARTITION 201901") == "201901_2_2_0\t1\t\n" + assert node1.query("SELECT count() from replicated_mt") == "4\n" diff --git a/dbms/tests/queries/1_stateful/00077_log_tinylog_stripelog.reference b/dbms/tests/queries/1_stateful/00077_log_tinylog_stripelog.reference index 98f50687de4..d8ccbd66592 100644 --- a/dbms/tests/queries/1_stateful/00077_log_tinylog_stripelog.reference +++ b/dbms/tests/queries/1_stateful/00077_log_tinylog_stripelog.reference @@ -1,10 +1,17 @@ -1 -1 -1 8873898 12457120258355519194 8873898 12457120258355519194 8873898 12457120258355519194 8873898 12457120258355519194 -1 -1 -1 +AdvEngineID.bin 1 +CounterID.bin 1 +RegionID.bin 1 +SearchPhrase.bin 1 +UserID.bin 1 +__marks.mrk 1 +AdvEngineID.bin 1 +CounterID.bin 1 +RegionID.bin 1 +SearchPhrase.bin 1 +UserID.bin 1 +data.bin 1 +index.mrk 1 From 4557412feda17ca6b5564a15a67ed92e27e33a62 Mon Sep 17 00:00:00 2001 From: alesapin Date: Wed, 3 Jul 2019 23:57:05 +0300 Subject: [PATCH 05/62] Remove unexpected changes --- dbms/src/Parsers/tests/select_parser.cpp | 21 ++++++++++----------- dbms/tests/clickhouse-client.xml | 1 - 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/dbms/src/Parsers/tests/select_parser.cpp b/dbms/src/Parsers/tests/select_parser.cpp index a5e0b84fe33..f5d94746aa1 100644 --- a/dbms/src/Parsers/tests/select_parser.cpp +++ b/dbms/src/Parsers/tests/select_parser.cpp @@ -10,17 +10,16 @@ try { using namespace DB; - //std::string input = - // " SELECT 18446744073709551615, f(1), '\\\\', [a, b, c], (a, b, c), 1 + 2 * -3, a = b OR c > d.1 + 2 * -g[0] AND NOT e < f * (x + y)" - // " FROM default.hits" - // " WHERE CounterID = 101500 AND UniqID % 3 = 0" - // " GROUP BY UniqID" - // " HAVING SUM(Refresh) > 100" - // " ORDER BY Visits, PageViews" - // " LIMIT LENGTH('STRING OF 20 SYMBOLS') - 20 + 1000, 10.05 / 5.025 * 5" - // " INTO OUTFILE 'test.out'" - // " FORMAT TabSeparated"; - std::string input = "CHECK TABLE txxx PARTITION 201910"; + std::string input = + " SELECT 18446744073709551615, f(1), '\\\\', [a, b, c], (a, b, c), 1 + 2 * -3, a = b OR c > d.1 + 2 * -g[0] AND NOT e < f * (x + y)" + " FROM default.hits" + " WHERE CounterID = 101500 AND UniqID % 3 = 0" + " GROUP BY UniqID" + " HAVING SUM(Refresh) > 100" + " ORDER BY Visits, PageViews" + " LIMIT LENGTH('STRING OF 20 SYMBOLS') - 20 + 1000, 10.05 / 5.025 * 5" + " INTO OUTFILE 'test.out'" + " FORMAT TabSeparated"; ParserQueryWithOutput parser; ASTPtr ast = parseQuery(parser, input.data(), input.data() + input.size(), "", 0); diff --git a/dbms/tests/clickhouse-client.xml b/dbms/tests/clickhouse-client.xml index 6524397fab3..b6003ca2d09 100644 --- a/dbms/tests/clickhouse-client.xml +++ b/dbms/tests/clickhouse-client.xml @@ -1,4 +1,3 @@ 100000 -HELLO From 83ca611c59fab6a5766d234de62fd36ab0990df7 Mon Sep 17 00:00:00 2001 From: alesapin Date: Wed, 3 Jul 2019 23:59:47 +0300 Subject: [PATCH 06/62] Remove more unexpected changes --- dbms/src/Parsers/ParserCheckQuery.cpp | 2 -- dbms/src/Parsers/ParserQueryWithOutput.cpp | 2 -- 2 files changed, 4 deletions(-) diff --git a/dbms/src/Parsers/ParserCheckQuery.cpp b/dbms/src/Parsers/ParserCheckQuery.cpp index 1ae0b0322ca..5ba8119571d 100644 --- a/dbms/src/Parsers/ParserCheckQuery.cpp +++ b/dbms/src/Parsers/ParserCheckQuery.cpp @@ -4,8 +4,6 @@ #include #include #include -#include -#include namespace DB diff --git a/dbms/src/Parsers/ParserQueryWithOutput.cpp b/dbms/src/Parsers/ParserQueryWithOutput.cpp index a5582bb4b4e..23c89ae20f9 100644 --- a/dbms/src/Parsers/ParserQueryWithOutput.cpp +++ b/dbms/src/Parsers/ParserQueryWithOutput.cpp @@ -13,8 +13,6 @@ #include #include #include -#include -#include From 18544e8f64e6db394d599530bec55e014fd2dc76 Mon Sep 17 00:00:00 2001 From: alesapin Date: Thu, 4 Jul 2019 09:04:31 +0300 Subject: [PATCH 07/62] Missed file and style fix --- dbms/src/Storages/CheckResults.h | 27 +++++++++++++++++++ .../Storages/StorageReplicatedMergeTree.cpp | 2 +- 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 dbms/src/Storages/CheckResults.h diff --git a/dbms/src/Storages/CheckResults.h b/dbms/src/Storages/CheckResults.h new file mode 100644 index 00000000000..53f1e7c0ca2 --- /dev/null +++ b/dbms/src/Storages/CheckResults.h @@ -0,0 +1,27 @@ +#pragma once + +#include +#include + +namespace DB +{ + +/// Result of CHECK TABLE query for single part of table +struct CheckResult +{ + /// Part name for merge tree or file name for simplier tables + String fs_path; + /// Does check passed + bool success; + /// Failure message if any + String failure_message; + + CheckResult() = default; + CheckResult(const String & fs_path_, bool success_, String failure_message_) + : fs_path(fs_path_), success(success_), failure_message(failure_message_) + {} +}; + +using CheckResults = std::vector; + +} diff --git a/dbms/src/Storages/StorageReplicatedMergeTree.cpp b/dbms/src/Storages/StorageReplicatedMergeTree.cpp index f385fb374b7..6a56b6f0752 100644 --- a/dbms/src/Storages/StorageReplicatedMergeTree.cpp +++ b/dbms/src/Storages/StorageReplicatedMergeTree.cpp @@ -5121,7 +5121,7 @@ CheckResults StorageReplicatedMergeTree::checkData(const ASTPtr & query, const C { results.push_back(part_check_thread.checkPart(part->name)); } - catch(Exception & ex) + catch (Exception & ex) { results.emplace_back(part->name, false, "Error during check:" + ex.message()); } From 65437d22899dd7b58a8ecbd2d7a5929c4eacc48b Mon Sep 17 00:00:00 2001 From: alesapin Date: Thu, 4 Jul 2019 13:21:14 +0300 Subject: [PATCH 08/62] Fix final mark and tests --- dbms/src/Storages/MergeTree/checkDataPart.cpp | 29 ++++++++++++++----- .../integration/test_check_table/test.py | 17 +++++++---- .../0_stateless/00063_check_query.reference | 7 +++-- 3 files changed, 37 insertions(+), 16 deletions(-) diff --git a/dbms/src/Storages/MergeTree/checkDataPart.cpp b/dbms/src/Storages/MergeTree/checkDataPart.cpp index 2b17a675144..af34cb1ea1e 100644 --- a/dbms/src/Storages/MergeTree/checkDataPart.cpp +++ b/dbms/src/Storages/MergeTree/checkDataPart.cpp @@ -53,6 +53,20 @@ public: private: ReadBufferFromFile mrk_file_buf; + + std::pair readMarkFromFile() + { + size_t mrk_rows; + MarkInCompressedFile mrk_mark; + readIntBinary(mrk_mark.offset_in_compressed_file, mrk_hashing_buf); + readIntBinary(mrk_mark.offset_in_decompressed_block, mrk_hashing_buf); + if (mrk_file_extension == ".mrk2") + readIntBinary(mrk_rows, mrk_hashing_buf); + else + mrk_rows = index_granularity.getMarkRows(mark_position); + + return {mrk_mark, mrk_rows}; + } public: HashingReadBuffer mrk_hashing_buf; @@ -78,15 +92,8 @@ public: void assertMark(bool only_read=false) { - MarkInCompressedFile mrk_mark; - readIntBinary(mrk_mark.offset_in_compressed_file, mrk_hashing_buf); - readIntBinary(mrk_mark.offset_in_decompressed_block, mrk_hashing_buf); - size_t mrk_rows; - if (mrk_file_extension == ".mrk2") - readIntBinary(mrk_rows, mrk_hashing_buf); - else - mrk_rows = index_granularity.getMarkRows(mark_position); + auto [mrk_mark, mrk_rows] = readMarkFromFile(); bool has_alternative_mark = false; MarkInCompressedFile alternative_data_mark = {}; MarkInCompressedFile data_mark = {}; @@ -136,6 +143,12 @@ public: + toString(compressed_hashing_buf.count()) + " (compressed), " + toString(uncompressed_hashing_buf.count()) + " (uncompressed)", ErrorCodes::CORRUPTED_DATA); + if (index_granularity.hasFinalMark()) + { + auto [final_mark, final_mark_rows] = readMarkFromFile(); + if (final_mark_rows != 0) + throw Exception("Incorrect final mark at the end of " + mrk_file_path + " expected 0 rows, got " + toString(final_mark_rows), ErrorCodes::CORRUPTED_DATA); + } if (!mrk_hashing_buf.eof()) throw Exception("EOF expected in " + mrk_file_path + " file" + " at position " diff --git a/dbms/tests/integration/test_check_table/test.py b/dbms/tests/integration/test_check_table/test.py index e788b13f21e..195c421f854 100644 --- a/dbms/tests/integration/test_check_table/test.py +++ b/dbms/tests/integration/test_check_table/test.py @@ -43,6 +43,8 @@ def remove_checksums_on_disk(node, table, part_name): def remove_part_from_disk(node, table, part_name): part_path = node.query("SELECT path FROM system.parts WHERE table = '{}' and name = '{}'".format(table, part_name)).strip() + if not part_path: + raise Exception("Part " + part_name + "doesn't exist") node.exec_in_container(['bash', '-c', 'rm -r {p}/*'.format(p=part_path)], privileged=True) @@ -80,6 +82,8 @@ def test_check_normal_table_corruption(started_cluster): def test_check_replicated_table_simple(started_cluster): + node1.query("TRUNCATE TABLE replicated_mt") + node2.query("SYSTEM SYNC REPLICA replicated_mt") node1.query("INSERT INTO replicated_mt VALUES (toDate('2019-02-01'), 1, 10), (toDate('2019-02-01'), 2, 12)") node2.query("SYSTEM SYNC REPLICA replicated_mt") @@ -100,6 +104,7 @@ def test_check_replicated_table_simple(started_cluster): def test_check_replicated_table_corruption(started_cluster): node1.query("TRUNCATE TABLE replicated_mt") + node2.query("SYSTEM SYNC REPLICA replicated_mt") node1.query("INSERT INTO replicated_mt VALUES (toDate('2019-02-01'), 1, 10), (toDate('2019-02-01'), 2, 12)") node1.query("INSERT INTO replicated_mt VALUES (toDate('2019-01-02'), 3, 10), (toDate('2019-01-02'), 4, 12)") node2.query("SYSTEM SYNC REPLICA replicated_mt") @@ -107,16 +112,16 @@ def test_check_replicated_table_corruption(started_cluster): assert node1.query("SELECT count() from replicated_mt") == "4\n" assert node2.query("SELECT count() from replicated_mt") == "4\n" - corrupt_data_part_on_disk(node1, "replicated_mt", "201901_2_2_0") - assert node1.query("CHECK TABLE replicated_mt PARTITION 201901") == "201901_2_2_0\t0\tPart 201901_2_2_0 looks broken. Removing it and queueing a fetch.\n" + corrupt_data_part_on_disk(node1, "replicated_mt", "201901_0_0_0") + assert node1.query("CHECK TABLE replicated_mt PARTITION 201901") == "201901_0_0_0\t0\tPart 201901_0_0_0 looks broken. Removing it and queueing a fetch.\n" node1.query("SYSTEM SYNC REPLICA replicated_mt") - assert node1.query("CHECK TABLE replicated_mt PARTITION 201901") == "201901_2_2_0\t1\t\n" + assert node1.query("CHECK TABLE replicated_mt PARTITION 201901") == "201901_0_0_0\t1\t\n" assert node1.query("SELECT count() from replicated_mt") == "4\n" - remove_part_from_disk(node2, "replicated_mt", "201901_2_2_0") - assert node2.query("CHECK TABLE replicated_mt PARTITION 201901") == "201901_2_2_0\t0\tPart 201901_2_2_0 looks broken. Removing it and queueing a fetch.\n" + remove_part_from_disk(node2, "replicated_mt", "201901_0_0_0") + assert node2.query("CHECK TABLE replicated_mt PARTITION 201901") == "201901_0_0_0\t0\tPart 201901_0_0_0 looks broken. Removing it and queueing a fetch.\n" node1.query("SYSTEM SYNC REPLICA replicated_mt") - assert node1.query("CHECK TABLE replicated_mt PARTITION 201901") == "201901_2_2_0\t1\t\n" + assert node1.query("CHECK TABLE replicated_mt PARTITION 201901") == "201901_0_0_0\t1\t\n" assert node1.query("SELECT count() from replicated_mt") == "4\n" diff --git a/dbms/tests/queries/0_stateless/00063_check_query.reference b/dbms/tests/queries/0_stateless/00063_check_query.reference index 6ed281c757a..9b20cc02e31 100644 --- a/dbms/tests/queries/0_stateless/00063_check_query.reference +++ b/dbms/tests/queries/0_stateless/00063_check_query.reference @@ -1,2 +1,5 @@ -1 -1 +N.bin 1 +S.bin 1 +N.bin 1 +S.bin 1 +__marks.mrk 1 From b3ebb4d7844aead7968868790373e51c72e6e944 Mon Sep 17 00:00:00 2001 From: alesapin Date: Fri, 5 Jul 2019 18:12:18 +0300 Subject: [PATCH 09/62] Add coverage images for CI --- CMakeLists.txt | 3 ++- docker/packager/packager | 8 ++++++-- docker/test/coverage/Dockerfile | 33 ++++++++++++++++++++++++++++++++ docker/test/stateful/Dockerfile | 6 ++++-- docker/test/stateless/Dockerfile | 7 +++++-- 5 files changed, 50 insertions(+), 7 deletions(-) create mode 100644 docker/test/coverage/Dockerfile diff --git a/CMakeLists.txt b/CMakeLists.txt index 0273abec108..0c1154d5b6c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -189,6 +189,7 @@ if(WITH_COVERAGE AND COMPILER_CLANG) endif() if(WITH_COVERAGE AND COMPILER_GCC) set(COMPILER_FLAGS "${COMPILER_FLAGS} -fprofile-arcs -ftest-coverage") + set(COVERAGE_OPTION "-lgcov") endif() set (CMAKE_BUILD_COLOR_MAKEFILE ON) @@ -252,7 +253,7 @@ if (OS_LINUX AND NOT UNBUNDLED AND (GLIBC_COMPATIBILITY OR USE_INTERNAL_UNWIND_L if (USE_LIBCXX) set (DEFAULT_LIBS "${DEFAULT_LIBS} -Wl,-Bstatic -lc++ -lc++abi ${EXCEPTION_HANDLING_LIBRARY} ${BUILTINS_LIB_PATH} -Wl,-Bdynamic") else () - set (DEFAULT_LIBS "${DEFAULT_LIBS} -Wl,-Bstatic -lstdc++ ${EXCEPTION_HANDLING_LIBRARY} -lgcc ${BUILTINS_LIB_PATH} -Wl,-Bdynamic") + set (DEFAULT_LIBS "${DEFAULT_LIBS} -Wl,-Bstatic -lstdc++ ${EXCEPTION_HANDLING_LIBRARY} ${COVERAGE_OPTION} -lgcc ${BUILTINS_LIB_PATH} -Wl,-Bdynamic") endif () # Linking with GLIBC prevents portability of binaries to older systems. diff --git a/docker/packager/packager b/docker/packager/packager index 9dadb96b46d..d0d46fd5f51 100755 --- a/docker/packager/packager +++ b/docker/packager/packager @@ -103,7 +103,7 @@ def run_vagrant_box_with_env(image_path, output_dir, ch_root): logging.info("Copying binary back") vagrant.copy_from_image("~/ClickHouse/dbms/programs/clickhouse", output_dir) -def parse_env_variables(build_type, compiler, sanitizer, package_type, cache, distcc_hosts, unbundled, split_binary, version, author, official, alien_pkgs): +def parse_env_variables(build_type, compiler, sanitizer, package_type, cache, distcc_hosts, unbundled, split_binary, version, author, official, alien_pkgs, with_coverage): result = [] cmake_flags = ['$CMAKE_FLAGS'] @@ -148,6 +148,9 @@ def parse_env_variables(build_type, compiler, sanitizer, package_type, cache, di if split_binary: cmake_flags.append('-DUSE_STATIC_LIBRARIES=0 -DSPLIT_SHARED_LIBRARIES=1 -DCLICKHOUSE_SPLIT_BINARY=1') + if with_coverage: + cmake_flags.append('-DWITH_COVERAGE=1') + if version: result.append("VERSION_STRING='{}'".format(version)) @@ -180,6 +183,7 @@ if __name__ == "__main__": parser.add_argument("--author", default="clickhouse") parser.add_argument("--official", action="store_true") parser.add_argument("--alien-pkgs", nargs='+', default=[]) + parser.add_argument("--with-coverage", action="store_true") args = parser.parse_args() if not os.path.isabs(args.output_dir): @@ -202,7 +206,7 @@ if __name__ == "__main__": env_prepared = parse_env_variables( args.build_type, args.compiler, args.sanitizer, args.package_type, args.cache, args.distcc_hosts, args.unbundled, args.split_binary, - args.version, args.author, args.official, args.alien_pkgs) + args.version, args.author, args.official, args.alien_pkgs, args.with_coverage) if args.package_type != "freebsd": run_docker_image_with_env(image_name, args.output_dir, env_prepared, ch_root, args.ccache_dir) else: diff --git a/docker/test/coverage/Dockerfile b/docker/test/coverage/Dockerfile new file mode 100644 index 00000000000..c3837d6df12 --- /dev/null +++ b/docker/test/coverage/Dockerfile @@ -0,0 +1,33 @@ +# docker build -t yandex/clickhouse-coverage . +FROM ubuntu:18.04 + +RUN apt-get --allow-unauthenticated update -y \ + && env DEBIAN_FRONTEND=noninteractive \ + apt-get --allow-unauthenticated install --yes --no-install-recommends \ + bash \ + fakeroot \ + cmake \ + ccache \ + curl \ + software-properties-common + + +RUN echo "deb [trusted=yes] http://apt.llvm.org/bionic/ llvm-toolchain-bionic main" >> /etc/apt/sources.list + +RUN apt-get --allow-unauthenticated update -y \ + && env DEBIAN_FRONTEND=noninteractive \ + apt-get --allow-unauthenticated install --yes --no-install-recommends \ + perl \ + lcov \ + llvm-9 \ + tzdata + + +ENV COVERAGE_DIR=/coverage_reports +ENV SOURCE_DIR=/build +ENV OUTPUT_DIR=/output + +CMD dpkg -i /package_folder/clickhouse-common-static_*.deb; \ + llvm-profdata-9 merge -sparse ${COVERAGE_DIR}/* -o clickhouse.profdata && \ + llvm-cov-9 export /usr/bin/clickhouse -instr-profile=clickhouse.profdata -arch=x86_64 -j=16 -format=lcov -skip-functions -ignore-filename-regex '.*contrib.*' > output.lcov && \ + genhtml output.lcov --ignore-errors source --output-directory ${OUTPUT_DIR} diff --git a/docker/test/stateful/Dockerfile b/docker/test/stateful/Dockerfile index 65943bf6955..336dddfdc03 100644 --- a/docker/test/stateful/Dockerfile +++ b/docker/test/stateful/Dockerfile @@ -11,7 +11,8 @@ COPY s3downloader /s3downloader ENV DATASETS="hits visits" -CMD dpkg -i package_folder/clickhouse-common-static_*.deb; \ +CMD chmod 777 /; \ + dpkg -i package_folder/clickhouse-common-static_*.deb; \ dpkg -i package_folder/clickhouse-common-static-dbg_*.deb; \ dpkg -i package_folder/clickhouse-server_*.deb; \ dpkg -i package_folder/clickhouse-client_*.deb; \ @@ -36,4 +37,5 @@ CMD dpkg -i package_folder/clickhouse-common-static_*.deb; \ && clickhouse-client --query "RENAME TABLE datasets.hits_v1 TO test.hits" \ && clickhouse-client --query "RENAME TABLE datasets.visits_v1 TO test.visits" \ && clickhouse-client --query "SHOW TABLES FROM test" \ - && clickhouse-test --shard --zookeeper --no-stateless $SKIP_TESTS_OPTION 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt + && clickhouse-test --shard --zookeeper --no-stateless $SKIP_TESTS_OPTION 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt; \ + if [ $WITH_COVERAGE -eq 1 ]; then cp /*.profraw /profraw; fi diff --git a/docker/test/stateless/Dockerfile b/docker/test/stateless/Dockerfile index 16f8fc437bc..268fd618785 100644 --- a/docker/test/stateless/Dockerfile +++ b/docker/test/stateless/Dockerfile @@ -31,7 +31,8 @@ ENV TZ=Europe/Moscow RUN ln -snf /usr/share/zoneinfo/$TZ /etc/localtime && echo $TZ > /etc/timezone -CMD dpkg -i package_folder/clickhouse-common-static_*.deb; \ +CMD chmod 777 /; \ + dpkg -i package_folder/clickhouse-common-static_*.deb; \ dpkg -i package_folder/clickhouse-common-static-dbg_*.deb; \ dpkg -i package_folder/clickhouse-server_*.deb; \ dpkg -i package_folder/clickhouse-client_*.deb; \ @@ -52,4 +53,6 @@ CMD dpkg -i package_folder/clickhouse-common-static_*.deb; \ echo "TSAN_SYMBOLIZER_PATH=/usr/lib/llvm-8/bin/llvm-symbolizer" >> /etc/environment; \ echo "LLVM_SYMBOLIZER_PATH=/usr/lib/llvm-6.0/bin/llvm-symbolizer" >> /etc/environment; \ service zookeeper start; sleep 5; \ - service clickhouse-server start && sleep 5 && clickhouse-test --shard --zookeeper $ADDITIONAL_OPTIONS $SKIP_TESTS_OPTION 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt + service clickhouse-server start && sleep 5 && \ + clickhouse-test --shard --zookeeper $ADDITIONAL_OPTIONS $SKIP_TESTS_OPTION 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt; \ + if [ $WITH_COVERAGE -eq 1 ]; then cp /*.profraw /profraw; fi From 67d0f7f7cb77acb06fa6d93e764e7c082d5a9282 Mon Sep 17 00:00:00 2001 From: alesapin Date: Fri, 5 Jul 2019 18:16:10 +0300 Subject: [PATCH 10/62] Don't fail if no coverage info --- docker/test/stateful/Dockerfile | 2 +- docker/test/stateless/Dockerfile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docker/test/stateful/Dockerfile b/docker/test/stateful/Dockerfile index 336dddfdc03..29550519166 100644 --- a/docker/test/stateful/Dockerfile +++ b/docker/test/stateful/Dockerfile @@ -38,4 +38,4 @@ CMD chmod 777 /; \ && clickhouse-client --query "RENAME TABLE datasets.visits_v1 TO test.visits" \ && clickhouse-client --query "SHOW TABLES FROM test" \ && clickhouse-test --shard --zookeeper --no-stateless $SKIP_TESTS_OPTION 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt; \ - if [ $WITH_COVERAGE -eq 1 ]; then cp /*.profraw /profraw; fi + if [ $WITH_COVERAGE -eq 1 ]; then cp /*.profraw /profraw ||:; fi diff --git a/docker/test/stateless/Dockerfile b/docker/test/stateless/Dockerfile index 268fd618785..d1860e561d8 100644 --- a/docker/test/stateless/Dockerfile +++ b/docker/test/stateless/Dockerfile @@ -55,4 +55,4 @@ CMD chmod 777 /; \ service zookeeper start; sleep 5; \ service clickhouse-server start && sleep 5 && \ clickhouse-test --shard --zookeeper $ADDITIONAL_OPTIONS $SKIP_TESTS_OPTION 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt; \ - if [ $WITH_COVERAGE -eq 1 ]; then cp /*.profraw /profraw; fi + if [ $WITH_COVERAGE -eq 1 ]; then cp /*.profraw /profraw ||:; fi From 75b2d5c59a8bac478e46fcd445d0394b4035492e Mon Sep 17 00:00:00 2001 From: alesapin Date: Fri, 5 Jul 2019 18:18:48 +0300 Subject: [PATCH 11/62] Add ignore variable --- docker/test/coverage/Dockerfile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docker/test/coverage/Dockerfile b/docker/test/coverage/Dockerfile index c3837d6df12..78835ec8c41 100644 --- a/docker/test/coverage/Dockerfile +++ b/docker/test/coverage/Dockerfile @@ -26,8 +26,10 @@ RUN apt-get --allow-unauthenticated update -y \ ENV COVERAGE_DIR=/coverage_reports ENV SOURCE_DIR=/build ENV OUTPUT_DIR=/output +ENV IGNORE='.*contrib.*' + CMD dpkg -i /package_folder/clickhouse-common-static_*.deb; \ llvm-profdata-9 merge -sparse ${COVERAGE_DIR}/* -o clickhouse.profdata && \ - llvm-cov-9 export /usr/bin/clickhouse -instr-profile=clickhouse.profdata -arch=x86_64 -j=16 -format=lcov -skip-functions -ignore-filename-regex '.*contrib.*' > output.lcov && \ + llvm-cov-9 export /usr/bin/clickhouse -instr-profile=clickhouse.profdata -arch=x86_64 -j=16 -format=lcov -skip-functions -ignore-filename-regex $IGNORE > output.lcov && \ genhtml output.lcov --ignore-errors source --output-directory ${OUTPUT_DIR} From 3a825029cd92ab2a3d64691d98cd2bc8e005fd62 Mon Sep 17 00:00:00 2001 From: alesapin Date: Sat, 6 Jul 2019 23:20:16 +0300 Subject: [PATCH 12/62] add missed config files --- docker/test/stateful/Dockerfile | 9 +++++++++ docker/test/stateless/Dockerfile | 1 + 2 files changed, 10 insertions(+) diff --git a/docker/test/stateful/Dockerfile b/docker/test/stateful/Dockerfile index 29550519166..6e8017d0945 100644 --- a/docker/test/stateful/Dockerfile +++ b/docker/test/stateful/Dockerfile @@ -10,6 +10,7 @@ RUN apt-get update -y \ COPY s3downloader /s3downloader ENV DATASETS="hits visits" +ENV WITH_COVERAGE=0 CMD chmod 777 /; \ dpkg -i package_folder/clickhouse-common-static_*.deb; \ @@ -17,6 +18,14 @@ CMD chmod 777 /; \ dpkg -i package_folder/clickhouse-server_*.deb; \ dpkg -i package_folder/clickhouse-client_*.deb; \ dpkg -i package_folder/clickhouse-test_*.deb; \ + ln -s /usr/share/clickhouse-test/config/zookeeper.xml /etc/clickhouse-server/config.d/; \ + ln -s /usr/share/clickhouse-test/config/listen.xml /etc/clickhouse-server/config.d/; \ + ln -s /usr/share/clickhouse-test/config/part_log.xml /etc/clickhouse-server/config.d/; \ + ln -s /usr/share/clickhouse-test/config/log_queries.xml /etc/clickhouse-server/users.d/; \ + ln -s /usr/share/clickhouse-test/config/readonly.xml /etc/clickhouse-server/users.d/; \ + ln -s /usr/share/clickhouse-test/config/ints_dictionary.xml /etc/clickhouse-server/; \ + ln -s /usr/share/clickhouse-test/config/strings_dictionary.xml /etc/clickhouse-server/; \ + ln -s /usr/share/clickhouse-test/config/decimals_dictionary.xml /etc/clickhouse-server/; \ ln -s /usr/lib/llvm-8/bin/llvm-symbolizer /usr/bin/llvm-symbolizer; \ echo "TSAN_OPTIONS='halt_on_error=1 history_size=7'" >> /etc/environment; \ echo "TSAN_SYMBOLIZER_PATH=/usr/lib/llvm-8/bin/llvm-symbolizer" >> /etc/environment; \ diff --git a/docker/test/stateless/Dockerfile b/docker/test/stateless/Dockerfile index d1860e561d8..ea35cf03189 100644 --- a/docker/test/stateless/Dockerfile +++ b/docker/test/stateless/Dockerfile @@ -29,6 +29,7 @@ RUN apt-get update -y \ ENV TZ=Europe/Moscow RUN ln -snf /usr/share/zoneinfo/$TZ /etc/localtime && echo $TZ > /etc/timezone +ENV WITH_COVERAGE=0 CMD chmod 777 /; \ From bba2641bd0f30423f6fe0cac567968f73f41e928 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Mon, 8 Jul 2019 02:25:15 +0300 Subject: [PATCH 13/62] A few fixes for PVS-Studio --- dbms/src/Client/Connection.cpp | 14 +- .../Common/tests/parallel_aggregation2.cpp | 4 +- dbms/src/Interpreters/executeQuery.cpp | 5 +- libs/libmysqlxx/CMakeLists.txt | 1 - libs/libmysqlxx/include/mysqlxx/Manip.h | 483 ------------------ 5 files changed, 7 insertions(+), 500 deletions(-) delete mode 100644 libs/libmysqlxx/include/mysqlxx/Manip.h diff --git a/dbms/src/Client/Connection.cpp b/dbms/src/Client/Connection.cpp index 9651ef54e1b..b607fbf5276 100644 --- a/dbms/src/Client/Connection.cpp +++ b/dbms/src/Client/Connection.cpp @@ -595,7 +595,9 @@ Connection::Packet Connection::receivePacket() switch (res.type) { - case Protocol::Server::Data: + case Protocol::Server::Data: [[fallthrough]]; + case Protocol::Server::Totals: [[fallthrough]]; + case Protocol::Server::Extremes: res.block = receiveData(); return res; @@ -611,16 +613,6 @@ Connection::Packet Connection::receivePacket() res.profile_info = receiveProfileInfo(); return res; - case Protocol::Server::Totals: - /// Block with total values is passed in same form as ordinary block. The only difference is packed id. - res.block = receiveData(); - return res; - - case Protocol::Server::Extremes: - /// Same as above. - res.block = receiveData(); - return res; - case Protocol::Server::Log: res.block = receiveLogData(); return res; diff --git a/dbms/src/Common/tests/parallel_aggregation2.cpp b/dbms/src/Common/tests/parallel_aggregation2.cpp index 5bee292f58d..a2b26e82420 100644 --- a/dbms/src/Common/tests/parallel_aggregation2.cpp +++ b/dbms/src/Common/tests/parallel_aggregation2.cpp @@ -34,7 +34,7 @@ struct AggregateIndependent { results.reserve(num_threads); for (size_t i = 0; i < num_threads; ++i) - results.emplace_back(new Map); + results.emplace_back(std::make_unique()); for (size_t i = 0; i < num_threads; ++i) { @@ -77,7 +77,7 @@ struct AggregateIndependentWithSequentialKeysOptimization { results.reserve(num_threads); for (size_t i = 0; i < num_threads; ++i) - results.emplace_back(new Map); + results.emplace_back(std::make_unique()); for (size_t i = 0; i < num_threads; ++i) { diff --git a/dbms/src/Interpreters/executeQuery.cpp b/dbms/src/Interpreters/executeQuery.cpp index 4b6a5da2d67..16ef37efb6e 100644 --- a/dbms/src/Interpreters/executeQuery.cpp +++ b/dbms/src/Interpreters/executeQuery.cpp @@ -208,11 +208,10 @@ static std::tuple executeQueryImpl( { ReplaceQueryParameterVisitor visitor(context.getQueryParameters()); visitor.visit(ast); - } - /// Get new query after substitutions. - if (context.hasQueryParameters()) + /// Get new query after substitutions. query = serializeAST(*ast); + } logQuery(query.substr(0, settings.log_queries_cut_to_length), context, internal); diff --git a/libs/libmysqlxx/CMakeLists.txt b/libs/libmysqlxx/CMakeLists.txt index 166c10e69f7..263a031d7b0 100644 --- a/libs/libmysqlxx/CMakeLists.txt +++ b/libs/libmysqlxx/CMakeLists.txt @@ -12,7 +12,6 @@ add_library (mysqlxx include/mysqlxx/Connection.h include/mysqlxx/Exception.h - include/mysqlxx/Manip.h include/mysqlxx/mysqlxx.h include/mysqlxx/Null.h include/mysqlxx/Pool.h diff --git a/libs/libmysqlxx/include/mysqlxx/Manip.h b/libs/libmysqlxx/include/mysqlxx/Manip.h deleted file mode 100644 index 4b777b68f21..00000000000 --- a/libs/libmysqlxx/include/mysqlxx/Manip.h +++ /dev/null @@ -1,483 +0,0 @@ -#pragma once - -#include -#include -#include -#include - -#include -#include -#include - - -namespace mysqlxx -{ - -/** @brief Манипулятор ostream, который escape-ит строки для записи в tab delimited файл. - * Использование: tab_separated_ostr << mysqlxx::escape << x; - */ -enum escape_enum -{ - escape -}; - - -/** @brief Манипулятор ostream, который quote-ит строки для записи в MySQL запрос. - * Внимание! Не использует функции MySQL API, а использует свой метод quote-инга, - * который может быть некорректным при использовании некоторых кодировок - * (multi-byte attack), а также может оказаться некорректным при изменении libmysqlclient. - * Это сделано для увеличения производительности и это имеет значение. - * - * Использование: query << mysqlxx::quote << x; - */ -enum quote_enum -{ - quote -}; - - -struct EscapeManipResult -{ - std::ostream & ostr; - - EscapeManipResult(std::ostream & ostr_) : ostr(ostr_) {} - - std::ostream & operator<< (bool value) { return ostr << static_cast(value); } - std::ostream & operator<< (char value) { return ostr << static_cast(value); } - std::ostream & operator<< (unsigned char value) { return ostr << static_cast(value); } - std::ostream & operator<< (signed char value) { return ostr << static_cast(value); } - - template - typename std::enable_if::value, std::ostream &>::type - operator<< (T value) { return ostr << value; } - - std::ostream & operator<< (const LocalDate & value) { return ostr << value; } - std::ostream & operator<< (const LocalDateTime & value) { return ostr << value; } - - std::ostream & operator<< (const std::string & value) - { - writeEscapedData(value.data(), value.length()); - return ostr; - } - - - std::ostream & operator<< (const char * value) - { - while (const char * it = std::strpbrk(value, "\t\n\\")) - { - ostr.write(value, it - value); - switch (*it) - { - case '\t': - ostr.write("\\t", 2); - break; - case '\n': - ostr.write("\\n", 2); - break; - case '\\': - ostr.write("\\\\", 2); - break; - default: - ; - } - value = it + 1; - } - return ostr << value; - } - - - std::ostream & operator<< (const Value & string) - { - writeEscapedData(string.data(), string.size()); - return ostr; - } - - - std::ostream & operator<< (const Row & row) - { - for (size_t i = 0; i < row.size(); ++i) - { - if (i != 0) - ostr << '\t'; - - if (row[i].isNull()) - { - ostr << "\\N"; - continue; - } - - (*this) << row[i]; - } - - return ostr; - } - - - template - std::ostream & operator<< (const Null & value) - { - if(value.is_null) - ostr << "\\N"; - else - *this << value.data; - - return ostr ; - } - - - template - std::ostream & operator<< (const std::vector & value) - { - throw Poco::Exception(std::string(__PRETTY_FUNCTION__) + " is not implemented"); - } - -private: - - void writeEscapedData(const char * data, size_t length) - { - size_t i = 0; - - while (true) - { - size_t remaining_length = std::strlen(data); - (*this) << data; - if (i + remaining_length == length) - break; - - ostr.write("\\0", 2); - i += remaining_length + 1; - data += remaining_length + 1; - } - } -}; - -inline EscapeManipResult operator<< (std::ostream & ostr, escape_enum manip) -{ - return EscapeManipResult(ostr); -} - - -struct QuoteManipResult -{ -public: - std::ostream & ostr; - - QuoteManipResult(std::ostream & ostr_) : ostr(ostr_) {} - - std::ostream & operator<< (bool value) { return ostr << static_cast(value); } - std::ostream & operator<< (char value) { return ostr << static_cast(value); } - std::ostream & operator<< (unsigned char value) { return ostr << static_cast(value); } - std::ostream & operator<< (signed char value) { return ostr << static_cast(value); } - - template - typename std::enable_if::value, std::ostream &>::type - operator<< (T value) { return ostr << value; } - - std::ostream & operator<< (const LocalDate & value) { return ostr << '\'' << value << '\''; } - std::ostream & operator<< (const LocalDateTime & value) { return ostr << '\'' << value << '\''; } - - std::ostream & operator<< (const std::string & value) - { - ostr.put('\''); - writeEscapedData(value.data(), value.length()); - ostr.put('\''); - - return ostr; - } - - - std::ostream & operator<< (const char * value) - { - ostr.put('\''); - writeEscapedCString(value); - ostr.put('\''); - return ostr; - } - - template - std::ostream & operator<< (const Null & value) - { - if(value.is_null) - { - ostr << "\\N"; - } - else - { - *this << value.data; - } - return ostr ; - } - - template - std::ostream & operator<< (const std::vector & value) - { - throw Poco::Exception(std::string(__PRETTY_FUNCTION__) + " is not implemented"); - } - -private: - - void writeEscapedCString(const char * value) - { - while (const char * it = std::strpbrk(value, "'\\\"")) - { - ostr.write(value, it - value); - switch (*it) - { - case '"': - ostr.write("\\\"", 2); - break; - case '\'': - ostr.write("\\'", 2); - break; - case '\\': - ostr.write("\\\\", 2); - break; - default: - ; - } - value = it + 1; - } - ostr << value; - } - - - void writeEscapedData(const char * data, size_t length) - { - size_t i = 0; - - while (true) - { - size_t remaining_length = std::strlen(data); - writeEscapedCString(data); - if (i + remaining_length == length) - break; - - ostr.write("\\0", 2); - i += remaining_length + 1; - data += remaining_length + 1; - } - } -}; - -inline QuoteManipResult operator<< (std::ostream & ostr, quote_enum manip) -{ - return QuoteManipResult(ostr); -} - - -/** Манипулятор istream, позволяет считывать значения из tab delimited файла. - */ -enum unescape_enum -{ - unescape -}; - - -/** Манипулятор istream, который позволяет читать значения в кавычках или без. - */ -enum unquote_enum -{ - unquote -}; - - -inline void parseEscapeSequence(std::istream & istr, std::string & value) -{ - char c = istr.get(); - if (!istr.good()) - throw Poco::Exception("Cannot parse string: unexpected end of input."); - - switch(c) - { - case 'b': - value.push_back('\b'); - break; - case 'f': - value.push_back('\f'); - break; - case 'n': - value.push_back('\n'); - break; - case 'r': - value.push_back('\r'); - break; - case 't': - value.push_back('\t'); - break; - default: - value.push_back(c); - break; - } -} - - -struct UnEscapeManipResult -{ - std::istream & istr; - - UnEscapeManipResult(std::istream & istr_) : istr(istr_) {} - - std::istream & operator>> (bool & value) { int tmp = 0; istr >> tmp; value = tmp; return istr; } - std::istream & operator>> (char & value) { int tmp = 0; istr >> tmp; value = tmp; return istr; } - std::istream & operator>> (unsigned char & value) { int tmp = 0; istr >> tmp; value = tmp; return istr; } - std::istream & operator>> (signed char & value) { int tmp = 0; istr >> tmp; value = tmp; return istr; } - - template - typename std::enable_if::value, std::istream &>::type - operator>> (T & value) { return istr >> value; } - - std::istream & operator>> (std::string & value) - { - value.clear(); - - char c; - while (1) - { - istr.get(c); - if (!istr.good()) - break; - - switch (c) - { - case '\\': - parseEscapeSequence(istr, value); - break; - - case '\t': - istr.unget(); - return istr; - break; - - case '\n': - istr.unget(); - return istr; - break; - - default: - value.push_back(c); - break; - } - } - return istr; - } - - /// Чтение NULL-able типа. - template - std::istream & operator>> (Null & value) - { - char c; - istr.get(c); - if (c == '\\' && istr.peek() == 'N') - { - value.is_null = true; - istr.ignore(); - } - else - { - istr.unget(); - value.is_null = false; - *this >> value.data; - } - return istr; - } - - std::istream & operator>> (LocalDate & value) - { - std::string s; - (*this) >> s; - value = LocalDate(s); - return istr; - } - - std::istream & operator>> (LocalDateTime & value) - { - std::string s; - (*this) >> s; - value = LocalDateTime(s); - return istr; - } - - template - std::istream & operator>> (std::vector & value) - { - throw Poco::Exception(std::string(__PRETTY_FUNCTION__) + " is not implemented"); - } -}; - -inline UnEscapeManipResult operator>> (std::istream & istr, unescape_enum manip) -{ - return UnEscapeManipResult(istr); -} - - -struct UnQuoteManipResult -{ -public: - std::istream & istr; - - UnQuoteManipResult(std::istream & istr_) : istr(istr_) {} - - std::istream & operator>> (bool & value) { int tmp = 0; istr >> tmp; value = tmp; return istr; } - std::istream & operator>> (char & value) { int tmp = 0; istr >> tmp; value = tmp; return istr; } - std::istream & operator>> (unsigned char & value) { int tmp = 0; istr >> tmp; value = tmp; return istr; } - std::istream & operator>> (signed char & value) { int tmp = 0; istr >> tmp; value = tmp; return istr; } - - template - typename std::enable_if::value, std::istream &>::type - operator>> (T & value) { return istr >> value; } - - std::istream & operator>> (std::string & value) - { - value.clear(); - readQuote(); - - char c; - while (1) - { - istr.get(c); - if (!istr.good()) - break; - - switch (c) - { - case '\\': - parseEscapeSequence(istr, value); - break; - - case '\'': - return istr; - break; - - default: - value.push_back(c); - break; - } - } - throw Poco::Exception("Cannot parse string: unexpected end of input."); - } - - template - std::istream & operator>> (std::vector & value) - { - throw Poco::Exception(std::string(__PRETTY_FUNCTION__) + " is not implemented"); - } - -private: - - void readQuote() - { - char c = istr.get(); - if (!istr.good()) - throw Poco::Exception("Cannot parse string: unexpected end of input."); - if (c != '\'') - throw Poco::Exception("Cannot parse string: missing opening single quote."); - } -}; - -inline UnQuoteManipResult operator>> (std::istream & istr, unquote_enum manip) -{ - return UnQuoteManipResult(istr); -} - - -} From 19059ef65062817eb3e9b8f4b05e4587c35ad9f5 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Mon, 8 Jul 2019 02:29:54 +0300 Subject: [PATCH 14/62] One more fix for PVS-Studio --- dbms/src/Storages/MergeTree/MergedBlockOutputStream.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dbms/src/Storages/MergeTree/MergedBlockOutputStream.cpp b/dbms/src/Storages/MergeTree/MergedBlockOutputStream.cpp index 38c2a612b1d..8b38f1ff32e 100644 --- a/dbms/src/Storages/MergeTree/MergedBlockOutputStream.cpp +++ b/dbms/src/Storages/MergeTree/MergedBlockOutputStream.cpp @@ -392,7 +392,7 @@ void MergedBlockOutputStream::writeImpl(const Block & block, const IColumn::Perm auto & stream = *skip_indices_streams[i]; size_t prev_pos = 0; - size_t current_mark = 0; + size_t skip_index_current_mark = 0; while (prev_pos < rows) { UInt64 limit = 0; @@ -402,7 +402,7 @@ void MergedBlockOutputStream::writeImpl(const Block & block, const IColumn::Perm } else { - limit = index_granularity.getMarkRows(current_mark); + limit = index_granularity.getMarkRows(skip_index_current_mark); if (skip_indices_aggregators[i]->empty()) { skip_indices_aggregators[i] = index->createIndexAggregator(); @@ -435,7 +435,7 @@ void MergedBlockOutputStream::writeImpl(const Block & block, const IColumn::Perm } } prev_pos = pos; - current_mark++; + ++skip_index_current_mark; } } } From 5ffcaa462ae17e391d27f8ea317c98a96edef153 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Mon, 8 Jul 2019 03:08:49 +0300 Subject: [PATCH 15/62] Addition to prev. revision --- libs/libmysqlxx/include/mysqlxx/mysqlxx.h | 1 - libs/libmysqlxx/src/tests/mysqlxx_test.cpp | 16 ++++++++-------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/libs/libmysqlxx/include/mysqlxx/mysqlxx.h b/libs/libmysqlxx/include/mysqlxx/mysqlxx.h index f9da8ec5c04..179d550519e 100644 --- a/libs/libmysqlxx/include/mysqlxx/mysqlxx.h +++ b/libs/libmysqlxx/include/mysqlxx/mysqlxx.h @@ -2,7 +2,6 @@ #include #include -#include #include #include #include diff --git a/libs/libmysqlxx/src/tests/mysqlxx_test.cpp b/libs/libmysqlxx/src/tests/mysqlxx_test.cpp index d8c15eecaba..d99900b1a39 100644 --- a/libs/libmysqlxx/src/tests/mysqlxx_test.cpp +++ b/libs/libmysqlxx/src/tests/mysqlxx_test.cpp @@ -2,7 +2,7 @@ #include -int main(int argc, char ** argv) +int main(int, char **) { try { @@ -25,10 +25,10 @@ int main(int argc, char ** argv) << ", " << row[1].getDate() << ", " << row[1].getDateTime() << std::endl - << mysqlxx::escape << row[1].getDate() << ", " << mysqlxx::escape << row[1].getDateTime() << std::endl - << mysqlxx::quote << row[1].getDate() << ", " << mysqlxx::quote << row[1].getDateTime() << std::endl - << mysqlxx::escape << row[1].getDate() << ", " << mysqlxx::escape << row[1].getDateTime() << std::endl - << mysqlxx::quote << row[1].getDate() << ", " << mysqlxx::quote << row[1].getDateTime() << std::endl + << row[1].getDate() << ", " << row[1].getDateTime() << std::endl + << row[1].getDate() << ", " << row[1].getDateTime() << std::endl + << row[1].getDate() << ", " << row[1].getDateTime() << std::endl + << row[1].getDate() << ", " << row[1].getDateTime() << std::endl ; time_t t1 = row[0]; @@ -51,7 +51,7 @@ int main(int argc, char ** argv) mysqlxx::UseQueryResult result = connection.query("SELECT 'abc\\\\def' x").use(); mysqlxx::Row row = result.fetch(); std::cerr << row << std::endl; - std::cerr << mysqlxx::escape << row << std::endl; + std::cerr << row << std::endl; } { @@ -71,7 +71,7 @@ int main(int argc, char ** argv) for (Queries::iterator it = queries.begin(); it != queries.end(); ++it) { std::cerr << it->str() << std::endl; - std::cerr << mysqlxx::escape << it->store().at(0) << std::endl; + std::cerr << it->store().at(0) << std::endl; } } @@ -95,7 +95,7 @@ int main(int argc, char ** argv) for (Queries::iterator it = queries.begin(); it != queries.end(); ++it) { std::cerr << it->str() << std::endl; - std::cerr << mysqlxx::escape << it->store().at(0) << std::endl; + std::cerr << it->store().at(0) << std::endl; } } From e64e520e5b21ac6f81053fe093265a4f3bbfcd61 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Mon, 8 Jul 2019 03:11:39 +0300 Subject: [PATCH 16/62] One more fix for PVS-Studio --- dbms/src/Common/formatIPv6.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/Common/formatIPv6.cpp b/dbms/src/Common/formatIPv6.cpp index f8100557ba2..9ec6c427ab4 100644 --- a/dbms/src/Common/formatIPv6.cpp +++ b/dbms/src/Common/formatIPv6.cpp @@ -153,7 +153,7 @@ void formatIPv6(const unsigned char * src, char *& dst, UInt8 zeroed_tail_bytes_ } /// Was it a trailing run of 0x00's? - if (best.base != -1 && size_t(best.base + best.len) == words.size()) + if (best.base != -1 && size_t(best.base) + size_t(best.len) == words.size()) *dst++ = ':'; *dst++ = '\0'; From 03712aabec8d958104fa546648825ec568d8dcb5 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Mon, 8 Jul 2019 03:16:39 +0300 Subject: [PATCH 17/62] Add two more warnings from -Weverything --- dbms/CMakeLists.txt | 2 +- dbms/src/Core/Field.h | 12 ++++++------ dbms/src/Core/SettingsCommon.h | 2 +- dbms/src/Formats/ProtobufReader.cpp | 2 +- dbms/src/Functions/CRC32.cpp | 2 +- dbms/src/Functions/URL/decodeURLComponent.cpp | 2 +- dbms/src/Functions/isValidUTF8.cpp | 2 +- dbms/src/Functions/lengthUTF8.cpp | 2 +- dbms/src/Functions/reverseUTF8.cpp | 2 +- dbms/src/Functions/toValidUTF8.cpp | 2 +- dbms/src/IO/ReadHelpers.h | 4 ++-- dbms/src/IO/WriteHelpers.h | 2 +- 12 files changed, 18 insertions(+), 18 deletions(-) diff --git a/dbms/CMakeLists.txt b/dbms/CMakeLists.txt index 98428d42540..c3b4106ade8 100644 --- a/dbms/CMakeLists.txt +++ b/dbms/CMakeLists.txt @@ -48,7 +48,7 @@ if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang") set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wshadow -Wshadow-uncaptured-local -Wextra-semi -Wcomma -Winconsistent-missing-destructor-override -Wunused-exception-parameter -Wcovered-switch-default -Wold-style-cast -Wrange-loop-analysis -Wunused-member-function -Wunreachable-code -Wunreachable-code-return -Wnewline-eof -Wembedded-directive -Wgnu-case-range -Wunused-macros -Wconditional-uninitialized -Wdeprecated -Wundef -Wreserved-id-macro -Wredundant-parens -Wzero-as-null-pointer-constant") if (WEVERYTHING) - set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Weverything -Wno-c++98-compat -Wno-c++98-compat-pedantic -Wno-missing-noreturn -Wno-padded -Wno-switch-enum -Wno-shadow-field-in-constructor -Wno-deprecated-dynamic-exception-spec -Wno-float-equal -Wno-weak-vtables -Wno-shift-sign-overflow -Wno-sign-conversion -Wno-conversion -Wno-exit-time-destructors -Wno-undefined-func-template -Wno-documentation-unknown-command -Wno-missing-variable-declarations -Wno-unused-template -Wno-global-constructors -Wno-c99-extensions -Wno-missing-prototypes -Wno-weak-template-vtables -Wno-zero-length-array -Wno-gnu-anonymous-struct -Wno-nested-anon-types -Wno-double-promotion -Wno-disabled-macro-expansion -Wno-used-but-marked-unused -Wno-vla-extension -Wno-vla -Wno-packed") + set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Weverything -Wno-c++98-compat -Wno-c++98-compat-pedantic -Wno-padded -Wno-switch-enum -Wno-shadow-field-in-constructor -Wno-deprecated-dynamic-exception-spec -Wno-float-equal -Wno-weak-vtables -Wno-shift-sign-overflow -Wno-sign-conversion -Wno-conversion -Wno-exit-time-destructors -Wno-undefined-func-template -Wno-documentation-unknown-command -Wno-missing-variable-declarations -Wno-unused-template -Wno-global-constructors -Wno-c99-extensions -Wno-missing-prototypes -Wno-weak-template-vtables -Wno-zero-length-array -Wno-gnu-anonymous-struct -Wno-nested-anon-types -Wno-double-promotion -Wno-disabled-macro-expansion -Wno-vla-extension -Wno-vla -Wno-packed") # TODO Enable conversion, sign-conversion, double-promotion warnings. endif () diff --git a/dbms/src/Core/Field.h b/dbms/src/Core/Field.h index 832b45cd6f8..156f272312b 100644 --- a/dbms/src/Core/Field.h +++ b/dbms/src/Core/Field.h @@ -717,8 +717,8 @@ class WriteBuffer; /// It is assumed that all elements of the array have the same type. void readBinary(Array & x, ReadBuffer & buf); -inline void readText(Array &, ReadBuffer &) { throw Exception("Cannot read Array.", ErrorCodes::NOT_IMPLEMENTED); } -inline void readQuoted(Array &, ReadBuffer &) { throw Exception("Cannot read Array.", ErrorCodes::NOT_IMPLEMENTED); } +[[noreturn]] inline void readText(Array &, ReadBuffer &) { throw Exception("Cannot read Array.", ErrorCodes::NOT_IMPLEMENTED); } +[[noreturn]] inline void readQuoted(Array &, ReadBuffer &) { throw Exception("Cannot read Array.", ErrorCodes::NOT_IMPLEMENTED); } /// It is assumed that all elements of the array have the same type. /// Also write size and type into buf. UInt64 and Int64 is written in variadic size form @@ -726,16 +726,16 @@ void writeBinary(const Array & x, WriteBuffer & buf); void writeText(const Array & x, WriteBuffer & buf); -inline void writeQuoted(const Array &, WriteBuffer &) { throw Exception("Cannot write Array quoted.", ErrorCodes::NOT_IMPLEMENTED); } +[[noreturn]] inline void writeQuoted(const Array &, WriteBuffer &) { throw Exception("Cannot write Array quoted.", ErrorCodes::NOT_IMPLEMENTED); } void readBinary(Tuple & x, ReadBuffer & buf); -inline void readText(Tuple &, ReadBuffer &) { throw Exception("Cannot read Tuple.", ErrorCodes::NOT_IMPLEMENTED); } -inline void readQuoted(Tuple &, ReadBuffer &) { throw Exception("Cannot read Tuple.", ErrorCodes::NOT_IMPLEMENTED); } +[[noreturn]] inline void readText(Tuple &, ReadBuffer &) { throw Exception("Cannot read Tuple.", ErrorCodes::NOT_IMPLEMENTED); } +[[noreturn]] inline void readQuoted(Tuple &, ReadBuffer &) { throw Exception("Cannot read Tuple.", ErrorCodes::NOT_IMPLEMENTED); } void writeBinary(const Tuple & x, WriteBuffer & buf); void writeText(const Tuple & x, WriteBuffer & buf); -inline void writeQuoted(const Tuple &, WriteBuffer &) { throw Exception("Cannot write Tuple quoted.", ErrorCodes::NOT_IMPLEMENTED); } +[[noreturn]] inline void writeQuoted(const Tuple &, WriteBuffer &) { throw Exception("Cannot write Tuple quoted.", ErrorCodes::NOT_IMPLEMENTED); } } diff --git a/dbms/src/Core/SettingsCommon.h b/dbms/src/Core/SettingsCommon.h index b30b5b50c68..08064096133 100644 --- a/dbms/src/Core/SettingsCommon.h +++ b/dbms/src/Core/SettingsCommon.h @@ -275,7 +275,7 @@ namespace details { static void serializeName(const StringRef & name, WriteBuffer & buf); static String deserializeName(ReadBuffer & buf); - static void throwNameNotFound(const StringRef & name); + [[noreturn]] static void throwNameNotFound(const StringRef & name); }; } diff --git a/dbms/src/Formats/ProtobufReader.cpp b/dbms/src/Formats/ProtobufReader.cpp index 2c0ea1251f4..54e8a050316 100644 --- a/dbms/src/Formats/ProtobufReader.cpp +++ b/dbms/src/Formats/ProtobufReader.cpp @@ -42,7 +42,7 @@ namespace Int64 decodeZigZag(UInt64 n) { return static_cast((n >> 1) ^ (~(n & 1) + 1)); } - void unknownFormat() + [[noreturn]] void unknownFormat() { throw Exception("Protobuf messages are corrupted or don't match the provided schema. Please note that Protobuf stream is length-delimited: every message is prefixed by its length in varint.", ErrorCodes::UNKNOWN_PROTOBUF_FORMAT); } diff --git a/dbms/src/Functions/CRC32.cpp b/dbms/src/Functions/CRC32.cpp index 95b93701783..80e0f163571 100644 --- a/dbms/src/Functions/CRC32.cpp +++ b/dbms/src/Functions/CRC32.cpp @@ -41,7 +41,7 @@ struct CRC32Impl } } - static void array(const ColumnString::Offsets & /*offsets*/, PaddedPODArray & /*res*/) + [[noreturn]] static void array(const ColumnString::Offsets & /*offsets*/, PaddedPODArray & /*res*/) { throw Exception("Cannot apply function CRC32 to Array argument", ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT); } diff --git a/dbms/src/Functions/URL/decodeURLComponent.cpp b/dbms/src/Functions/URL/decodeURLComponent.cpp index ee60498af69..008e3b0645f 100644 --- a/dbms/src/Functions/URL/decodeURLComponent.cpp +++ b/dbms/src/Functions/URL/decodeURLComponent.cpp @@ -89,7 +89,7 @@ struct DecodeURLComponentImpl res_data.resize(res_offset); } - static void vector_fixed(const ColumnString::Chars &, size_t, ColumnString::Chars &) + [[noreturn]] static void vector_fixed(const ColumnString::Chars &, size_t, ColumnString::Chars &) { throw Exception("Column of type FixedString is not supported by URL functions", ErrorCodes::ILLEGAL_COLUMN); } diff --git a/dbms/src/Functions/isValidUTF8.cpp b/dbms/src/Functions/isValidUTF8.cpp index 09b7b81030e..ff3c4466115 100644 --- a/dbms/src/Functions/isValidUTF8.cpp +++ b/dbms/src/Functions/isValidUTF8.cpp @@ -309,7 +309,7 @@ SOFTWARE. res[i] = isValidUTF8(data.data() + i * n, n); } - static void array(const ColumnString::Offsets &, PaddedPODArray &) + [[noreturn]] static void array(const ColumnString::Offsets &, PaddedPODArray &) { throw Exception("Cannot apply function isValidUTF8 to Array argument", ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT); } diff --git a/dbms/src/Functions/lengthUTF8.cpp b/dbms/src/Functions/lengthUTF8.cpp index ba2214fc9d7..2549e32b8f7 100644 --- a/dbms/src/Functions/lengthUTF8.cpp +++ b/dbms/src/Functions/lengthUTF8.cpp @@ -48,7 +48,7 @@ struct LengthUTF8Impl } } - static void array(const ColumnString::Offsets &, PaddedPODArray &) + [[noreturn]] static void array(const ColumnString::Offsets &, PaddedPODArray &) { throw Exception("Cannot apply function lengthUTF8 to Array argument", ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT); } diff --git a/dbms/src/Functions/reverseUTF8.cpp b/dbms/src/Functions/reverseUTF8.cpp index 188b48bf7dc..261aa6935e2 100644 --- a/dbms/src/Functions/reverseUTF8.cpp +++ b/dbms/src/Functions/reverseUTF8.cpp @@ -61,7 +61,7 @@ struct ReverseUTF8Impl } } - static void vector_fixed(const ColumnString::Chars &, size_t, ColumnString::Chars &) + [[noreturn]] static void vector_fixed(const ColumnString::Chars &, size_t, ColumnString::Chars &) { throw Exception("Cannot apply function reverseUTF8 to fixed string.", ErrorCodes::ILLEGAL_COLUMN); } diff --git a/dbms/src/Functions/toValidUTF8.cpp b/dbms/src/Functions/toValidUTF8.cpp index a1dc78c7915..63e3bdab21d 100644 --- a/dbms/src/Functions/toValidUTF8.cpp +++ b/dbms/src/Functions/toValidUTF8.cpp @@ -124,7 +124,7 @@ struct ToValidUTF8Impl write_buffer.finish(); } - static void vector_fixed(const ColumnString::Chars &, size_t, ColumnString::Chars &) + [[noreturn]] static void vector_fixed(const ColumnString::Chars &, size_t, ColumnString::Chars &) { throw Exception("Column of type FixedString is not supported by toValidUTF8 function", ErrorCodes::ILLEGAL_COLUMN); } diff --git a/dbms/src/IO/ReadHelpers.h b/dbms/src/IO/ReadHelpers.h index a24c1b4546c..e1f0480a7a9 100644 --- a/dbms/src/IO/ReadHelpers.h +++ b/dbms/src/IO/ReadHelpers.h @@ -676,7 +676,7 @@ inline void readText(String & x, ReadBuffer & buf) { readEscapedString(x, buf); inline void readText(LocalDate & x, ReadBuffer & buf) { readDateText(x, buf); } inline void readText(LocalDateTime & x, ReadBuffer & buf) { readDateTimeText(x, buf); } inline void readText(UUID & x, ReadBuffer & buf) { readUUIDText(x, buf); } -inline void readText(UInt128 &, ReadBuffer &) +[[noreturn]] inline void readText(UInt128 &, ReadBuffer &) { /** Because UInt128 isn't a natural type, without arithmetic operator and only use as an intermediary type -for UUID- * it should never arrive here. But because we used the DataTypeNumber class we should have at least a definition of it. @@ -755,7 +755,7 @@ inline void readCSV(String & x, ReadBuffer & buf, const FormatSettings::CSV & se inline void readCSV(LocalDate & x, ReadBuffer & buf) { readCSVSimple(x, buf); } inline void readCSV(LocalDateTime & x, ReadBuffer & buf) { readCSVSimple(x, buf); } inline void readCSV(UUID & x, ReadBuffer & buf) { readCSVSimple(x, buf); } -inline void readCSV(UInt128 &, ReadBuffer &) +[[noreturn]] inline void readCSV(UInt128 &, ReadBuffer &) { /** Because UInt128 isn't a natural type, without arithmetic operator and only use as an intermediary type -for UUID- * it should never arrive here. But because we used the DataTypeNumber class we should have at least a definition of it. diff --git a/dbms/src/IO/WriteHelpers.h b/dbms/src/IO/WriteHelpers.h index b494f863421..073888c8712 100644 --- a/dbms/src/IO/WriteHelpers.h +++ b/dbms/src/IO/WriteHelpers.h @@ -830,7 +830,7 @@ inline void writeCSV(const String & x, WriteBuffer & buf) { writeCSVString<>(x, inline void writeCSV(const LocalDate & x, WriteBuffer & buf) { writeDoubleQuoted(x, buf); } inline void writeCSV(const LocalDateTime & x, WriteBuffer & buf) { writeDoubleQuoted(x, buf); } inline void writeCSV(const UUID & x, WriteBuffer & buf) { writeDoubleQuoted(x, buf); } -inline void writeCSV(const UInt128, WriteBuffer &) +[[noreturn]] inline void writeCSV(const UInt128, WriteBuffer &) { /** Because UInt128 isn't a natural type, without arithmetic operator and only use as an intermediary type -for UUID- * it should never arrive here. But because we used the DataTypeNumber class we should have at least a definition of it. From 553e6a273e3d7591a0cef445ea45aabccf8ab3e8 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Mon, 8 Jul 2019 03:51:43 +0300 Subject: [PATCH 18/62] Two more fixes for PVS-Studio --- dbms/programs/copier/ClusterCopier.cpp | 1 - dbms/programs/local/LocalServer.cpp | 1 - dbms/programs/odbc-bridge/ODBCBridge.cpp | 1 - dbms/programs/server/HTTPHandler.cpp | 4 ++-- dbms/programs/server/MySQLHandler.cpp | 2 +- dbms/programs/server/Server.cpp | 1 - dbms/programs/server/TCPHandler.cpp | 2 +- dbms/src/Dictionaries/ClickHouseDictionarySource.h | 1 + dbms/src/Dictionaries/ExecutableDictionarySource.h | 1 + dbms/src/Dictionaries/HTTPDictionarySource.h | 1 + dbms/src/Dictionaries/LibraryDictionarySource.h | 1 + dbms/src/Dictionaries/MySQLDictionarySource.h | 1 + dbms/src/Dictionaries/XDBCDictionarySource.h | 1 + dbms/src/Interpreters/Context.cpp | 1 + dbms/src/Interpreters/Context.h | 6 ++++-- dbms/src/Interpreters/InterpreterCreateQuery.cpp | 2 +- dbms/src/Interpreters/executeQuery.cpp | 2 +- 17 files changed, 17 insertions(+), 12 deletions(-) diff --git a/dbms/programs/copier/ClusterCopier.cpp b/dbms/programs/copier/ClusterCopier.cpp index e48ab92c4f8..6bba1ceb5f3 100644 --- a/dbms/programs/copier/ClusterCopier.cpp +++ b/dbms/programs/copier/ClusterCopier.cpp @@ -2174,7 +2174,6 @@ void ClusterCopierApp::mainImpl() SCOPE_EXIT(context->shutdown()); context->setConfig(loaded_config.configuration); - context->setGlobalContext(*context); context->setApplicationType(Context::ApplicationType::LOCAL); context->setPath(process_path); diff --git a/dbms/programs/local/LocalServer.cpp b/dbms/programs/local/LocalServer.cpp index 3e3b249fe82..8c87a5d0fa4 100644 --- a/dbms/programs/local/LocalServer.cpp +++ b/dbms/programs/local/LocalServer.cpp @@ -131,7 +131,6 @@ try context = std::make_unique(Context::createGlobal()); - context->setGlobalContext(*context); context->setApplicationType(Context::ApplicationType::LOCAL); tryInitPath(); diff --git a/dbms/programs/odbc-bridge/ODBCBridge.cpp b/dbms/programs/odbc-bridge/ODBCBridge.cpp index aaacdfca826..6327cee3e27 100644 --- a/dbms/programs/odbc-bridge/ODBCBridge.cpp +++ b/dbms/programs/odbc-bridge/ODBCBridge.cpp @@ -160,7 +160,6 @@ int ODBCBridge::main(const std::vector & /*args*/) http_params->setKeepAliveTimeout(keep_alive_timeout); context = std::make_shared(Context::createGlobal()); - context->setGlobalContext(*context); auto server = Poco::Net::HTTPServer( new HandlerFactory("ODBCRequestHandlerFactory-factory", keep_alive_timeout, context), server_pool, socket, http_params); diff --git a/dbms/programs/server/HTTPHandler.cpp b/dbms/programs/server/HTTPHandler.cpp index 2349ab337f0..18ca9b84518 100644 --- a/dbms/programs/server/HTTPHandler.cpp +++ b/dbms/programs/server/HTTPHandler.cpp @@ -211,7 +211,7 @@ void HTTPHandler::processQuery( Output & used_output) { Context context = server.context(); - context.setGlobalContext(server.context()); + context.makeGlobalContext(); CurrentThread::QueryScope query_scope(context); @@ -285,7 +285,7 @@ void HTTPHandler::processQuery( session = context.acquireSession(session_id, session_timeout, session_check == "1"); context = *session; - context.setSessionContext(*session); + context.makeSessionContext(); } SCOPE_EXIT({ diff --git a/dbms/programs/server/MySQLHandler.cpp b/dbms/programs/server/MySQLHandler.cpp index a935f13e82a..30d8037ce34 100644 --- a/dbms/programs/server/MySQLHandler.cpp +++ b/dbms/programs/server/MySQLHandler.cpp @@ -48,7 +48,7 @@ MySQLHandler::MySQLHandler(IServer & server_, const Poco::Net::StreamSocket & so void MySQLHandler::run() { connection_context = server.context(); - connection_context.setSessionContext(connection_context); + connection_context.makeSessionContext(); connection_context.setDefaultFormat("MySQLWire"); in = std::make_shared(socket()); diff --git a/dbms/programs/server/Server.cpp b/dbms/programs/server/Server.cpp index 9267214e1ba..2ccdc5289f2 100644 --- a/dbms/programs/server/Server.cpp +++ b/dbms/programs/server/Server.cpp @@ -187,7 +187,6 @@ int Server::main(const std::vector & /*args*/) * settings, available functions, data types, aggregate functions, databases... */ global_context = std::make_unique(Context::createGlobal()); - global_context->setGlobalContext(*global_context); global_context->setApplicationType(Context::ApplicationType::SERVER); bool has_zookeeper = config().has("zookeeper"); diff --git a/dbms/programs/server/TCPHandler.cpp b/dbms/programs/server/TCPHandler.cpp index b436c213e7d..bfbc0a590a2 100644 --- a/dbms/programs/server/TCPHandler.cpp +++ b/dbms/programs/server/TCPHandler.cpp @@ -54,7 +54,7 @@ void TCPHandler::runImpl() ThreadStatus thread_status; connection_context = server.context(); - connection_context.setSessionContext(connection_context); + connection_context.makeSessionContext(); Settings global_settings = connection_context.getSettings(); diff --git a/dbms/src/Dictionaries/ClickHouseDictionarySource.h b/dbms/src/Dictionaries/ClickHouseDictionarySource.h index 2603f24fa0f..991782b1549 100644 --- a/dbms/src/Dictionaries/ClickHouseDictionarySource.h +++ b/dbms/src/Dictionaries/ClickHouseDictionarySource.h @@ -27,6 +27,7 @@ public: /// copy-constructor is provided in order to support cloneability ClickHouseDictionarySource(const ClickHouseDictionarySource & other); + ClickHouseDictionarySource & operator=(const ClickHouseDictionarySource &) = delete; BlockInputStreamPtr loadAll() override; diff --git a/dbms/src/Dictionaries/ExecutableDictionarySource.h b/dbms/src/Dictionaries/ExecutableDictionarySource.h index c5ace9ab78f..9816161a70e 100644 --- a/dbms/src/Dictionaries/ExecutableDictionarySource.h +++ b/dbms/src/Dictionaries/ExecutableDictionarySource.h @@ -23,6 +23,7 @@ public: const Context & context); ExecutableDictionarySource(const ExecutableDictionarySource & other); + ExecutableDictionarySource & operator=(const ExecutableDictionarySource &) = delete; BlockInputStreamPtr loadAll() override; diff --git a/dbms/src/Dictionaries/HTTPDictionarySource.h b/dbms/src/Dictionaries/HTTPDictionarySource.h index 92980e183e3..78fe5193533 100644 --- a/dbms/src/Dictionaries/HTTPDictionarySource.h +++ b/dbms/src/Dictionaries/HTTPDictionarySource.h @@ -26,6 +26,7 @@ public: const Context & context); HTTPDictionarySource(const HTTPDictionarySource & other); + HTTPDictionarySource & operator=(const HTTPDictionarySource &) = delete; BlockInputStreamPtr loadAll() override; diff --git a/dbms/src/Dictionaries/LibraryDictionarySource.h b/dbms/src/Dictionaries/LibraryDictionarySource.h index a667b286be3..d09e5eee691 100644 --- a/dbms/src/Dictionaries/LibraryDictionarySource.h +++ b/dbms/src/Dictionaries/LibraryDictionarySource.h @@ -35,6 +35,7 @@ public: Block & sample_block); LibraryDictionarySource(const LibraryDictionarySource & other); + LibraryDictionarySource & operator=(const LibraryDictionarySource &) = delete; ~LibraryDictionarySource() override; diff --git a/dbms/src/Dictionaries/MySQLDictionarySource.h b/dbms/src/Dictionaries/MySQLDictionarySource.h index 30447a49aea..cfc45f42bb3 100644 --- a/dbms/src/Dictionaries/MySQLDictionarySource.h +++ b/dbms/src/Dictionaries/MySQLDictionarySource.h @@ -37,6 +37,7 @@ public: /// copy-constructor is provided in order to support cloneability MySQLDictionarySource(const MySQLDictionarySource & other); + MySQLDictionarySource & operator=(const MySQLDictionarySource &) = delete; BlockInputStreamPtr loadAll() override; diff --git a/dbms/src/Dictionaries/XDBCDictionarySource.h b/dbms/src/Dictionaries/XDBCDictionarySource.h index 5197300c341..253f802d8fd 100644 --- a/dbms/src/Dictionaries/XDBCDictionarySource.h +++ b/dbms/src/Dictionaries/XDBCDictionarySource.h @@ -36,6 +36,7 @@ public: /// copy-constructor is provided in order to support cloneability XDBCDictionarySource(const XDBCDictionarySource & other); + XDBCDictionarySource & operator=(const XDBCDictionarySource &) = delete; BlockInputStreamPtr loadAll() override; diff --git a/dbms/src/Interpreters/Context.cpp b/dbms/src/Interpreters/Context.cpp index 2909de8a60b..c89f4273b3e 100644 --- a/dbms/src/Interpreters/Context.cpp +++ b/dbms/src/Interpreters/Context.cpp @@ -305,6 +305,7 @@ Context Context::createGlobal(std::unique_ptr runtime Context res; res.shared = std::make_shared(std::move(runtime_components_factory)); res.quota = std::make_shared(); + res.global_context = &res; return res; } diff --git a/dbms/src/Interpreters/Context.h b/dbms/src/Interpreters/Context.h index 45f4006b0cd..c49f509d2a8 100644 --- a/dbms/src/Interpreters/Context.h +++ b/dbms/src/Interpreters/Context.h @@ -133,7 +133,7 @@ private: Tables table_function_results; /// Temporary tables obtained by execution of table functions. Keyed by AST tree id. Context * query_context = nullptr; Context * session_context = nullptr; /// Session context or nullptr. Could be equal to this. - Context * global_context = nullptr; /// Global context or nullptr. Could be equal to this. + Context * global_context = nullptr; /// Global context. Could be equal to this. UInt64 session_close_cycle = 0; bool session_is_used = false; @@ -344,7 +344,9 @@ public: void setQueryContext(Context & context_) { query_context = &context_; } void setSessionContext(Context & context_) { session_context = &context_; } - void setGlobalContext(Context & context_) { global_context = &context_; } + + void makeQueryContext() { query_context = this; } + void makeSessionContext() { session_context = this; } const Settings & getSettingsRef() const { return settings; } Settings & getSettingsRef() { return settings; } diff --git a/dbms/src/Interpreters/InterpreterCreateQuery.cpp b/dbms/src/Interpreters/InterpreterCreateQuery.cpp index 7853e0c0841..7e1f46e674e 100644 --- a/dbms/src/Interpreters/InterpreterCreateQuery.cpp +++ b/dbms/src/Interpreters/InterpreterCreateQuery.cpp @@ -625,7 +625,7 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create) insert->select = create.select->clone(); if (create.temporary && !context.getSessionContext().hasQueryContext()) - context.getSessionContext().setQueryContext(context.getSessionContext()); + context.getSessionContext().makeQueryContext(); return InterpreterInsertQuery(insert, create.temporary ? context.getSessionContext() : context, diff --git a/dbms/src/Interpreters/executeQuery.cpp b/dbms/src/Interpreters/executeQuery.cpp index 16ef37efb6e..031749d5840 100644 --- a/dbms/src/Interpreters/executeQuery.cpp +++ b/dbms/src/Interpreters/executeQuery.cpp @@ -151,7 +151,7 @@ static std::tuple executeQueryImpl( { time_t current_time = time(nullptr); - context.setQueryContext(context); + context.makeQueryContext(); CurrentThread::attachQueryContext(context); const Settings & settings = context.getSettingsRef(); From c35a832095f96223fb5c4f469f2c3c35bbf4158a Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Mon, 8 Jul 2019 03:56:52 +0300 Subject: [PATCH 19/62] Two more fixes for PVS-Studio --- dbms/programs/server/HTTPHandler.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/dbms/programs/server/HTTPHandler.cpp b/dbms/programs/server/HTTPHandler.cpp index 18ca9b84518..a7a195950cc 100644 --- a/dbms/programs/server/HTTPHandler.cpp +++ b/dbms/programs/server/HTTPHandler.cpp @@ -211,7 +211,6 @@ void HTTPHandler::processQuery( Output & used_output) { Context context = server.context(); - context.makeGlobalContext(); CurrentThread::QueryScope query_scope(context); From 4752dae9bbf51679176762dfc02b6844d0e27242 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Mon, 8 Jul 2019 04:43:41 +0300 Subject: [PATCH 20/62] Allowed unresolvable addresses in cluster configuration #5714 --- dbms/src/Client/Connection.cpp | 20 +++++------ dbms/src/Client/Connection.h | 10 +++--- dbms/src/Common/isLocalAddress.h | 2 -- dbms/src/Interpreters/Cluster.cpp | 36 ++++++++++++++----- dbms/src/Interpreters/Cluster.h | 11 +++--- dbms/src/Interpreters/DDLWorker.cpp | 5 +-- .../Storages/System/StorageSystemClusters.cpp | 6 ++-- 7 files changed, 53 insertions(+), 37 deletions(-) diff --git a/dbms/src/Client/Connection.cpp b/dbms/src/Client/Connection.cpp index 9651ef54e1b..0bc7b2fb628 100644 --- a/dbms/src/Client/Connection.cpp +++ b/dbms/src/Client/Connection.cpp @@ -73,7 +73,7 @@ void Connection::connect(const ConnectionTimeouts & timeouts) current_resolved_address = DNSResolver::instance().resolveAddress(host, port); - socket->connect(current_resolved_address, timeouts.connection_timeout); + socket->connect(*current_resolved_address, timeouts.connection_timeout); socket->setReceiveTimeout(timeouts.receive_timeout); socket->setSendTimeout(timeouts.send_timeout); socket->setNoDelay(true); @@ -533,12 +533,9 @@ void Connection::sendExternalTablesData(ExternalTablesData & data) LOG_DEBUG(log_wrapper.get(), msg.rdbuf()); } -Poco::Net::SocketAddress Connection::getResolvedAddress() const +std::optional Connection::getResolvedAddress() const { - if (connected) - return current_resolved_address; - - return DNSResolver::instance().resolveAddress(host, port); + return current_resolved_address; } @@ -720,11 +717,14 @@ void Connection::initBlockLogsInput() void Connection::setDescription() { auto resolved_address = getResolvedAddress(); - description = host + ":" + toString(resolved_address.port()); - auto ip_address = resolved_address.host().toString(); + description = host + ":" + toString(port); - if (host != ip_address) - description += ", " + ip_address; + if (resolved_address) + { + auto ip_address = resolved_address->host().toString(); + if (host != ip_address) + description += ", " + ip_address; + } } diff --git a/dbms/src/Client/Connection.h b/dbms/src/Client/Connection.h index 2338e4c8965..03a771c257f 100644 --- a/dbms/src/Client/Connection.h +++ b/dbms/src/Client/Connection.h @@ -63,7 +63,7 @@ public: Poco::Timespan sync_request_timeout_ = Poco::Timespan(DBMS_DEFAULT_SYNC_REQUEST_TIMEOUT_SEC, 0)) : host(host_), port(port_), default_database(default_database_), - user(user_), password(password_), current_resolved_address(host, port), + user(user_), password(password_), client_name(client_name_), compression(compression_), secure(secure_), @@ -168,9 +168,6 @@ public: size_t outBytesCount() const { return out ? out->count() : 0; } size_t inBytesCount() const { return in ? in->count() : 0; } - /// Returns initially resolved address - Poco::Net::SocketAddress getResolvedAddress() const; - private: String host; UInt16 port; @@ -180,12 +177,15 @@ private: /// Address is resolved during the first connection (or the following reconnects) /// Use it only for logging purposes - Poco::Net::SocketAddress current_resolved_address; + std::optional current_resolved_address; /// For messages in log and in exceptions. String description; void setDescription(); + /// Returns resolved address if it was resolved. + std::optional getResolvedAddress() const; + String client_name; bool connected = false; diff --git a/dbms/src/Common/isLocalAddress.h b/dbms/src/Common/isLocalAddress.h index 81039dff68e..63de5e000a9 100644 --- a/dbms/src/Common/isLocalAddress.h +++ b/dbms/src/Common/isLocalAddress.h @@ -23,9 +23,7 @@ namespace DB * - the routing rules that affect which network interface we go to the specified address are not checked. */ bool isLocalAddress(const Poco::Net::SocketAddress & address, UInt16 clickhouse_port); - bool isLocalAddress(const Poco::Net::SocketAddress & address); - bool isLocalAddress(const Poco::Net::IPAddress & address); /// Returns number of different bytes in hostnames, used for load balancing diff --git a/dbms/src/Interpreters/Cluster.cpp b/dbms/src/Interpreters/Cluster.cpp index 986f99e0aaf..3c7c9bbe9da 100644 --- a/dbms/src/Interpreters/Cluster.cpp +++ b/dbms/src/Interpreters/Cluster.cpp @@ -29,9 +29,9 @@ namespace /// Default shard weight. static constexpr UInt32 default_weight = 1; -inline bool isLocal(const Cluster::Address & address, const Poco::Net::SocketAddress & resolved_address, UInt16 clickhouse_port) +inline bool isLocalImpl(const Cluster::Address & address, const Poco::Net::SocketAddress & resolved_address, UInt16 clickhouse_port) { - /// If there is replica, for which: + /// If there is replica, for which: /// - its port is the same that the server is listening; /// - its host is resolved to set of addresses, one of which is the same as one of addresses of network interfaces of the server machine*; /// then we must go to this shard without any inter-process communication. @@ -48,10 +48,31 @@ inline bool isLocal(const Cluster::Address & address, const Poco::Net::SocketAdd /// Implementation of Cluster::Address class +std::optional Cluster::Address::getResolvedAddress() const +{ + try + { + return DNSResolver::instance().resolveAddress(host_name, port); + } + catch (...) + { + /// Failure in DNS resolution in cluster initialization is Ok. + tryLogCurrentException("Cluster"); + return {}; + } +} + + +bool Cluster::Address::isLocal(UInt16 clickhouse_port) const +{ + if (auto resolved = getResolvedAddress()) + return isLocalImpl(*this, *resolved, clickhouse_port); + return false; +} + + Cluster::Address::Address(const Poco::Util::AbstractConfiguration & config, const String & config_prefix) { - UInt16 clickhouse_port = static_cast(config.getInt("tcp_port", 0)); - host_name = config.getString(config_prefix + ".host"); port = static_cast(config.getInt(config_prefix + ".port")); if (config.has(config_prefix + ".user")) @@ -60,10 +81,9 @@ Cluster::Address::Address(const Poco::Util::AbstractConfiguration & config, cons user = config.getString(config_prefix + ".user", "default"); password = config.getString(config_prefix + ".password", ""); default_database = config.getString(config_prefix + ".default_database", ""); - initially_resolved_address = DNSResolver::instance().resolveAddress(host_name, port); - is_local = isLocal(*this, initially_resolved_address, clickhouse_port); secure = config.getBool(config_prefix + ".secure", false) ? Protocol::Secure::Enable : Protocol::Secure::Disable; compression = config.getBool(config_prefix + ".compression", true) ? Protocol::Compression::Enable : Protocol::Compression::Disable; + is_local = isLocal(config.getInt("tcp_port", 0)); } @@ -74,9 +94,7 @@ Cluster::Address::Address(const String & host_port_, const String & user_, const host_name = parsed_host_port.first; port = parsed_host_port.second; secure = secure_ ? Protocol::Secure::Enable : Protocol::Secure::Disable; - - initially_resolved_address = DNSResolver::instance().resolveAddress(parsed_host_port.first, parsed_host_port.second); - is_local = isLocal(*this, initially_resolved_address, clickhouse_port); + is_local = isLocal(clickhouse_port); } diff --git a/dbms/src/Interpreters/Cluster.h b/dbms/src/Interpreters/Cluster.h index 06714da5cef..e778c9bcf6f 100644 --- a/dbms/src/Interpreters/Cluster.h +++ b/dbms/src/Interpreters/Cluster.h @@ -60,7 +60,7 @@ public: /// This database is selected when no database is specified for Distributed table String default_database; /// The locality is determined at the initialization, and is not changed even if DNS is changed - bool is_local; + bool is_local = false; bool user_specified = false; Protocol::Compression compression = Protocol::Compression::Enable; @@ -84,17 +84,14 @@ public: String toFullString() const; static Address fromFullString(const String & address_full_string); - /// Returns initially resolved address - Poco::Net::SocketAddress getResolvedAddress() const - { - return initially_resolved_address; - } + /// Returns resolved address if it does resolve. + std::optional getResolvedAddress() const; auto tuple() const { return std::tie(host_name, port, secure, user, password, default_database); } bool operator==(const Address & other) const { return tuple() == other.tuple(); } private: - Poco::Net::SocketAddress initially_resolved_address; + bool isLocal(UInt16 clickhouse_port) const; }; using Addresses = std::vector
; diff --git a/dbms/src/Interpreters/DDLWorker.cpp b/dbms/src/Interpreters/DDLWorker.cpp index e1cea632a37..1a4113a0d9c 100644 --- a/dbms/src/Interpreters/DDLWorker.cpp +++ b/dbms/src/Interpreters/DDLWorker.cpp @@ -506,8 +506,9 @@ void DDLWorker::parseQueryAndResolveHost(DDLTask & task) { const Cluster::Address & address = shards[shard_num][replica_num]; - if (isLocalAddress(address.getResolvedAddress(), context.getTCPPort()) - || (context.getTCPPortSecure() && isLocalAddress(address.getResolvedAddress(), *context.getTCPPortSecure()))) + if (auto resolved = address.getResolvedAddress(); + resolved && (isLocalAddress(*resolved, context.getTCPPort()) + || (context.getTCPPortSecure() && isLocalAddress(*resolved, *context.getTCPPortSecure())))) { if (found_via_resolving) { diff --git a/dbms/src/Storages/System/StorageSystemClusters.cpp b/dbms/src/Storages/System/StorageSystemClusters.cpp index b33b2d86d0e..d9403aba688 100644 --- a/dbms/src/Storages/System/StorageSystemClusters.cpp +++ b/dbms/src/Storages/System/StorageSystemClusters.cpp @@ -10,7 +10,8 @@ namespace DB NamesAndTypesList StorageSystemClusters::getNamesAndTypes() { - return { + return + { {"cluster", std::make_shared()}, {"shard_num", std::make_shared()}, {"shard_weight", std::make_shared()}, @@ -48,7 +49,8 @@ void StorageSystemClusters::fillData(MutableColumns & res_columns, const Context res_columns[i++]->insert(shard_info.weight); res_columns[i++]->insert(replica_index + 1); res_columns[i++]->insert(address.host_name); - res_columns[i++]->insert(DNSResolver::instance().resolveHost(address.host_name).toString()); + auto resolved = address.getResolvedAddress(); + res_columns[i++]->insert(resolved ? resolved->host().toString() : String()); res_columns[i++]->insert(address.port); res_columns[i++]->insert(shard_info.isLocal()); res_columns[i++]->insert(address.user); From ac20c515ab6835f03ec08fe1fac65c30d740118d Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Mon, 8 Jul 2019 04:58:15 +0300 Subject: [PATCH 21/62] Better code in unit tests --- dbms/CMakeLists.txt | 4 ++++ dbms/src/Columns/tests/column_unique.cpp | 6 ------ .../ZooKeeper/tests/gtest_zkutil_test_multi_exception.cpp | 5 ----- dbms/src/Common/tests/gtest_rw_lock.cpp | 5 ----- dbms/src/Common/tests/gtest_shell_command.cpp | 5 ----- dbms/src/Common/tests/gtest_thread_pool_concurrent_wait.cpp | 5 ----- dbms/src/Common/tests/gtest_thread_pool_limit.cpp | 5 ----- dbms/src/Common/tests/gtest_thread_pool_loop.cpp | 5 ----- .../Common/tests/gtest_thread_pool_schedule_exception.cpp | 5 ----- dbms/src/Common/tests/gtest_unescapeForFileName.cpp | 5 ----- dbms/src/Compression/tests/gtest_compressionCodec.cpp | 5 ----- .../DataStreams/tests/gtest_blocks_size_merging_streams.cpp | 6 ------ .../src/DataTypes/tests/gtest_data_type_get_common_type.cpp | 5 ----- dbms/src/IO/tests/gtest_aio_seek_back_after_eof.cpp | 5 ----- dbms/src/IO/tests/gtest_bit_io.cpp | 6 ------ dbms/src/IO/tests/gtest_cascade_and_memory_write_buffer.cpp | 5 ----- .../tests/gtest_aux_funcs_for_adaptive_granularity.cpp | 5 ----- dbms/src/Storages/tests/gtest_row_source_bits_test.cpp | 5 ----- .../tests/gtest_transform_query_for_external_database.cpp | 5 ----- 19 files changed, 4 insertions(+), 93 deletions(-) diff --git a/dbms/CMakeLists.txt b/dbms/CMakeLists.txt index c3b4106ade8..60a3cda0556 100644 --- a/dbms/CMakeLists.txt +++ b/dbms/CMakeLists.txt @@ -387,6 +387,10 @@ if (ENABLE_TESTS AND USE_GTEST) # attach all dbms gtest sources grep_gtest_sources(${ClickHouse_SOURCE_DIR}/dbms dbms_gtest_sources) add_executable(unit_tests_dbms ${dbms_gtest_sources}) + + # gtest framework has substandard code + target_compile_options(unit_tests_dbms PRIVATE -Wno-zero-as-null-pointer-constant -Wno-undef -Wno-sign-compare -Wno-used-but-marked-unused) + target_link_libraries(unit_tests_dbms PRIVATE ${GTEST_BOTH_LIBRARIES} clickhouse_functions clickhouse_parsers dbms clickhouse_common_zookeeper) add_check(unit_tests_dbms) endif () diff --git a/dbms/src/Columns/tests/column_unique.cpp b/dbms/src/Columns/tests/column_unique.cpp index 68b9367ee86..1d5ea5b4080 100644 --- a/dbms/src/Columns/tests/column_unique.cpp +++ b/dbms/src/Columns/tests/column_unique.cpp @@ -7,12 +7,6 @@ #include #include -#pragma GCC diagnostic ignored "-Wsign-compare" -#ifdef __clang__ -#pragma clang diagnostic ignored "-Wzero-as-null-pointer-constant" -#pragma clang diagnostic ignored "-Wundef" -#endif - #include #include diff --git a/dbms/src/Common/ZooKeeper/tests/gtest_zkutil_test_multi_exception.cpp b/dbms/src/Common/ZooKeeper/tests/gtest_zkutil_test_multi_exception.cpp index 489ac04c5e4..2f332c7039d 100644 --- a/dbms/src/Common/ZooKeeper/tests/gtest_zkutil_test_multi_exception.cpp +++ b/dbms/src/Common/ZooKeeper/tests/gtest_zkutil_test_multi_exception.cpp @@ -5,11 +5,6 @@ #include #include -#pragma GCC diagnostic ignored "-Wsign-compare" -#ifdef __clang__ - #pragma clang diagnostic ignored "-Wzero-as-null-pointer-constant" - #pragma clang diagnostic ignored "-Wundef" -#endif #include #include diff --git a/dbms/src/Common/tests/gtest_rw_lock.cpp b/dbms/src/Common/tests/gtest_rw_lock.cpp index 6e9b91dc048..68927c8bc4a 100644 --- a/dbms/src/Common/tests/gtest_rw_lock.cpp +++ b/dbms/src/Common/tests/gtest_rw_lock.cpp @@ -1,8 +1,3 @@ -#pragma GCC diagnostic ignored "-Wsign-compare" -#ifdef __clang__ - #pragma clang diagnostic ignored "-Wzero-as-null-pointer-constant" - #pragma clang diagnostic ignored "-Wundef" -#endif #include #include diff --git a/dbms/src/Common/tests/gtest_shell_command.cpp b/dbms/src/Common/tests/gtest_shell_command.cpp index 2378cda2ee7..79b8309d0fa 100644 --- a/dbms/src/Common/tests/gtest_shell_command.cpp +++ b/dbms/src/Common/tests/gtest_shell_command.cpp @@ -9,11 +9,6 @@ #include #include -#pragma GCC diagnostic ignored "-Wsign-compare" -#ifdef __clang__ - #pragma clang diagnostic ignored "-Wzero-as-null-pointer-constant" - #pragma clang diagnostic ignored "-Wundef" -#endif #include diff --git a/dbms/src/Common/tests/gtest_thread_pool_concurrent_wait.cpp b/dbms/src/Common/tests/gtest_thread_pool_concurrent_wait.cpp index 1e38e418a22..213e70ce3dd 100644 --- a/dbms/src/Common/tests/gtest_thread_pool_concurrent_wait.cpp +++ b/dbms/src/Common/tests/gtest_thread_pool_concurrent_wait.cpp @@ -1,10 +1,5 @@ #include -#pragma GCC diagnostic ignored "-Wsign-compare" -#ifdef __clang__ - #pragma clang diagnostic ignored "-Wzero-as-null-pointer-constant" - #pragma clang diagnostic ignored "-Wundef" -#endif #include /** Reproduces bug in ThreadPool. diff --git a/dbms/src/Common/tests/gtest_thread_pool_limit.cpp b/dbms/src/Common/tests/gtest_thread_pool_limit.cpp index 2bd38f34d10..c18ff2e38ee 100644 --- a/dbms/src/Common/tests/gtest_thread_pool_limit.cpp +++ b/dbms/src/Common/tests/gtest_thread_pool_limit.cpp @@ -2,11 +2,6 @@ #include #include -#pragma GCC diagnostic ignored "-Wsign-compare" -#ifdef __clang__ - #pragma clang diagnostic ignored "-Wzero-as-null-pointer-constant" - #pragma clang diagnostic ignored "-Wundef" -#endif #include /// Test for thread self-removal when number of free threads in pool is too large. diff --git a/dbms/src/Common/tests/gtest_thread_pool_loop.cpp b/dbms/src/Common/tests/gtest_thread_pool_loop.cpp index 80b7b94d988..63d4114b867 100644 --- a/dbms/src/Common/tests/gtest_thread_pool_loop.cpp +++ b/dbms/src/Common/tests/gtest_thread_pool_loop.cpp @@ -2,11 +2,6 @@ #include #include -#pragma GCC diagnostic ignored "-Wsign-compare" -#ifdef __clang__ - #pragma clang diagnostic ignored "-Wzero-as-null-pointer-constant" - #pragma clang diagnostic ignored "-Wundef" -#endif #include diff --git a/dbms/src/Common/tests/gtest_thread_pool_schedule_exception.cpp b/dbms/src/Common/tests/gtest_thread_pool_schedule_exception.cpp index 001d9c30b27..52091a1ea7f 100644 --- a/dbms/src/Common/tests/gtest_thread_pool_schedule_exception.cpp +++ b/dbms/src/Common/tests/gtest_thread_pool_schedule_exception.cpp @@ -2,11 +2,6 @@ #include #include -#pragma GCC diagnostic ignored "-Wsign-compare" -#ifdef __clang__ - #pragma clang diagnostic ignored "-Wzero-as-null-pointer-constant" - #pragma clang diagnostic ignored "-Wundef" -#endif #include diff --git a/dbms/src/Common/tests/gtest_unescapeForFileName.cpp b/dbms/src/Common/tests/gtest_unescapeForFileName.cpp index b7e045b4318..33194a6dade 100644 --- a/dbms/src/Common/tests/gtest_unescapeForFileName.cpp +++ b/dbms/src/Common/tests/gtest_unescapeForFileName.cpp @@ -1,10 +1,5 @@ #include -#pragma GCC diagnostic ignored "-Wsign-compare" -#ifdef __clang__ - #pragma clang diagnostic ignored "-Wzero-as-null-pointer-constant" - #pragma clang diagnostic ignored "-Wundef" -#endif #include diff --git a/dbms/src/Compression/tests/gtest_compressionCodec.cpp b/dbms/src/Compression/tests/gtest_compressionCodec.cpp index e1413ccd7bd..96a70d46433 100644 --- a/dbms/src/Compression/tests/gtest_compressionCodec.cpp +++ b/dbms/src/Compression/tests/gtest_compressionCodec.cpp @@ -20,11 +20,6 @@ #include #include -#pragma GCC diagnostic ignored "-Wsign-compare" -#ifdef __clang__ -#pragma clang diagnostic ignored "-Wzero-as-null-pointer-constant" -#pragma clang diagnostic ignored "-Wundef" -#endif #include using namespace DB; diff --git a/dbms/src/DataStreams/tests/gtest_blocks_size_merging_streams.cpp b/dbms/src/DataStreams/tests/gtest_blocks_size_merging_streams.cpp index 9cbe82111a2..57cd68d16d8 100644 --- a/dbms/src/DataStreams/tests/gtest_blocks_size_merging_streams.cpp +++ b/dbms/src/DataStreams/tests/gtest_blocks_size_merging_streams.cpp @@ -1,9 +1,3 @@ -#pragma GCC diagnostic ignored "-Wsign-compare" -#ifdef __clang__ -#pragma clang diagnostic ignored "-Wzero-as-null-pointer-constant" -#pragma clang diagnostic ignored "-Wundef" -#endif - #include #include #include diff --git a/dbms/src/DataTypes/tests/gtest_data_type_get_common_type.cpp b/dbms/src/DataTypes/tests/gtest_data_type_get_common_type.cpp index d2782ae9179..8ad8e955e75 100644 --- a/dbms/src/DataTypes/tests/gtest_data_type_get_common_type.cpp +++ b/dbms/src/DataTypes/tests/gtest_data_type_get_common_type.cpp @@ -4,11 +4,6 @@ #include -#pragma GCC diagnostic ignored "-Wsign-compare" -#ifdef __clang__ - #pragma clang diagnostic ignored "-Wzero-as-null-pointer-constant" - #pragma clang diagnostic ignored "-Wundef" -#endif #include diff --git a/dbms/src/IO/tests/gtest_aio_seek_back_after_eof.cpp b/dbms/src/IO/tests/gtest_aio_seek_back_after_eof.cpp index 382182e8cbf..85643d2a78b 100644 --- a/dbms/src/IO/tests/gtest_aio_seek_back_after_eof.cpp +++ b/dbms/src/IO/tests/gtest_aio_seek_back_after_eof.cpp @@ -1,10 +1,5 @@ #if defined(__linux__) || defined(__FreeBSD__) -#pragma GCC diagnostic ignored "-Wsign-compare" -#ifdef __clang__ -#pragma clang diagnostic ignored "-Wzero-as-null-pointer-constant" -#pragma clang diagnostic ignored "-Wundef" -#endif #include #include diff --git a/dbms/src/IO/tests/gtest_bit_io.cpp b/dbms/src/IO/tests/gtest_bit_io.cpp index abb5a53e346..85df2580783 100644 --- a/dbms/src/IO/tests/gtest_bit_io.cpp +++ b/dbms/src/IO/tests/gtest_bit_io.cpp @@ -16,12 +16,6 @@ #include #include -#pragma GCC diagnostic ignored "-Wsign-compare" -#ifdef __clang__ -#pragma clang diagnostic ignored "-Wzero-as-null-pointer-constant" -#pragma clang diagnostic ignored "-Wundef" -#endif - #include using namespace DB; diff --git a/dbms/src/IO/tests/gtest_cascade_and_memory_write_buffer.cpp b/dbms/src/IO/tests/gtest_cascade_and_memory_write_buffer.cpp index ecf4054e122..aa95ab7cb58 100644 --- a/dbms/src/IO/tests/gtest_cascade_and_memory_write_buffer.cpp +++ b/dbms/src/IO/tests/gtest_cascade_and_memory_write_buffer.cpp @@ -1,8 +1,3 @@ -#pragma GCC diagnostic ignored "-Wsign-compare" -#ifdef __clang__ - #pragma clang diagnostic ignored "-Wzero-as-null-pointer-constant" - #pragma clang diagnostic ignored "-Wundef" -#endif #include #include diff --git a/dbms/src/Storages/tests/gtest_aux_funcs_for_adaptive_granularity.cpp b/dbms/src/Storages/tests/gtest_aux_funcs_for_adaptive_granularity.cpp index 85df83be2de..95c56c74132 100644 --- a/dbms/src/Storages/tests/gtest_aux_funcs_for_adaptive_granularity.cpp +++ b/dbms/src/Storages/tests/gtest_aux_funcs_for_adaptive_granularity.cpp @@ -1,8 +1,3 @@ -#pragma GCC diagnostic ignored "-Wsign-compare" -#ifdef __clang__ -#pragma clang diagnostic ignored "-Wzero-as-null-pointer-constant" -#pragma clang diagnostic ignored "-Wundef" -#endif #include #include #include diff --git a/dbms/src/Storages/tests/gtest_row_source_bits_test.cpp b/dbms/src/Storages/tests/gtest_row_source_bits_test.cpp index 7b2f25061b6..a6d9179c106 100644 --- a/dbms/src/Storages/tests/gtest_row_source_bits_test.cpp +++ b/dbms/src/Storages/tests/gtest_row_source_bits_test.cpp @@ -1,8 +1,3 @@ -#pragma GCC diagnostic ignored "-Wsign-compare" -#ifdef __clang__ - #pragma clang diagnostic ignored "-Wzero-as-null-pointer-constant" - #pragma clang diagnostic ignored "-Wundef" -#endif #include #include diff --git a/dbms/src/Storages/tests/gtest_transform_query_for_external_database.cpp b/dbms/src/Storages/tests/gtest_transform_query_for_external_database.cpp index bcee0b8d8e1..bc849e7f8ce 100644 --- a/dbms/src/Storages/tests/gtest_transform_query_for_external_database.cpp +++ b/dbms/src/Storages/tests/gtest_transform_query_for_external_database.cpp @@ -1,8 +1,3 @@ -#pragma GCC diagnostic ignored "-Wsign-compare" -#ifdef __clang__ - #pragma clang diagnostic ignored "-Wzero-as-null-pointer-constant" - #pragma clang diagnostic ignored "-Wundef" -#endif #include #include From f9f85b4c65b01e1ac53f51f68642158660dfa10c Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Mon, 8 Jul 2019 05:03:01 +0300 Subject: [PATCH 22/62] Added test --- .../0_stateless/00965_shard_unresolvable_addresses.reference | 1 + .../queries/0_stateless/00965_shard_unresolvable_addresses.sql | 2 ++ 2 files changed, 3 insertions(+) create mode 100644 dbms/tests/queries/0_stateless/00965_shard_unresolvable_addresses.reference create mode 100644 dbms/tests/queries/0_stateless/00965_shard_unresolvable_addresses.sql diff --git a/dbms/tests/queries/0_stateless/00965_shard_unresolvable_addresses.reference b/dbms/tests/queries/0_stateless/00965_shard_unresolvable_addresses.reference new file mode 100644 index 00000000000..d00491fd7e5 --- /dev/null +++ b/dbms/tests/queries/0_stateless/00965_shard_unresolvable_addresses.reference @@ -0,0 +1 @@ +1 diff --git a/dbms/tests/queries/0_stateless/00965_shard_unresolvable_addresses.sql b/dbms/tests/queries/0_stateless/00965_shard_unresolvable_addresses.sql new file mode 100644 index 00000000000..b6b981c7d00 --- /dev/null +++ b/dbms/tests/queries/0_stateless/00965_shard_unresolvable_addresses.sql @@ -0,0 +1,2 @@ +SELECT count() FROM remote('127.0.0.1,localhos', system.one); -- { serverError 279 } +SELECT count() FROM remote('127.0.0.1|localhos', system.one); From 57d8dac95d40f723f134d867be3183a820b212d6 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Mon, 8 Jul 2019 05:06:15 +0300 Subject: [PATCH 23/62] Two more fixes for PVS-Studio --- dbms/programs/local/LocalServer.cpp | 4 ++-- libs/libcommon/include/common/JSON.h | 12 +++++++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/dbms/programs/local/LocalServer.cpp b/dbms/programs/local/LocalServer.cpp index 8c87a5d0fa4..c379786cab2 100644 --- a/dbms/programs/local/LocalServer.cpp +++ b/dbms/programs/local/LocalServer.cpp @@ -274,8 +274,8 @@ void LocalServer::processQueries() if (!parse_res.second) throw Exception("Cannot parse and execute the following part of query: " + String(parse_res.first), ErrorCodes::SYNTAX_ERROR); - context->setSessionContext(*context); - context->setQueryContext(*context); + context->makeSessionContext(); + context->makeQueryContext(); context->setUser("default", "", Poco::Net::SocketAddress{}, ""); context->setCurrentQueryId(""); diff --git a/libs/libcommon/include/common/JSON.h b/libs/libcommon/include/common/JSON.h index 9bb109a840f..04c98851742 100644 --- a/libs/libcommon/include/common/JSON.h +++ b/libs/libcommon/include/common/JSON.h @@ -60,7 +60,17 @@ public: checkInit(); } - JSON(const JSON & rhs) : ptr_begin(rhs.ptr_begin), ptr_end(rhs.ptr_end), level(rhs.level) {} + JSON(const JSON & rhs) + { + *this = rhs; + } + + JSON & operator=(const JSON & rhs) + { + ptr_begin = rhs.ptr_begin; + ptr_end = rhs.ptr_end; + level = rhs.level; + } const char * data() const { return ptr_begin; } const char * dataEnd() const { return ptr_end; } From 6577ecec9040f298dd393fbd0df3d4553c5e90d7 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Mon, 8 Jul 2019 05:06:37 +0300 Subject: [PATCH 24/62] Two more fixes for PVS-Studio --- libs/libcommon/include/common/JSON.h | 1 + 1 file changed, 1 insertion(+) diff --git a/libs/libcommon/include/common/JSON.h b/libs/libcommon/include/common/JSON.h index 04c98851742..5f3d9325626 100644 --- a/libs/libcommon/include/common/JSON.h +++ b/libs/libcommon/include/common/JSON.h @@ -70,6 +70,7 @@ public: ptr_begin = rhs.ptr_begin; ptr_end = rhs.ptr_end; level = rhs.level; + return *this; } const char * data() const { return ptr_begin; } From f40b70884ffbd86f3020b651122ccc5ab41253ab Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Mon, 8 Jul 2019 05:14:32 +0300 Subject: [PATCH 25/62] Two more fixes for PVS-Studio --- dbms/programs/benchmark/Benchmark.cpp | 2 ++ dbms/programs/client/Client.cpp | 1 + dbms/programs/copier/ClusterCopier.cpp | 1 + dbms/programs/local/LocalServer.cpp | 1 + dbms/programs/obfuscator/Obfuscator.cpp | 1 + dbms/programs/odbc-bridge/ODBCBridge.cpp | 1 + dbms/programs/performance-test/PerformanceTestSuite.cpp | 1 + dbms/programs/server/Server.cpp | 1 + dbms/src/DataStreams/tests/collapsing_sorted_stream.cpp | 1 + dbms/src/DataStreams/tests/expression_stream.cpp | 1 + dbms/src/DataStreams/tests/filter_stream.cpp | 1 + dbms/src/DataStreams/tests/union_stream2.cpp | 1 + dbms/src/Interpreters/Context.cpp | 1 - dbms/src/Interpreters/Context.h | 1 + dbms/src/Interpreters/tests/create_query.cpp | 1 + dbms/src/Interpreters/tests/expression.cpp | 1 + dbms/src/Interpreters/tests/expression_analyzer.cpp | 1 + .../Interpreters/tests/in_join_subqueries_preprocessor.cpp | 1 + dbms/src/Interpreters/tests/select_query.cpp | 1 + .../tests/gtest_transform_query_for_external_database.cpp | 1 + dbms/src/Storages/tests/storage_log.cpp | 1 + dbms/src/Storages/tests/system_numbers.cpp | 5 +++-- 22 files changed, 24 insertions(+), 3 deletions(-) diff --git a/dbms/programs/benchmark/Benchmark.cpp b/dbms/programs/benchmark/Benchmark.cpp index 019080e2391..c69e9a54feb 100644 --- a/dbms/programs/benchmark/Benchmark.cpp +++ b/dbms/programs/benchmark/Benchmark.cpp @@ -61,6 +61,8 @@ public: randomize(randomize_), max_iterations(max_iterations_), max_time(max_time_), json_path(json_path_), settings(settings_), global_context(Context::createGlobal()), pool(concurrency) { + global_context.makeGlobalContext(); + std::cerr << std::fixed << std::setprecision(3); /// This is needed to receive blocks with columns of AggregateFunction data type diff --git a/dbms/programs/client/Client.cpp b/dbms/programs/client/Client.cpp index 2da1c4a987d..e7eb7531e99 100644 --- a/dbms/programs/client/Client.cpp +++ b/dbms/programs/client/Client.cpp @@ -218,6 +218,7 @@ private: configReadClient(config(), home_path); + global_context.makeGlobalContext(); context.setApplicationType(Context::ApplicationType::CLIENT); /// settings and limits could be specified in config file, but passed settings has higher priority diff --git a/dbms/programs/copier/ClusterCopier.cpp b/dbms/programs/copier/ClusterCopier.cpp index 6bba1ceb5f3..43158dedd71 100644 --- a/dbms/programs/copier/ClusterCopier.cpp +++ b/dbms/programs/copier/ClusterCopier.cpp @@ -2171,6 +2171,7 @@ void ClusterCopierApp::mainImpl() << "revision " << ClickHouseRevision::get() << ")"); auto context = std::make_unique(Context::createGlobal()); + context->makeGlobalContext(); SCOPE_EXIT(context->shutdown()); context->setConfig(loaded_config.configuration); diff --git a/dbms/programs/local/LocalServer.cpp b/dbms/programs/local/LocalServer.cpp index c379786cab2..bed55a0fc5f 100644 --- a/dbms/programs/local/LocalServer.cpp +++ b/dbms/programs/local/LocalServer.cpp @@ -131,6 +131,7 @@ try context = std::make_unique(Context::createGlobal()); + context->makeGlobalContext(); context->setApplicationType(Context::ApplicationType::LOCAL); tryInitPath(); diff --git a/dbms/programs/obfuscator/Obfuscator.cpp b/dbms/programs/obfuscator/Obfuscator.cpp index 7488433d569..ac6828c584b 100644 --- a/dbms/programs/obfuscator/Obfuscator.cpp +++ b/dbms/programs/obfuscator/Obfuscator.cpp @@ -1024,6 +1024,7 @@ try } Context context = Context::createGlobal(); + context->makeGlobalContext(); ReadBufferFromFileDescriptor file_in(STDIN_FILENO); WriteBufferFromFileDescriptor file_out(STDOUT_FILENO); diff --git a/dbms/programs/odbc-bridge/ODBCBridge.cpp b/dbms/programs/odbc-bridge/ODBCBridge.cpp index 6327cee3e27..cf265eb6abb 100644 --- a/dbms/programs/odbc-bridge/ODBCBridge.cpp +++ b/dbms/programs/odbc-bridge/ODBCBridge.cpp @@ -160,6 +160,7 @@ int ODBCBridge::main(const std::vector & /*args*/) http_params->setKeepAliveTimeout(keep_alive_timeout); context = std::make_shared(Context::createGlobal()); + context->makeGlobalContext(); auto server = Poco::Net::HTTPServer( new HandlerFactory("ODBCRequestHandlerFactory-factory", keep_alive_timeout, context), server_pool, socket, http_params); diff --git a/dbms/programs/performance-test/PerformanceTestSuite.cpp b/dbms/programs/performance-test/PerformanceTestSuite.cpp index cfa7d202d1d..eb472985edd 100644 --- a/dbms/programs/performance-test/PerformanceTestSuite.cpp +++ b/dbms/programs/performance-test/PerformanceTestSuite.cpp @@ -127,6 +127,7 @@ private: std::unordered_map> query_indexes; Context global_context = Context::createGlobal(); + global_context.makeGlobalContext(); std::shared_ptr report_builder; std::string server_version; diff --git a/dbms/programs/server/Server.cpp b/dbms/programs/server/Server.cpp index 2ccdc5289f2..c82d3e1e95b 100644 --- a/dbms/programs/server/Server.cpp +++ b/dbms/programs/server/Server.cpp @@ -187,6 +187,7 @@ int Server::main(const std::vector & /*args*/) * settings, available functions, data types, aggregate functions, databases... */ global_context = std::make_unique(Context::createGlobal()); + global_context->makeGlobalContext(); global_context->setApplicationType(Context::ApplicationType::SERVER); bool has_zookeeper = config().has("zookeeper"); diff --git a/dbms/src/DataStreams/tests/collapsing_sorted_stream.cpp b/dbms/src/DataStreams/tests/collapsing_sorted_stream.cpp index a43b7d347eb..43205a9e7b5 100644 --- a/dbms/src/DataStreams/tests/collapsing_sorted_stream.cpp +++ b/dbms/src/DataStreams/tests/collapsing_sorted_stream.cpp @@ -68,6 +68,7 @@ try CollapsingFinalBlockInputStream collapsed(inputs, descr, "Sign"); Context context = Context::createGlobal(); + context.makeGlobalContext(); WriteBufferFromFileDescriptor out_buf(STDERR_FILENO); BlockOutputStreamPtr output = context.getOutputFormat("TabSeparated", out_buf, block1); diff --git a/dbms/src/DataStreams/tests/expression_stream.cpp b/dbms/src/DataStreams/tests/expression_stream.cpp index 3cbce14649d..b63d9d1f3b5 100644 --- a/dbms/src/DataStreams/tests/expression_stream.cpp +++ b/dbms/src/DataStreams/tests/expression_stream.cpp @@ -35,6 +35,7 @@ try ASTPtr ast = parseQuery(parser, input.data(), input.data() + input.size(), "", 0); Context context = Context::createGlobal(); + context.makeGlobalContext(); NamesAndTypesList source_columns = {{"number", std::make_shared()}}; auto syntax_result = SyntaxAnalyzer(context, {}).analyze(ast, source_columns); diff --git a/dbms/src/DataStreams/tests/filter_stream.cpp b/dbms/src/DataStreams/tests/filter_stream.cpp index ed12c09dc99..4199ba40b53 100644 --- a/dbms/src/DataStreams/tests/filter_stream.cpp +++ b/dbms/src/DataStreams/tests/filter_stream.cpp @@ -40,6 +40,7 @@ try std::cerr << std::endl; Context context = Context::createGlobal(); + context.makeGlobalContext(); NamesAndTypesList source_columns = {{"number", std::make_shared()}}; auto syntax_result = SyntaxAnalyzer(context, {}).analyze(ast, source_columns); diff --git a/dbms/src/DataStreams/tests/union_stream2.cpp b/dbms/src/DataStreams/tests/union_stream2.cpp index 284e66b91a9..3eb1927f80a 100644 --- a/dbms/src/DataStreams/tests/union_stream2.cpp +++ b/dbms/src/DataStreams/tests/union_stream2.cpp @@ -23,6 +23,7 @@ int main(int, char **) try { Context context = Context::createGlobal(); + context.makeGlobalContext(); Settings settings = context.getSettings(); context.setPath("./"); diff --git a/dbms/src/Interpreters/Context.cpp b/dbms/src/Interpreters/Context.cpp index c89f4273b3e..2909de8a60b 100644 --- a/dbms/src/Interpreters/Context.cpp +++ b/dbms/src/Interpreters/Context.cpp @@ -305,7 +305,6 @@ Context Context::createGlobal(std::unique_ptr runtime Context res; res.shared = std::make_shared(std::move(runtime_components_factory)); res.quota = std::make_shared(); - res.global_context = &res; return res; } diff --git a/dbms/src/Interpreters/Context.h b/dbms/src/Interpreters/Context.h index c49f509d2a8..e414271756f 100644 --- a/dbms/src/Interpreters/Context.h +++ b/dbms/src/Interpreters/Context.h @@ -347,6 +347,7 @@ public: void makeQueryContext() { query_context = this; } void makeSessionContext() { session_context = this; } + void makeGlobalContext() { global_context = this; } const Settings & getSettingsRef() const { return settings; } Settings & getSettingsRef() { return settings; } diff --git a/dbms/src/Interpreters/tests/create_query.cpp b/dbms/src/Interpreters/tests/create_query.cpp index b49a87c6945..11f01ef3a6a 100644 --- a/dbms/src/Interpreters/tests/create_query.cpp +++ b/dbms/src/Interpreters/tests/create_query.cpp @@ -79,6 +79,7 @@ try ASTPtr ast = parseQuery(parser, input.data(), input.data() + input.size(), "", 0); Context context = Context::createGlobal(); + context.makeGlobalContext(); context.setPath("./"); auto database = std::make_shared("test", "./metadata/test/", context); diff --git a/dbms/src/Interpreters/tests/expression.cpp b/dbms/src/Interpreters/tests/expression.cpp index 73502d9067e..2dbf01d148b 100644 --- a/dbms/src/Interpreters/tests/expression.cpp +++ b/dbms/src/Interpreters/tests/expression.cpp @@ -47,6 +47,7 @@ int main(int argc, char ** argv) std::cerr << std::endl; Context context = Context::createGlobal(); + context.makeGlobalContext(); NamesAndTypesList columns { {"x", std::make_shared()}, diff --git a/dbms/src/Interpreters/tests/expression_analyzer.cpp b/dbms/src/Interpreters/tests/expression_analyzer.cpp index 3732e7dce72..079a22620bc 100644 --- a/dbms/src/Interpreters/tests/expression_analyzer.cpp +++ b/dbms/src/Interpreters/tests/expression_analyzer.cpp @@ -97,6 +97,7 @@ int main() }; Context context = Context::createGlobal(); + context.makeGlobalContext(); auto system_database = std::make_shared("system"); context.addDatabase("system", system_database); diff --git a/dbms/src/Interpreters/tests/in_join_subqueries_preprocessor.cpp b/dbms/src/Interpreters/tests/in_join_subqueries_preprocessor.cpp index ecb21c58831..e34e95a00ef 100644 --- a/dbms/src/Interpreters/tests/in_join_subqueries_preprocessor.cpp +++ b/dbms/src/Interpreters/tests/in_join_subqueries_preprocessor.cpp @@ -1158,6 +1158,7 @@ bool run() TestResult check(const TestEntry & entry) { static DB::Context context = DB::Context::createGlobal(); + context.makeGlobalContext(); try { diff --git a/dbms/src/Interpreters/tests/select_query.cpp b/dbms/src/Interpreters/tests/select_query.cpp index 951d8e0723a..7267c408999 100644 --- a/dbms/src/Interpreters/tests/select_query.cpp +++ b/dbms/src/Interpreters/tests/select_query.cpp @@ -31,6 +31,7 @@ try DateLUT::instance(); Context context = Context::createGlobal(); + context.makeGlobalContext(); context.setPath("./"); diff --git a/dbms/src/Storages/tests/gtest_transform_query_for_external_database.cpp b/dbms/src/Storages/tests/gtest_transform_query_for_external_database.cpp index bcee0b8d8e1..ee9e936351d 100644 --- a/dbms/src/Storages/tests/gtest_transform_query_for_external_database.cpp +++ b/dbms/src/Storages/tests/gtest_transform_query_for_external_database.cpp @@ -29,6 +29,7 @@ struct State registerFunctions(); DatabasePtr database = std::make_shared("test"); database->attachTable("table", StorageMemory::create("table", ColumnsDescription{columns})); + context.makeGlobalContext(); context.addDatabase("test", database); context.setCurrentDatabase("test"); } diff --git a/dbms/src/Storages/tests/storage_log.cpp b/dbms/src/Storages/tests/storage_log.cpp index db905731e6f..1ac1300a144 100644 --- a/dbms/src/Storages/tests/storage_log.cpp +++ b/dbms/src/Storages/tests/storage_log.cpp @@ -29,6 +29,7 @@ try table->startup(); auto context = Context::createGlobal(); + context.makeGlobalContext(); /// write into it { diff --git a/dbms/src/Storages/tests/system_numbers.cpp b/dbms/src/Storages/tests/system_numbers.cpp index 92d1f9d65e2..0ba94fb5d0a 100644 --- a/dbms/src/Storages/tests/system_numbers.cpp +++ b/dbms/src/Storages/tests/system_numbers.cpp @@ -26,9 +26,10 @@ try WriteBufferFromOStream out_buf(std::cout); - QueryProcessingStage::Enum stage = table->getQueryProcessingStage(Context::createGlobal()); - auto context = Context::createGlobal(); + context.makeGlobalContext(); + QueryProcessingStage::Enum stage = table->getQueryProcessingStage(context); + LimitBlockInputStream input(table->read(column_names, {}, context, stage, 10, 1)[0], 10, 96); BlockOutputStreamPtr out = FormatFactory::instance().getOutput("TabSeparated", out_buf, sample, context); From 36269c4e2ad5536bcf509dca96dea5f107fcad21 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Mon, 8 Jul 2019 06:41:37 +0300 Subject: [PATCH 26/62] Addition to prev. revision --- dbms/programs/client/Client.cpp | 2 +- dbms/programs/obfuscator/Obfuscator.cpp | 2 +- dbms/programs/performance-test/PerformanceTestSuite.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dbms/programs/client/Client.cpp b/dbms/programs/client/Client.cpp index e7eb7531e99..091a1ac063f 100644 --- a/dbms/programs/client/Client.cpp +++ b/dbms/programs/client/Client.cpp @@ -218,7 +218,7 @@ private: configReadClient(config(), home_path); - global_context.makeGlobalContext(); + context.makeGlobalContext(); context.setApplicationType(Context::ApplicationType::CLIENT); /// settings and limits could be specified in config file, but passed settings has higher priority diff --git a/dbms/programs/obfuscator/Obfuscator.cpp b/dbms/programs/obfuscator/Obfuscator.cpp index ac6828c584b..3c20510d481 100644 --- a/dbms/programs/obfuscator/Obfuscator.cpp +++ b/dbms/programs/obfuscator/Obfuscator.cpp @@ -1024,7 +1024,7 @@ try } Context context = Context::createGlobal(); - context->makeGlobalContext(); + context.makeGlobalContext(); ReadBufferFromFileDescriptor file_in(STDIN_FILENO); WriteBufferFromFileDescriptor file_out(STDOUT_FILENO); diff --git a/dbms/programs/performance-test/PerformanceTestSuite.cpp b/dbms/programs/performance-test/PerformanceTestSuite.cpp index eb472985edd..5f5c13f3390 100644 --- a/dbms/programs/performance-test/PerformanceTestSuite.cpp +++ b/dbms/programs/performance-test/PerformanceTestSuite.cpp @@ -89,6 +89,7 @@ public: , input_files(input_files_) , log(&Poco::Logger::get("PerformanceTestSuite")) { + global_context.makeGlobalContext(); global_context.getSettingsRef().copyChangesFrom(cmd_settings); if (input_files.size() < 1) throw Exception("No tests were specified", ErrorCodes::BAD_ARGUMENTS); @@ -127,7 +128,6 @@ private: std::unordered_map> query_indexes; Context global_context = Context::createGlobal(); - global_context.makeGlobalContext(); std::shared_ptr report_builder; std::string server_version; From 81d6509fd4686ec027096571cbcd93629ef64d6c Mon Sep 17 00:00:00 2001 From: alesapin Date: Mon, 8 Jul 2019 13:46:04 +0300 Subject: [PATCH 27/62] Add two new images for coverage tests run --- docker/images.json | 2 + docker/test/coverage/Dockerfile | 4 +- docker/test/stateful/Dockerfile | 7 +- docker/test/stateful_with_coverage/Dockerfile | 16 ++++ docker/test/stateful_with_coverage/run.sh | 65 ++++++++++++++ .../test/stateful_with_coverage/s3downloader | 86 +++++++++++++++++++ docker/test/stateless/Dockerfile | 8 +- .../test/stateless_with_coverage/Dockerfile | 55 ++++++++++++ docker/test/stateless_with_coverage/run.sh | 46 ++++++++++ 9 files changed, 276 insertions(+), 13 deletions(-) create mode 100644 docker/test/stateful_with_coverage/Dockerfile create mode 100755 docker/test/stateful_with_coverage/run.sh create mode 100755 docker/test/stateful_with_coverage/s3downloader create mode 100644 docker/test/stateless_with_coverage/Dockerfile create mode 100755 docker/test/stateless_with_coverage/run.sh diff --git a/docker/images.json b/docker/images.json index 6ab12504a7d..e2282fcb653 100644 --- a/docker/images.json +++ b/docker/images.json @@ -7,7 +7,9 @@ "docker/test/performance": "yandex/clickhouse-performance-test", "docker/test/pvs": "yandex/clickhouse-pvs-test", "docker/test/stateful": "yandex/clickhouse-stateful-test", + "docker/test/stateful_with_coverage": "yandex/clickhouse-stateful-with-coverage-test", "docker/test/stateless": "yandex/clickhouse-stateless-test", + "docker/test/stateless_with_coverage": "yandex/clickhouse-stateless-with-coverage-test", "docker/test/unit": "yandex/clickhouse-unit-test", "docker/test/stress": "yandex/clickhouse-stress-test", "dbms/tests/integration/image": "yandex/clickhouse-integration-tests-runner" diff --git a/docker/test/coverage/Dockerfile b/docker/test/coverage/Dockerfile index 78835ec8c41..007384b69ac 100644 --- a/docker/test/coverage/Dockerfile +++ b/docker/test/coverage/Dockerfile @@ -31,5 +31,5 @@ ENV IGNORE='.*contrib.*' CMD dpkg -i /package_folder/clickhouse-common-static_*.deb; \ llvm-profdata-9 merge -sparse ${COVERAGE_DIR}/* -o clickhouse.profdata && \ - llvm-cov-9 export /usr/bin/clickhouse -instr-profile=clickhouse.profdata -arch=x86_64 -j=16 -format=lcov -skip-functions -ignore-filename-regex $IGNORE > output.lcov && \ - genhtml output.lcov --ignore-errors source --output-directory ${OUTPUT_DIR} + llvm-cov-9 export /usr/bin/clickhouse -instr-profile=clickhouse.profdata -j=16 -format=lcov -skip-functions -ignore-filename-regex $IGNORE > output.lcov && \ + genhtml output.lcov --ignore-errors source --output-directory ${OUTPUT_DIR} diff --git a/docker/test/stateful/Dockerfile b/docker/test/stateful/Dockerfile index 6e8017d0945..9c0f640a056 100644 --- a/docker/test/stateful/Dockerfile +++ b/docker/test/stateful/Dockerfile @@ -10,10 +10,8 @@ RUN apt-get update -y \ COPY s3downloader /s3downloader ENV DATASETS="hits visits" -ENV WITH_COVERAGE=0 -CMD chmod 777 /; \ - dpkg -i package_folder/clickhouse-common-static_*.deb; \ +CMD dpkg -i package_folder/clickhouse-common-static_*.deb; \ dpkg -i package_folder/clickhouse-common-static-dbg_*.deb; \ dpkg -i package_folder/clickhouse-server_*.deb; \ dpkg -i package_folder/clickhouse-client_*.deb; \ @@ -46,5 +44,4 @@ CMD chmod 777 /; \ && clickhouse-client --query "RENAME TABLE datasets.hits_v1 TO test.hits" \ && clickhouse-client --query "RENAME TABLE datasets.visits_v1 TO test.visits" \ && clickhouse-client --query "SHOW TABLES FROM test" \ - && clickhouse-test --shard --zookeeper --no-stateless $SKIP_TESTS_OPTION 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt; \ - if [ $WITH_COVERAGE -eq 1 ]; then cp /*.profraw /profraw ||:; fi + && clickhouse-test --shard --zookeeper --no-stateless $SKIP_TESTS_OPTION 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt; diff --git a/docker/test/stateful_with_coverage/Dockerfile b/docker/test/stateful_with_coverage/Dockerfile new file mode 100644 index 00000000000..2f7764db2dc --- /dev/null +++ b/docker/test/stateful_with_coverage/Dockerfile @@ -0,0 +1,16 @@ +# docker build -t yandex/clickhouse-stateful-test . +FROM yandex/clickhouse-stateless-test + +RUN apt-get update -y \ + && env DEBIAN_FRONTEND=noninteractive \ + apt-get install --yes --no-install-recommends \ + python-requests \ + llvm-8 + +COPY s3downloader /s3downloader +COPY run.sh /run.sh + +ENV DATASETS="hits visits" + +CMD ["/bin/bash", "/run.sh"] + diff --git a/docker/test/stateful_with_coverage/run.sh b/docker/test/stateful_with_coverage/run.sh new file mode 100755 index 00000000000..555b6807d4c --- /dev/null +++ b/docker/test/stateful_with_coverage/run.sh @@ -0,0 +1,65 @@ +#!/bin/bash + +set -x + +kill_clickhouse () { + while kill -0 `pgrep -u clickhouse`; + do + kill `pgrep -u clickhouse` 2>/dev/null + echo "Process" `pgrep -u clickhouse` "still alive" + sleep 10 + done +} + +start_clickhouse () { + LLVM_PROFILE_FILE='server_%h_%p_%m.profraw' sudo -Eu clickhouse /usr/bin/clickhouse-server --config /etc/clickhouse-server/config.xml & +} + +chmod 777 / + +dpkg -i package_folder/clickhouse-common-static_*.deb; \ + dpkg -i package_folder/clickhouse-common-static-dbg_*.deb; \ + dpkg -i package_folder/clickhouse-server_*.deb; \ + dpkg -i package_folder/clickhouse-client_*.deb; \ + dpkg -i package_folder/clickhouse-test_*.deb + +ln -s /usr/share/clickhouse-test/config/zookeeper.xml /etc/clickhouse-server/config.d/; \ + ln -s /usr/share/clickhouse-test/config/listen.xml /etc/clickhouse-server/config.d/; \ + ln -s /usr/share/clickhouse-test/config/part_log.xml /etc/clickhouse-server/config.d/; \ + ln -s /usr/share/clickhouse-test/config/log_queries.xml /etc/clickhouse-server/users.d/; \ + ln -s /usr/share/clickhouse-test/config/readonly.xml /etc/clickhouse-server/users.d/; \ + ln -s /usr/share/clickhouse-test/config/ints_dictionary.xml /etc/clickhouse-server/; \ + ln -s /usr/share/clickhouse-test/config/strings_dictionary.xml /etc/clickhouse-server/; \ + ln -s /usr/share/clickhouse-test/config/decimals_dictionary.xml /etc/clickhouse-server/; \ + ln -s /usr/lib/llvm-8/bin/llvm-symbolizer /usr/bin/llvm-symbolizer + +service zookeeper start + +sleep 5 + +start_clickhouse + +sleep 5 + +/s3downloader --dataset-names $DATASETS +chmod 777 -R /var/lib/clickhouse + +LLVM_PROFILE_FILE='client.profraw' clickhouse-client --query "SHOW DATABASES" +LLVM_PROFILE_FILE='client.profraw' clickhouse-client --query "CREATE DATABASE datasets" +LLVM_PROFILE_FILE='client.profraw' clickhouse-client --query "CREATE DATABASE test" + +kill_clickhouse +start_clickhouse + +sleep 10 + +LLVM_PROFILE_FILE='client.profraw' clickhouse-client --query "SHOW TABLES FROM datasets" +LLVM_PROFILE_FILE='client.profraw' clickhouse-client --query "SHOW TABLES FROM test" +LLVM_PROFILE_FILE='client.profraw' clickhouse-client --query "RENAME TABLE datasets.hits_v1 TO test.hits" +LLVM_PROFILE_FILE='client.profraw' clickhouse-client --query "RENAME TABLE datasets.visits_v1 TO test.visits" +LLVM_PROFILE_FILE='client.profraw' clickhouse-client --query "SHOW TABLES FROM test" +LLVM_PROFILE_FILE='client.profraw' clickhouse-test --shard --zookeeper --no-stateless 00146 $SKIP_TESTS_OPTION 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt + +kill_clickhouse + +cp /*.profraw /profraw ||: diff --git a/docker/test/stateful_with_coverage/s3downloader b/docker/test/stateful_with_coverage/s3downloader new file mode 100755 index 00000000000..f8e2bf3cbe4 --- /dev/null +++ b/docker/test/stateful_with_coverage/s3downloader @@ -0,0 +1,86 @@ +#!/usr/bin/env python +# -*- coding: utf-8 -*- +import os +import sys +import tarfile +import logging +import argparse +import requests +import tempfile + + +DEFAULT_URL = 'https://clickhouse-datasets.s3.yandex.net' + +AVAILABLE_DATASETS = { + 'hits': 'hits_v1.tar', + 'visits': 'visits_v1.tar', +} + +def _get_temp_file_name(): + return os.path.join(tempfile._get_default_tempdir(), next(tempfile._get_candidate_names())) + +def build_url(base_url, dataset): + return os.path.join(base_url, dataset, 'partitions', AVAILABLE_DATASETS[dataset]) + +def dowload_with_progress(url, path): + logging.info("Downloading from %s to temp path %s", url, path) + with open(path, 'w') as f: + response = requests.get(url, stream=True) + response.raise_for_status() + total_length = response.headers.get('content-length') + if total_length is None or int(total_length) == 0: + logging.info("No content-length, will download file without progress") + f.write(response.content) + else: + dl = 0 + total_length = int(total_length) + logging.info("Content length is %ld bytes", total_length) + for data in response.iter_content(chunk_size=4096): + dl += len(data) + f.write(data) + if sys.stdout.isatty(): + done = int(50 * dl / total_length) + percent = int(100 * float(dl) / total_length) + sys.stdout.write("\r[{}{}] {}%".format('=' * done, ' ' * (50-done), percent)) + sys.stdout.flush() + sys.stdout.write("\n") + logging.info("Downloading finished") + +def unpack_to_clickhouse_directory(tar_path, clickhouse_path): + logging.info("Will unpack data from temp path %s to clickhouse db %s", tar_path, clickhouse_path) + with tarfile.open(tar_path, 'r') as comp_file: + comp_file.extractall(path=clickhouse_path) + logging.info("Unpack finished") + + +if __name__ == "__main__": + logging.basicConfig(level=logging.INFO) + + parser = argparse.ArgumentParser( + description="Simple tool for dowloading datasets for clickhouse from S3") + + parser.add_argument('--dataset-names', required=True, nargs='+', choices=AVAILABLE_DATASETS.keys()) + parser.add_argument('--url-prefix', default=DEFAULT_URL) + parser.add_argument('--clickhouse-data-path', default='/var/lib/clickhouse/') + + args = parser.parse_args() + datasets = args.dataset_names + logging.info("Will fetch following datasets: %s", ', '.join(datasets)) + for dataset in datasets: + logging.info("Processing %s", dataset) + temp_archive_path = _get_temp_file_name() + try: + download_url_for_dataset = build_url(args.url_prefix, dataset) + dowload_with_progress(download_url_for_dataset, temp_archive_path) + unpack_to_clickhouse_directory(temp_archive_path, args.clickhouse_data_path) + except Exception as ex: + logging.info("Some exception occured %s", str(ex)) + raise + finally: + logging.info("Will remove dowloaded file %s from filesystem if it exists", temp_archive_path) + if os.path.exists(temp_archive_path): + os.remove(temp_archive_path) + logging.info("Processing of %s finished", dataset) + logging.info("Fetch finished, enjoy your tables!") + + diff --git a/docker/test/stateless/Dockerfile b/docker/test/stateless/Dockerfile index ea35cf03189..16f8fc437bc 100644 --- a/docker/test/stateless/Dockerfile +++ b/docker/test/stateless/Dockerfile @@ -29,11 +29,9 @@ RUN apt-get update -y \ ENV TZ=Europe/Moscow RUN ln -snf /usr/share/zoneinfo/$TZ /etc/localtime && echo $TZ > /etc/timezone -ENV WITH_COVERAGE=0 -CMD chmod 777 /; \ - dpkg -i package_folder/clickhouse-common-static_*.deb; \ +CMD dpkg -i package_folder/clickhouse-common-static_*.deb; \ dpkg -i package_folder/clickhouse-common-static-dbg_*.deb; \ dpkg -i package_folder/clickhouse-server_*.deb; \ dpkg -i package_folder/clickhouse-client_*.deb; \ @@ -54,6 +52,4 @@ CMD chmod 777 /; \ echo "TSAN_SYMBOLIZER_PATH=/usr/lib/llvm-8/bin/llvm-symbolizer" >> /etc/environment; \ echo "LLVM_SYMBOLIZER_PATH=/usr/lib/llvm-6.0/bin/llvm-symbolizer" >> /etc/environment; \ service zookeeper start; sleep 5; \ - service clickhouse-server start && sleep 5 && \ - clickhouse-test --shard --zookeeper $ADDITIONAL_OPTIONS $SKIP_TESTS_OPTION 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt; \ - if [ $WITH_COVERAGE -eq 1 ]; then cp /*.profraw /profraw ||:; fi + service clickhouse-server start && sleep 5 && clickhouse-test --shard --zookeeper $ADDITIONAL_OPTIONS $SKIP_TESTS_OPTION 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt diff --git a/docker/test/stateless_with_coverage/Dockerfile b/docker/test/stateless_with_coverage/Dockerfile new file mode 100644 index 00000000000..df37158763e --- /dev/null +++ b/docker/test/stateless_with_coverage/Dockerfile @@ -0,0 +1,55 @@ +# docker build -t yandex/clickhouse-stateless-with-coverage-test . +FROM yandex/clickhouse-deb-builder + +RUN apt-get update -y \ + && env DEBIAN_FRONTEND=noninteractive \ + apt-get install --yes --no-install-recommends \ + bash \ + tzdata \ + fakeroot \ + debhelper \ + zookeeper \ + zookeeperd \ + expect \ + python \ + python-lxml \ + python-termcolor \ + python-requests \ + curl \ + sudo \ + openssl \ + netcat-openbsd \ + telnet \ + moreutils \ + brotli \ + gdb \ + lsof + + +ENV TZ=Europe/Moscow +RUN ln -snf /usr/share/zoneinfo/$TZ /etc/localtime && echo $TZ > /etc/timezone +ENV LLVM_PROFILE_FILE='code_%h_%p_%m.profraw' + + +CMD chmod 777 /; \ + echo "LLVM_PROFILE_FILE='code_%h_%p_%m.profraw'" >> /etc/environment; \ + dpkg -i package_folder/clickhouse-common-static_*.deb; \ + dpkg -i package_folder/clickhouse-common-static-dbg_*.deb; \ + dpkg -i package_folder/clickhouse-server_*.deb; \ + dpkg -i package_folder/clickhouse-client_*.deb; \ + dpkg -i package_folder/clickhouse-test_*.deb; \ + ln -s /usr/share/clickhouse-test/config/zookeeper.xml /etc/clickhouse-server/config.d/; \ + ln -s /usr/share/clickhouse-test/config/listen.xml /etc/clickhouse-server/config.d/; \ + ln -s /usr/share/clickhouse-test/config/part_log.xml /etc/clickhouse-server/config.d/; \ + ln -s /usr/share/clickhouse-test/config/log_queries.xml /etc/clickhouse-server/users.d/; \ + ln -s /usr/share/clickhouse-test/config/readonly.xml /etc/clickhouse-server/users.d/; \ + ln -s /usr/share/clickhouse-test/config/ints_dictionary.xml /etc/clickhouse-server/; \ + ln -s /usr/share/clickhouse-test/config/strings_dictionary.xml /etc/clickhouse-server/; \ + ln -s /usr/share/clickhouse-test/config/decimals_dictionary.xml /etc/clickhouse-server/; \ + ln -s /usr/lib/llvm-8/bin/llvm-symbolizer /usr/bin/llvm-symbolizer; \ + service zookeeper start; sleep 5; \ + LLVM_PROFILE_FILE='code_%h_%p_%m.profraw' sudo -Eu clickhouse /usr/bin/clickhouse-server --config /etc/clickhouse-server/config.xml 1>/dev/null 2>/dev/null & \ + sleep 10; \ + LLVM_PROFILE_FILE='client.profraw' clickhouse-test --shard --zookeeper $ADDITIONAL_OPTIONS $SKIP_TESTS_OPTION 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt; \ + while kill -0 `pgrep -u clickhouse`; do kill `pgrep -u clickhouse` 2>/dev/null; echo "Process" `pgrep -u clickhouse` "still alive"; sleep 10; done; \ + cp /*.profraw /profraw ||: diff --git a/docker/test/stateless_with_coverage/run.sh b/docker/test/stateless_with_coverage/run.sh new file mode 100755 index 00000000000..c6fcc9c2e36 --- /dev/null +++ b/docker/test/stateless_with_coverage/run.sh @@ -0,0 +1,46 @@ +#!/bin/bash + +set -x + +kill_clickhouse () { + while kill -0 `pgrep -u clickhouse`; + do + kill `pgrep -u clickhouse` 2>/dev/null + echo "Process" `pgrep -u clickhouse` "still alive" + sleep 10 + done +} + +start_clickhouse () { + LLVM_PROFILE_FILE='server_%h_%p_%m.profraw' sudo -Eu clickhouse /usr/bin/clickhouse-server --config /etc/clickhouse-server/config.xml & +} + +chmod 777 / +dpkg -i package_folder/clickhouse-common-static_*.deb; \ + dpkg -i package_folder/clickhouse-common-static-dbg_*.deb; \ + dpkg -i package_folder/clickhouse-server_*.deb; \ + dpkg -i package_folder/clickhouse-client_*.deb; \ + dpkg -i package_folder/clickhouse-test_*.deb + +ln -s /usr/share/clickhouse-test/config/zookeeper.xml /etc/clickhouse-server/config.d/; \ + ln -s /usr/share/clickhouse-test/config/listen.xml /etc/clickhouse-server/config.d/; \ + ln -s /usr/share/clickhouse-test/config/part_log.xml /etc/clickhouse-server/config.d/; \ + ln -s /usr/share/clickhouse-test/config/log_queries.xml /etc/clickhouse-server/users.d/; \ + ln -s /usr/share/clickhouse-test/config/readonly.xml /etc/clickhouse-server/users.d/; \ + ln -s /usr/share/clickhouse-test/config/ints_dictionary.xml /etc/clickhouse-server/; \ + ln -s /usr/share/clickhouse-test/config/strings_dictionary.xml /etc/clickhouse-server/; \ + ln -s /usr/share/clickhouse-test/config/decimals_dictionary.xml /etc/clickhouse-server/; \ + ln -s /usr/lib/llvm-8/bin/llvm-symbolizer /usr/bin/llvm-symbolizer + +service zookeeper start +sleep 5 + +start_clickhouse + +sleep 10 + +LLVM_PROFILE_FILE='client.profraw' clickhouse-test --shard --zookeeper $ADDITIONAL_OPTIONS $SKIP_TESTS_OPTION 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt + +kill_clickhouse + +cp /*.profraw /profraw ||: From 53b5d9f0eb08b9294699033dfdd50324e71dc944 Mon Sep 17 00:00:00 2001 From: alesapin Date: Mon, 8 Jul 2019 13:47:04 +0300 Subject: [PATCH 28/62] Remove accident change --- docker/test/stateful/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/test/stateful/Dockerfile b/docker/test/stateful/Dockerfile index 9c0f640a056..d7707f1c3e0 100644 --- a/docker/test/stateful/Dockerfile +++ b/docker/test/stateful/Dockerfile @@ -44,4 +44,4 @@ CMD dpkg -i package_folder/clickhouse-common-static_*.deb; \ && clickhouse-client --query "RENAME TABLE datasets.hits_v1 TO test.hits" \ && clickhouse-client --query "RENAME TABLE datasets.visits_v1 TO test.visits" \ && clickhouse-client --query "SHOW TABLES FROM test" \ - && clickhouse-test --shard --zookeeper --no-stateless $SKIP_TESTS_OPTION 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt; + && clickhouse-test --shard --zookeeper --no-stateless $SKIP_TESTS_OPTION 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt From d44da017c237332af78b7a4dc6cf6654f45d3601 Mon Sep 17 00:00:00 2001 From: alesapin Date: Mon, 8 Jul 2019 13:49:59 +0300 Subject: [PATCH 29/62] remove redundant change --- docker/test/stateful_with_coverage/run.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/test/stateful_with_coverage/run.sh b/docker/test/stateful_with_coverage/run.sh index 555b6807d4c..e8269054d07 100755 --- a/docker/test/stateful_with_coverage/run.sh +++ b/docker/test/stateful_with_coverage/run.sh @@ -58,7 +58,7 @@ LLVM_PROFILE_FILE='client.profraw' clickhouse-client --query "SHOW TABLES FROM t LLVM_PROFILE_FILE='client.profraw' clickhouse-client --query "RENAME TABLE datasets.hits_v1 TO test.hits" LLVM_PROFILE_FILE='client.profraw' clickhouse-client --query "RENAME TABLE datasets.visits_v1 TO test.visits" LLVM_PROFILE_FILE='client.profraw' clickhouse-client --query "SHOW TABLES FROM test" -LLVM_PROFILE_FILE='client.profraw' clickhouse-test --shard --zookeeper --no-stateless 00146 $SKIP_TESTS_OPTION 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt +LLVM_PROFILE_FILE='client.profraw' clickhouse-test --shard --zookeeper --no-stateless $SKIP_TESTS_OPTION 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt kill_clickhouse From 0eff6f54136e3c3d380cada6139051b2bdd7d806 Mon Sep 17 00:00:00 2001 From: alesapin Date: Mon, 8 Jul 2019 14:09:46 +0300 Subject: [PATCH 30/62] Run cmake to generate some cpp files --- docker/test/coverage/Dockerfile | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docker/test/coverage/Dockerfile b/docker/test/coverage/Dockerfile index 007384b69ac..c0c31a42571 100644 --- a/docker/test/coverage/Dockerfile +++ b/docker/test/coverage/Dockerfile @@ -1,5 +1,5 @@ # docker build -t yandex/clickhouse-coverage . -FROM ubuntu:18.04 +FROM yandex/clickhouse-deb-builder RUN apt-get --allow-unauthenticated update -y \ && env DEBIAN_FRONTEND=noninteractive \ @@ -29,7 +29,8 @@ ENV OUTPUT_DIR=/output ENV IGNORE='.*contrib.*' -CMD dpkg -i /package_folder/clickhouse-common-static_*.deb; \ +CMD mkdir -p /build/obj-x86_64-linux-gnu && cd /build/obj-x86_64-linux-gnu && CC=clang-7 CXX=clang++-7 cmake .. && cd /; \ + dpkg -i /package_folder/clickhouse-common-static_*.deb; \ llvm-profdata-9 merge -sparse ${COVERAGE_DIR}/* -o clickhouse.profdata && \ llvm-cov-9 export /usr/bin/clickhouse -instr-profile=clickhouse.profdata -j=16 -format=lcov -skip-functions -ignore-filename-regex $IGNORE > output.lcov && \ genhtml output.lcov --ignore-errors source --output-directory ${OUTPUT_DIR} From c1a8cce20da2654e0bfd09d146dc0d184eadf867 Mon Sep 17 00:00:00 2001 From: alesapin Date: Mon, 8 Jul 2019 14:21:04 +0300 Subject: [PATCH 31/62] Fix stateless with coverage image --- .../test/stateless_with_coverage/Dockerfile | 26 ++----------------- 1 file changed, 2 insertions(+), 24 deletions(-) diff --git a/docker/test/stateless_with_coverage/Dockerfile b/docker/test/stateless_with_coverage/Dockerfile index df37158763e..e0e0463f1b6 100644 --- a/docker/test/stateless_with_coverage/Dockerfile +++ b/docker/test/stateless_with_coverage/Dockerfile @@ -28,28 +28,6 @@ RUN apt-get update -y \ ENV TZ=Europe/Moscow RUN ln -snf /usr/share/zoneinfo/$TZ /etc/localtime && echo $TZ > /etc/timezone -ENV LLVM_PROFILE_FILE='code_%h_%p_%m.profraw' +COPY run.sh /run.sh - -CMD chmod 777 /; \ - echo "LLVM_PROFILE_FILE='code_%h_%p_%m.profraw'" >> /etc/environment; \ - dpkg -i package_folder/clickhouse-common-static_*.deb; \ - dpkg -i package_folder/clickhouse-common-static-dbg_*.deb; \ - dpkg -i package_folder/clickhouse-server_*.deb; \ - dpkg -i package_folder/clickhouse-client_*.deb; \ - dpkg -i package_folder/clickhouse-test_*.deb; \ - ln -s /usr/share/clickhouse-test/config/zookeeper.xml /etc/clickhouse-server/config.d/; \ - ln -s /usr/share/clickhouse-test/config/listen.xml /etc/clickhouse-server/config.d/; \ - ln -s /usr/share/clickhouse-test/config/part_log.xml /etc/clickhouse-server/config.d/; \ - ln -s /usr/share/clickhouse-test/config/log_queries.xml /etc/clickhouse-server/users.d/; \ - ln -s /usr/share/clickhouse-test/config/readonly.xml /etc/clickhouse-server/users.d/; \ - ln -s /usr/share/clickhouse-test/config/ints_dictionary.xml /etc/clickhouse-server/; \ - ln -s /usr/share/clickhouse-test/config/strings_dictionary.xml /etc/clickhouse-server/; \ - ln -s /usr/share/clickhouse-test/config/decimals_dictionary.xml /etc/clickhouse-server/; \ - ln -s /usr/lib/llvm-8/bin/llvm-symbolizer /usr/bin/llvm-symbolizer; \ - service zookeeper start; sleep 5; \ - LLVM_PROFILE_FILE='code_%h_%p_%m.profraw' sudo -Eu clickhouse /usr/bin/clickhouse-server --config /etc/clickhouse-server/config.xml 1>/dev/null 2>/dev/null & \ - sleep 10; \ - LLVM_PROFILE_FILE='client.profraw' clickhouse-test --shard --zookeeper $ADDITIONAL_OPTIONS $SKIP_TESTS_OPTION 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt; \ - while kill -0 `pgrep -u clickhouse`; do kill `pgrep -u clickhouse` 2>/dev/null; echo "Process" `pgrep -u clickhouse` "still alive"; sleep 10; done; \ - cp /*.profraw /profraw ||: +CMD ["/bin/bash", "/run.sh"] From b86ff47e5a530e59107924d698e6ded86c5befa1 Mon Sep 17 00:00:00 2001 From: Artem Konovalov Date: Mon, 8 Jul 2019 16:50:06 +0300 Subject: [PATCH 32/62] update default value max_ast_elements parameter --- docs/ru/operations/settings/query_complexity.md | 2 +- website/deprecated/reference_en.html | 2 +- website/deprecated/reference_ru.html | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/ru/operations/settings/query_complexity.md b/docs/ru/operations/settings/query_complexity.md index ae7232d8ae8..24286999685 100644 --- a/docs/ru/operations/settings/query_complexity.md +++ b/docs/ru/operations/settings/query_complexity.md @@ -157,7 +157,7 @@ ## max_ast_elements Максимальное количество элементов синтаксического дерева запроса. Если превышено - кидается исключение. -Аналогично, проверяется уже после парсинга запроса. По умолчанию: 10 000. +Аналогично, проверяется уже после парсинга запроса. По умолчанию: 50 000. ## max_rows_in_set diff --git a/website/deprecated/reference_en.html b/website/deprecated/reference_en.html index a789aeb238d..2e6fe0ac30b 100644 --- a/website/deprecated/reference_en.html +++ b/website/deprecated/reference_en.html @@ -7014,7 +7014,7 @@ Maximum nesting depth of a query syntactic tree. If exceeded, an exception is th ===max_ast_elements=== Maximum number of elements in a query syntactic tree. If exceeded, an exception is thrown. -In the same way as the previous setting, it is checked only after parsing the query. By default, 10,000. +In the same way as the previous setting, it is checked only after parsing the query. By default, 50,000. ===max_rows_in_set=== diff --git a/website/deprecated/reference_ru.html b/website/deprecated/reference_ru.html index e18966ae322..89c91d7d1c1 100644 --- a/website/deprecated/reference_ru.html +++ b/website/deprecated/reference_ru.html @@ -7260,7 +7260,7 @@ any (только для group_by_overflow_mode) - продолжить агре ===max_ast_elements=== Максимальное количество элементов синтаксического дерева запроса. Если превышено - кидается исключение. -Аналогично, проверяется уже после парсинга запроса. По умолчанию: 10 000. +Аналогично, проверяется уже после парсинга запроса. По умолчанию: 50 000. ===max_rows_in_set=== From c5bc5204f86f8f583cf71871b4e79c5779b596eb Mon Sep 17 00:00:00 2001 From: stavrolia Date: Mon, 8 Jul 2019 17:03:14 +0300 Subject: [PATCH 33/62] CAdd changelog for backporting of 19.7 and 19.9 --- CHANGELOG.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ed9fca1a4b..5106186cf27 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,20 @@ +## ClickHouse release 19.9.4.1, 2019-07-05 + +### Bug Fix +* Fix potential infinite sleeping of low-priority queries. [#5842](https://github.com/yandex/ClickHouse/pull/5842) ([alexey-milovidov](https://github.com/alexey-milovidov)) +* Fix rare bug in checking of part with LowCardinality column. [#5832](https://github.com/yandex/ClickHouse/pull/5832) ([alesapin](https://github.com/alesapin)) +* Fix ClickHouse determines default time zone as UCT instead of UTC. [#5828](https://github.com/yandex/ClickHouse/pull/5828) ([alexey-milovidov](https://github.com/alexey-milovidov)) +* Fix bug about executing distributed DROP/ALTER/TRUNCATE/OPTIMIZE ON CLUSTER queries on follower replica before leader replica. Now they will be executed directly on leader replica. [#5757](https://github.com/yandex/ClickHouse/pull/5757) ([alesapin](https://github.com/alesapin)) +* Fix segfault in Delta codec which affects columns with values less than 32 bits size. The bug led to random memory corruption. [#5786](https://github.com/yandex/ClickHouse/pull/5786) ([alesapin](https://github.com/alesapin)) +* Fix race condition, which cause that some queries may not appear in query_log after SYSTEM FLUSH LOGS query. [#5802](https://github.com/yandex/ClickHouse/pull/5802) ([Anton Popov](https://github.com/CurtizJ)) +* Fix segfault in TTL merge with non-physical columns in block. [#5819](https://github.com/yandex/ClickHouse/pull/5819) ([Anton Popov](https://github.com/CurtizJ)) +* Added missing support for constant arguments to evalMLModel function. [#5820](https://github.com/yandex/ClickHouse/pull/5820) ([alexey-milovidov](https://github.com/alexey-milovidov)) + +## ClickHouse release 19.7.6.1, 2019-07-05 + +### Bug Fix +* Fix push require columns with join. [#5192](https://github.com/yandex/ClickHouse/pull/5192) ([Winter Zhang](https://github.com/zhang2014)) + ## ClickHouse release 19.9.2.4, 2019-06-24 ### New Feature From 3f163f4552b45afd3dadb127b816a6a507c7d2b7 Mon Sep 17 00:00:00 2001 From: stavrolia Date: Mon, 8 Jul 2019 17:59:02 +0300 Subject: [PATCH 34/62] Add ru version --- CHANGELOG.md | 1 - CHANGELOG_RU.md | 16 ++++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5106186cf27..5eb63dd95b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,6 @@ * Fix ClickHouse determines default time zone as UCT instead of UTC. [#5828](https://github.com/yandex/ClickHouse/pull/5828) ([alexey-milovidov](https://github.com/alexey-milovidov)) * Fix bug about executing distributed DROP/ALTER/TRUNCATE/OPTIMIZE ON CLUSTER queries on follower replica before leader replica. Now they will be executed directly on leader replica. [#5757](https://github.com/yandex/ClickHouse/pull/5757) ([alesapin](https://github.com/alesapin)) * Fix segfault in Delta codec which affects columns with values less than 32 bits size. The bug led to random memory corruption. [#5786](https://github.com/yandex/ClickHouse/pull/5786) ([alesapin](https://github.com/alesapin)) -* Fix race condition, which cause that some queries may not appear in query_log after SYSTEM FLUSH LOGS query. [#5802](https://github.com/yandex/ClickHouse/pull/5802) ([Anton Popov](https://github.com/CurtizJ)) * Fix segfault in TTL merge with non-physical columns in block. [#5819](https://github.com/yandex/ClickHouse/pull/5819) ([Anton Popov](https://github.com/CurtizJ)) * Added missing support for constant arguments to evalMLModel function. [#5820](https://github.com/yandex/ClickHouse/pull/5820) ([alexey-milovidov](https://github.com/alexey-milovidov)) diff --git a/CHANGELOG_RU.md b/CHANGELOG_RU.md index 265a60e342a..0e0470986ac 100644 --- a/CHANGELOG_RU.md +++ b/CHANGELOG_RU.md @@ -1,3 +1,19 @@ +## ClickHouse release 19.9.4.1, 2019-07-05 + +### Исправления ошибок +* Исправлена существовавшая возможность ухода в бесконечное ожидание на низко-приоритетных запросах. [#5842](https://github.com/yandex/ClickHouse/pull/5842) ([alexey-milovidov](https://github.com/alexey-milovidov)) +* Исправлена ошибка в проверке частей в LowCardinality колонках. [#5832](https://github.com/yandex/ClickHouse/pull/5832) ([alesapin](https://github.com/alesapin)) +* Исправлена ошибка определения формата таймзоны по умолчанию (UCT вместо UTC). [#5828](https://github.com/yandex/ClickHouse/pull/5828) ([alexey-milovidov](https://github.com/alexey-milovidov)) +* Исправлена ошибка в распределенных запросах вида DROP/ALTER/TRUNCATE/OPTIMIZE ON CLUSTER. [#5757](https://github.com/yandex/ClickHouse/pull/5757) ([alesapin](https://github.com/alesapin)) +* Исправлена ошибка сегментации в колонках с величинами размером меньше 32 бит. Ошибка могла приводить к повреждениям памяти. [#5786](https://github.com/yandex/ClickHouse/pull/5786) ([alesapin](https://github.com/alesapin)) +* Исправлена ошибка сегментации при слиянии кусков с истекшим TTL в случае, когда в блоке присутствуют столбцы, не входящие в структуру таблицы. [#5819](https://github.com/yandex/ClickHouse/pull/5819) ([Anton Popov](https://github.com/CurtizJ)) +* Добавлена отсутствовавшая поддержка константных аргументов для функции evalMLModel. [#5820](https://github.com/yandex/ClickHouse/pull/5820) ([alexey-milovidov](https://github.com/alexey-milovidov)) + +## ClickHouse release 19.7.6.1, 2019-07-05 + +### Исправления ошибок +* Исправлена просадка производительности в методе JOIN в некоторых видах запросов. [#5192](https://github.com/yandex/ClickHouse/pull/5192) ([Winter Zhang](https://github.com/zhang2014)) + ## ClickHouse release 19.8.3.8, 2019-06-11 ### Новые возможности From 50c06ad41838bb76721b2404f8a6325c8fe8a188 Mon Sep 17 00:00:00 2001 From: stavrolia Date: Mon, 8 Jul 2019 18:24:05 +0300 Subject: [PATCH 35/62] Fix --- CHANGELOG.md | 1 + CHANGELOG_RU.md | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5eb63dd95b5..61a7f3ae39b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ * Fix ClickHouse determines default time zone as UCT instead of UTC. [#5828](https://github.com/yandex/ClickHouse/pull/5828) ([alexey-milovidov](https://github.com/alexey-milovidov)) * Fix bug about executing distributed DROP/ALTER/TRUNCATE/OPTIMIZE ON CLUSTER queries on follower replica before leader replica. Now they will be executed directly on leader replica. [#5757](https://github.com/yandex/ClickHouse/pull/5757) ([alesapin](https://github.com/alesapin)) * Fix segfault in Delta codec which affects columns with values less than 32 bits size. The bug led to random memory corruption. [#5786](https://github.com/yandex/ClickHouse/pull/5786) ([alesapin](https://github.com/alesapin)) +* Fix race condition, which cause that some queries may not appear in query_log after SYSTEM FLUSH LOGS query. [#5685](https://github.com/yandex/ClickHouse/pull/5685) ([Anton Popov](https://github.com/CurtizJ)) * Fix segfault in TTL merge with non-physical columns in block. [#5819](https://github.com/yandex/ClickHouse/pull/5819) ([Anton Popov](https://github.com/CurtizJ)) * Added missing support for constant arguments to evalMLModel function. [#5820](https://github.com/yandex/ClickHouse/pull/5820) ([alexey-milovidov](https://github.com/alexey-milovidov)) diff --git a/CHANGELOG_RU.md b/CHANGELOG_RU.md index 0e0470986ac..1c03f95dc17 100644 --- a/CHANGELOG_RU.md +++ b/CHANGELOG_RU.md @@ -5,7 +5,8 @@ * Исправлена ошибка в проверке частей в LowCardinality колонках. [#5832](https://github.com/yandex/ClickHouse/pull/5832) ([alesapin](https://github.com/alesapin)) * Исправлена ошибка определения формата таймзоны по умолчанию (UCT вместо UTC). [#5828](https://github.com/yandex/ClickHouse/pull/5828) ([alexey-milovidov](https://github.com/alexey-milovidov)) * Исправлена ошибка в распределенных запросах вида DROP/ALTER/TRUNCATE/OPTIMIZE ON CLUSTER. [#5757](https://github.com/yandex/ClickHouse/pull/5757) ([alesapin](https://github.com/alesapin)) -* Исправлена ошибка сегментации в колонках с величинами размером меньше 32 бит. Ошибка могла приводить к повреждениям памяти. [#5786](https://github.com/yandex/ClickHouse/pull/5786) ([alesapin](https://github.com/alesapin)) +* Исправлена ошибка сегментации в кодеке сжатия Delta в колонках с величинами размером меньше 32 бит. Ошибка могла приводить к повреждениям памяти. [#5786](https://github.com/yandex/ClickHouse/pull/5786) ([alesapin](https://github.com/alesapin)) +* Исправлена ошибка, которая при распределенных запросах могла привести к тому, что некоторые запросы не появлялись в query_log после SYSTEM FLUSH LOGS запроса. [#5685](https://github.com/yandex/ClickHouse/pull/5685) ([Anton Popov](https://github.com/CurtizJ)) * Исправлена ошибка сегментации при слиянии кусков с истекшим TTL в случае, когда в блоке присутствуют столбцы, не входящие в структуру таблицы. [#5819](https://github.com/yandex/ClickHouse/pull/5819) ([Anton Popov](https://github.com/CurtizJ)) * Добавлена отсутствовавшая поддержка константных аргументов для функции evalMLModel. [#5820](https://github.com/yandex/ClickHouse/pull/5820) ([alexey-milovidov](https://github.com/alexey-milovidov)) From 98d27b746a5726f219622867fcf783ade8cb87b9 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Mon, 8 Jul 2019 18:25:17 +0300 Subject: [PATCH 36/62] Updated simdjson just in case --- contrib/simdjson | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/simdjson b/contrib/simdjson index 2151ad7f34c..3bd3116cf8f 160000 --- a/contrib/simdjson +++ b/contrib/simdjson @@ -1 +1 @@ -Subproject commit 2151ad7f34cf773a23f086e941d661f8a8873144 +Subproject commit 3bd3116cf8faf6d482dc31423b16533bfa2696f7 From 36446cfc97a6a3b5d670253d4376dc7830a52c7b Mon Sep 17 00:00:00 2001 From: alesapin Date: Mon, 8 Jul 2019 18:41:15 +0300 Subject: [PATCH 37/62] Fix coverage scripts --- docker/test/stateful_with_coverage/run.sh | 5 +++++ docker/test/stateless_with_coverage/run.sh | 7 +++++++ 2 files changed, 12 insertions(+) diff --git a/docker/test/stateful_with_coverage/run.sh b/docker/test/stateful_with_coverage/run.sh index e8269054d07..573fb3e4d8b 100755 --- a/docker/test/stateful_with_coverage/run.sh +++ b/docker/test/stateful_with_coverage/run.sh @@ -23,6 +23,10 @@ dpkg -i package_folder/clickhouse-common-static_*.deb; \ dpkg -i package_folder/clickhouse-client_*.deb; \ dpkg -i package_folder/clickhouse-test_*.deb +mkdir -p /var/lib/clickhouse +mkdir -p /var/log/clickhouse-server +chmod 777 -R /var/log/clickhouse-server/ + ln -s /usr/share/clickhouse-test/config/zookeeper.xml /etc/clickhouse-server/config.d/; \ ln -s /usr/share/clickhouse-test/config/listen.xml /etc/clickhouse-server/config.d/; \ ln -s /usr/share/clickhouse-test/config/part_log.xml /etc/clickhouse-server/config.d/; \ @@ -42,6 +46,7 @@ start_clickhouse sleep 5 /s3downloader --dataset-names $DATASETS + chmod 777 -R /var/lib/clickhouse LLVM_PROFILE_FILE='client.profraw' clickhouse-client --query "SHOW DATABASES" diff --git a/docker/test/stateless_with_coverage/run.sh b/docker/test/stateless_with_coverage/run.sh index c6fcc9c2e36..9dc912d6d6c 100755 --- a/docker/test/stateless_with_coverage/run.sh +++ b/docker/test/stateless_with_coverage/run.sh @@ -16,12 +16,19 @@ start_clickhouse () { } chmod 777 / + dpkg -i package_folder/clickhouse-common-static_*.deb; \ dpkg -i package_folder/clickhouse-common-static-dbg_*.deb; \ dpkg -i package_folder/clickhouse-server_*.deb; \ dpkg -i package_folder/clickhouse-client_*.deb; \ dpkg -i package_folder/clickhouse-test_*.deb + +mkdir -p /var/lib/clickhouse +mkdir -p /var/log/clickhouse-server +chmod 777 -R /var/lib/clickhouse +chmod 777 -R /var/log/clickhouse-server/ + ln -s /usr/share/clickhouse-test/config/zookeeper.xml /etc/clickhouse-server/config.d/; \ ln -s /usr/share/clickhouse-test/config/listen.xml /etc/clickhouse-server/config.d/; \ ln -s /usr/share/clickhouse-test/config/part_log.xml /etc/clickhouse-server/config.d/; \ From 3779a5cba201e3bdebe558f4512463652fd83b32 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Mon, 8 Jul 2019 19:01:00 +0300 Subject: [PATCH 38/62] Addition to prev. revision --- contrib/simdjson-cmake/CMakeLists.txt | 2 +- dbms/src/Functions/SimdJSONParser.h | 14 ++------------ 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/contrib/simdjson-cmake/CMakeLists.txt b/contrib/simdjson-cmake/CMakeLists.txt index 16a5dc1a791..9187f2ec940 100644 --- a/contrib/simdjson-cmake/CMakeLists.txt +++ b/contrib/simdjson-cmake/CMakeLists.txt @@ -14,5 +14,5 @@ set(SIMDJSON_SRC ) add_library(${SIMDJSON_LIBRARY} ${SIMDJSON_SRC}) -target_include_directories(${SIMDJSON_LIBRARY} PUBLIC "${SIMDJSON_INCLUDE_DIR}") +target_include_directories(${SIMDJSON_LIBRARY} SYSTEM PUBLIC "${SIMDJSON_INCLUDE_DIR}") target_compile_options(${SIMDJSON_LIBRARY} PRIVATE -mavx2 -mbmi -mbmi2 -mpclmul) diff --git a/dbms/src/Functions/SimdJSONParser.h b/dbms/src/Functions/SimdJSONParser.h index ef43e571635..dfa28ccfd56 100644 --- a/dbms/src/Functions/SimdJSONParser.h +++ b/dbms/src/Functions/SimdJSONParser.h @@ -7,18 +7,8 @@ #include #include -#ifdef __clang__ - #pragma clang diagnostic push - #pragma clang diagnostic ignored "-Wold-style-cast" - #pragma clang diagnostic ignored "-Wnewline-eof" -#endif - #include -#ifdef __clang__ - #pragma clang diagnostic pop -#endif - namespace DB { @@ -42,7 +32,7 @@ struct SimdJSONParser bool parse(const StringRef & json) { return !json_parse(json.data, json.size, pj); } - using Iterator = ParsedJson::iterator; + using Iterator = simdjson::ParsedJson::iterator; Iterator getRoot() { return Iterator{pj}; } static bool isInt64(const Iterator & it) { return it.is_integer(); } @@ -143,7 +133,7 @@ struct SimdJSONParser } private: - ParsedJson pj; + simdjson::ParsedJson pj; }; } From 27663d9a86513119079c268c905d5395d5d197b0 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Mon, 8 Jul 2019 19:42:49 +0300 Subject: [PATCH 39/62] Addition to prev. revision --- contrib/simdjson-cmake/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/contrib/simdjson-cmake/CMakeLists.txt b/contrib/simdjson-cmake/CMakeLists.txt index 9187f2ec940..bbb2d8e389f 100644 --- a/contrib/simdjson-cmake/CMakeLists.txt +++ b/contrib/simdjson-cmake/CMakeLists.txt @@ -11,6 +11,7 @@ set(SIMDJSON_SRC ${SIMDJSON_SRC_DIR}/stage2_build_tape.cpp ${SIMDJSON_SRC_DIR}/parsedjson.cpp ${SIMDJSON_SRC_DIR}/parsedjsoniterator.cpp + ${SIMDJSON_SRC_DIR}/simdjson.cpp ) add_library(${SIMDJSON_LIBRARY} ${SIMDJSON_SRC}) From 6512f923acc8a73fc57d8a005a50ee61fd02f642 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Mon, 8 Jul 2019 19:47:40 +0300 Subject: [PATCH 40/62] Fixed error with unit test --- dbms/src/Columns/tests/CMakeLists.txt | 4 ---- .../tests/{column_unique.cpp => gtest_column_unique.cpp} | 0 2 files changed, 4 deletions(-) rename dbms/src/Columns/tests/{column_unique.cpp => gtest_column_unique.cpp} (100%) diff --git a/dbms/src/Columns/tests/CMakeLists.txt b/dbms/src/Columns/tests/CMakeLists.txt index 302c554a1fd..e69de29bb2d 100644 --- a/dbms/src/Columns/tests/CMakeLists.txt +++ b/dbms/src/Columns/tests/CMakeLists.txt @@ -1,4 +0,0 @@ -if(USE_GTEST) - add_executable(column_unique column_unique.cpp) - target_link_libraries(column_unique PRIVATE dbms ${GTEST_BOTH_LIBRARIES}) -endif() \ No newline at end of file diff --git a/dbms/src/Columns/tests/column_unique.cpp b/dbms/src/Columns/tests/gtest_column_unique.cpp similarity index 100% rename from dbms/src/Columns/tests/column_unique.cpp rename to dbms/src/Columns/tests/gtest_column_unique.cpp From a5cce21ebda74f429d0230dc667903826c2a5d83 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Mon, 8 Jul 2019 19:53:49 +0300 Subject: [PATCH 41/62] Addition to prev. revision --- dbms/src/Common/CpuId.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/dbms/src/Common/CpuId.h b/dbms/src/Common/CpuId.h index 1bae7407818..808502ba086 100644 --- a/dbms/src/Common/CpuId.h +++ b/dbms/src/Common/CpuId.h @@ -14,8 +14,9 @@ namespace DB namespace Cpu { -#if defined(__x86_64__) || defined(__i386__) -inline UInt64 _xgetbv(UInt32 xcr) noexcept +#if (defined(__x86_64__) || defined(__i386__)) +/// Our version is independent of -mxsave option, because we do dynamic dispatch. +inline UInt64 our_xgetbv(UInt32 xcr) noexcept { UInt32 eax; UInt32 edx; @@ -185,7 +186,7 @@ bool haveAVX() noexcept // http://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-optimization-manual.pdf // https://bugs.chromium.org/p/chromium/issues/detail?id=375968 return haveOSXSAVE() // implies haveXSAVE() - && (_xgetbv(0) & 6u) == 6u // XMM state and YMM state are enabled by OS + && (our_xgetbv(0) & 6u) == 6u // XMM state and YMM state are enabled by OS && ((CpuInfo(0x1).ecx >> 28) & 1u); // AVX bit #else return false; @@ -217,8 +218,8 @@ bool haveAVX512F() noexcept #if defined(__x86_64__) || defined(__i386__) // https://software.intel.com/en-us/articles/how-to-detect-knl-instruction-support return haveOSXSAVE() // implies haveXSAVE() - && (_xgetbv(0) & 6u) == 6u // XMM state and YMM state are enabled by OS - && ((_xgetbv(0) >> 5) & 7u) == 7u // ZMM state is enabled by OS + && (our_xgetbv(0) & 6u) == 6u // XMM state and YMM state are enabled by OS + && ((our_xgetbv(0) >> 5) & 7u) == 7u // ZMM state is enabled by OS && CpuInfo(0x0).eax >= 0x7 // leaf 7 is present && ((CpuInfo(0x7).ebx >> 16) & 1u); // AVX512F bit #else From c1bb550c071ed2749b1d198529f338ee0d93ae34 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Mon, 8 Jul 2019 19:58:32 +0300 Subject: [PATCH 42/62] Addition to prev. revision --- dbms/programs/server/HTTPHandler.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/dbms/programs/server/HTTPHandler.cpp b/dbms/programs/server/HTTPHandler.cpp index a7a195950cc..18ca9b84518 100644 --- a/dbms/programs/server/HTTPHandler.cpp +++ b/dbms/programs/server/HTTPHandler.cpp @@ -211,6 +211,7 @@ void HTTPHandler::processQuery( Output & used_output) { Context context = server.context(); + context.makeGlobalContext(); CurrentThread::QueryScope query_scope(context); From 26f91c4a9610bc3cb80ee39d9174465a53b6d8e2 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Mon, 8 Jul 2019 20:05:38 +0300 Subject: [PATCH 43/62] Added a test --- .../0_stateless/00966_invalid_json_must_not_parse.reference | 1 + .../queries/0_stateless/00966_invalid_json_must_not_parse.sql | 1 + 2 files changed, 2 insertions(+) create mode 100644 dbms/tests/queries/0_stateless/00966_invalid_json_must_not_parse.reference create mode 100644 dbms/tests/queries/0_stateless/00966_invalid_json_must_not_parse.sql diff --git a/dbms/tests/queries/0_stateless/00966_invalid_json_must_not_parse.reference b/dbms/tests/queries/0_stateless/00966_invalid_json_must_not_parse.reference new file mode 100644 index 00000000000..573541ac970 --- /dev/null +++ b/dbms/tests/queries/0_stateless/00966_invalid_json_must_not_parse.reference @@ -0,0 +1 @@ +0 diff --git a/dbms/tests/queries/0_stateless/00966_invalid_json_must_not_parse.sql b/dbms/tests/queries/0_stateless/00966_invalid_json_must_not_parse.sql new file mode 100644 index 00000000000..ee6243d250f --- /dev/null +++ b/dbms/tests/queries/0_stateless/00966_invalid_json_must_not_parse.sql @@ -0,0 +1 @@ +SELECT JSONLength(unhex('5B30000E06D7AA5D')); From dcfb332fdf10b49c957f81e36d8780ae3672a1a0 Mon Sep 17 00:00:00 2001 From: alesapin Date: Mon, 8 Jul 2019 20:23:10 +0300 Subject: [PATCH 44/62] Merge client files --- docker/test/stateful_with_coverage/Dockerfile | 5 +- docker/test/stateful_with_coverage/run.sh | 50 +++++++++++++++---- .../test/stateless_with_coverage/Dockerfile | 5 +- docker/test/stateless_with_coverage/run.sh | 32 ++++++++++-- 4 files changed, 76 insertions(+), 16 deletions(-) diff --git a/docker/test/stateful_with_coverage/Dockerfile b/docker/test/stateful_with_coverage/Dockerfile index 2f7764db2dc..2a566bdcf01 100644 --- a/docker/test/stateful_with_coverage/Dockerfile +++ b/docker/test/stateful_with_coverage/Dockerfile @@ -1,11 +1,14 @@ # docker build -t yandex/clickhouse-stateful-test . FROM yandex/clickhouse-stateless-test +RUN echo "deb [trusted=yes] http://apt.llvm.org/bionic/ llvm-toolchain-bionic main" >> /etc/apt/sources.list + RUN apt-get update -y \ && env DEBIAN_FRONTEND=noninteractive \ apt-get install --yes --no-install-recommends \ python-requests \ - llvm-8 + llvm-8 \ + llvm-9 COPY s3downloader /s3downloader COPY run.sh /run.sh diff --git a/docker/test/stateful_with_coverage/run.sh b/docker/test/stateful_with_coverage/run.sh index 573fb3e4d8b..6ec0bfa155a 100755 --- a/docker/test/stateful_with_coverage/run.sh +++ b/docker/test/stateful_with_coverage/run.sh @@ -1,7 +1,5 @@ #!/bin/bash -set -x - kill_clickhouse () { while kill -0 `pgrep -u clickhouse`; do @@ -15,6 +13,23 @@ start_clickhouse () { LLVM_PROFILE_FILE='server_%h_%p_%m.profraw' sudo -Eu clickhouse /usr/bin/clickhouse-server --config /etc/clickhouse-server/config.xml & } +wait_llvm_profdata () { + while kill -0 `pgrep llvm-profdata-9`; + do + echo "Waiting for profdata " `pgrep llvm-profdata-9` "still alive" + sleep 3 + done +} + +merge_client_files_in_background () { + client_files=`ls /client_*profraw 2>/dev/null` + if [ ! -z "$client_files" ] + then + llvm-profdata-9 merge -sparse $client_files -o merged_client_`date +%s`.profraw + rm $client_files + fi +} + chmod 777 / dpkg -i package_folder/clickhouse-common-static_*.deb; \ @@ -37,6 +52,7 @@ ln -s /usr/share/clickhouse-test/config/zookeeper.xml /etc/clickhouse-server/con ln -s /usr/share/clickhouse-test/config/decimals_dictionary.xml /etc/clickhouse-server/; \ ln -s /usr/lib/llvm-8/bin/llvm-symbolizer /usr/bin/llvm-symbolizer + service zookeeper start sleep 5 @@ -49,22 +65,34 @@ sleep 5 chmod 777 -R /var/lib/clickhouse -LLVM_PROFILE_FILE='client.profraw' clickhouse-client --query "SHOW DATABASES" -LLVM_PROFILE_FILE='client.profraw' clickhouse-client --query "CREATE DATABASE datasets" -LLVM_PROFILE_FILE='client.profraw' clickhouse-client --query "CREATE DATABASE test" +while /bin/true; do + merge_client_files_in_background + sleep 2 +done & + +LLVM_PROFILE_FILE='client_%h_%p_%m.profraw' clickhouse-client --query "SHOW DATABASES" +LLVM_PROFILE_FILE='client_%h_%p_%m.profraw' clickhouse-client --query "CREATE DATABASE datasets" +LLVM_PROFILE_FILE='client_%h_%p_%m.profraw' clickhouse-client --query "CREATE DATABASE test" kill_clickhouse start_clickhouse sleep 10 -LLVM_PROFILE_FILE='client.profraw' clickhouse-client --query "SHOW TABLES FROM datasets" -LLVM_PROFILE_FILE='client.profraw' clickhouse-client --query "SHOW TABLES FROM test" -LLVM_PROFILE_FILE='client.profraw' clickhouse-client --query "RENAME TABLE datasets.hits_v1 TO test.hits" -LLVM_PROFILE_FILE='client.profraw' clickhouse-client --query "RENAME TABLE datasets.visits_v1 TO test.visits" -LLVM_PROFILE_FILE='client.profraw' clickhouse-client --query "SHOW TABLES FROM test" -LLVM_PROFILE_FILE='client.profraw' clickhouse-test --shard --zookeeper --no-stateless $SKIP_TESTS_OPTION 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt +LLVM_PROFILE_FILE='client_%h_%p_%m.profraw' clickhouse-client --query "SHOW TABLES FROM datasets" +LLVM_PROFILE_FILE='client_%h_%p_%m.profraw' clickhouse-client --query "SHOW TABLES FROM test" +LLVM_PROFILE_FILE='client_%h_%p_%m.profraw' clickhouse-client --query "RENAME TABLE datasets.hits_v1 TO test.hits" +LLVM_PROFILE_FILE='client_%h_%p_%m.profraw' clickhouse-client --query "RENAME TABLE datasets.visits_v1 TO test.visits" +LLVM_PROFILE_FILE='client_%h_%p_%m.profraw' clickhouse-client --query "SHOW TABLES FROM test" +LLVM_PROFILE_FILE='client_%h_%p_%m.profraw' clickhouse-test --shard --zookeeper --no-stateless $SKIP_TESTS_OPTION 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt kill_clickhouse +wait_llvm_profdata + +sleep 3 + +wait_llvm_profdata # 100% merged all parts + + cp /*.profraw /profraw ||: diff --git a/docker/test/stateless_with_coverage/Dockerfile b/docker/test/stateless_with_coverage/Dockerfile index e0e0463f1b6..b9da18223ab 100644 --- a/docker/test/stateless_with_coverage/Dockerfile +++ b/docker/test/stateless_with_coverage/Dockerfile @@ -1,6 +1,8 @@ # docker build -t yandex/clickhouse-stateless-with-coverage-test . FROM yandex/clickhouse-deb-builder +RUN echo "deb [trusted=yes] http://apt.llvm.org/bionic/ llvm-toolchain-bionic main" >> /etc/apt/sources.list + RUN apt-get update -y \ && env DEBIAN_FRONTEND=noninteractive \ apt-get install --yes --no-install-recommends \ @@ -23,7 +25,8 @@ RUN apt-get update -y \ moreutils \ brotli \ gdb \ - lsof + lsof \ + llvm-9 ENV TZ=Europe/Moscow diff --git a/docker/test/stateless_with_coverage/run.sh b/docker/test/stateless_with_coverage/run.sh index 9dc912d6d6c..50082337757 100755 --- a/docker/test/stateless_with_coverage/run.sh +++ b/docker/test/stateless_with_coverage/run.sh @@ -1,7 +1,5 @@ #!/bin/bash -set -x - kill_clickhouse () { while kill -0 `pgrep -u clickhouse`; do @@ -11,10 +9,27 @@ kill_clickhouse () { done } +wait_llvm_profdata () { + while kill -0 `pgrep llvm-profdata-9`; + do + echo "Waiting for profdata " `pgrep llvm-profdata-9` "still alive" + sleep 3 + done +} + start_clickhouse () { LLVM_PROFILE_FILE='server_%h_%p_%m.profraw' sudo -Eu clickhouse /usr/bin/clickhouse-server --config /etc/clickhouse-server/config.xml & } +merge_client_files_in_background () { + client_files=`ls /client_*profraw 2>/dev/null` + if [ ! -z "$client_files" ] + then + llvm-profdata-9 merge -sparse $client_files -o merged_client_`date +%s`.profraw + rm $client_files + fi +} + chmod 777 / dpkg -i package_folder/clickhouse-common-static_*.deb; \ @@ -46,8 +61,19 @@ start_clickhouse sleep 10 -LLVM_PROFILE_FILE='client.profraw' clickhouse-test --shard --zookeeper $ADDITIONAL_OPTIONS $SKIP_TESTS_OPTION 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt +while /bin/true; do + merge_client_files_in_background + sleep 2 +done & + +LLVM_PROFILE_FILE='client_%h_%p_%m.profraw' clickhouse-test --shard --zookeeper $ADDITIONAL_OPTIONS $SKIP_TESTS_OPTION 2>&1 | ts '%Y-%m-%d %H:%M:%S' | tee test_output/test_result.txt kill_clickhouse +wait_llvm_profdata + +sleep 3 + +wait_llvm_profdata # 100% merged all parts + cp /*.profraw /profraw ||: From e07235e2945657ef17f1f28a59ae8f41ca990012 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Mon, 8 Jul 2019 21:55:06 +0300 Subject: [PATCH 45/62] Addition to prev. revision --- contrib/CMakeLists.txt | 1 - dbms/CMakeLists.txt | 2 +- dbms/src/IO/tests/read_buffer_aio.cpp | 2 +- dbms/src/IO/tests/write_buffer_aio.cpp | 2 +- dbms/src/Interpreters/tests/hash_map_string.cpp | 2 +- 5 files changed, 4 insertions(+), 5 deletions(-) diff --git a/contrib/CMakeLists.txt b/contrib/CMakeLists.txt index ba75615aadc..bb4e9d6e520 100644 --- a/contrib/CMakeLists.txt +++ b/contrib/CMakeLists.txt @@ -251,7 +251,6 @@ if(USE_INTERNAL_GTEST_LIBRARY) add_subdirectory(${ClickHouse_SOURCE_DIR}/contrib/googletest/googletest ${CMAKE_CURRENT_BINARY_DIR}/googletest) # avoid problems with target_compile_definitions (gtest INTERFACE GTEST_HAS_POSIX_RE=0) - target_include_directories (gtest SYSTEM INTERFACE ${ClickHouse_SOURCE_DIR}/contrib/googletest/include) elseif(GTEST_SRC_DIR) add_subdirectory(${GTEST_SRC_DIR}/googletest ${CMAKE_CURRENT_BINARY_DIR}/googletest) target_compile_definitions(gtest INTERFACE GTEST_HAS_POSIX_RE=0) diff --git a/dbms/CMakeLists.txt b/dbms/CMakeLists.txt index 60a3cda0556..bcbe435569f 100644 --- a/dbms/CMakeLists.txt +++ b/dbms/CMakeLists.txt @@ -389,7 +389,7 @@ if (ENABLE_TESTS AND USE_GTEST) add_executable(unit_tests_dbms ${dbms_gtest_sources}) # gtest framework has substandard code - target_compile_options(unit_tests_dbms PRIVATE -Wno-zero-as-null-pointer-constant -Wno-undef -Wno-sign-compare -Wno-used-but-marked-unused) + target_compile_options(unit_tests_dbms PRIVATE -Wno-zero-as-null-pointer-constant -Wno-undef -Wno-sign-compare -Wno-used-but-marked-unused -Wno-missing-noreturn) target_link_libraries(unit_tests_dbms PRIVATE ${GTEST_BOTH_LIBRARIES} clickhouse_functions clickhouse_parsers dbms clickhouse_common_zookeeper) add_check(unit_tests_dbms) diff --git a/dbms/src/IO/tests/read_buffer_aio.cpp b/dbms/src/IO/tests/read_buffer_aio.cpp index 7ed07b0930c..81d04da79f2 100644 --- a/dbms/src/IO/tests/read_buffer_aio.cpp +++ b/dbms/src/IO/tests/read_buffer_aio.cpp @@ -18,7 +18,7 @@ void prepare2(std::string & filename, std::string & buf); void prepare3(std::string & filename, std::string & buf); void prepare4(std::string & filename, std::string & buf); std::string createTmpFile(); -void die(const std::string & msg); +[[noreturn]] void die(const std::string & msg); void runTest(unsigned int num, const std::function & func); bool test1(const std::string & filename); diff --git a/dbms/src/IO/tests/write_buffer_aio.cpp b/dbms/src/IO/tests/write_buffer_aio.cpp index df8dde6b190..4e76a91639c 100644 --- a/dbms/src/IO/tests/write_buffer_aio.cpp +++ b/dbms/src/IO/tests/write_buffer_aio.cpp @@ -14,7 +14,7 @@ namespace namespace fs = boost::filesystem; void run(); -void die(const std::string & msg); +[[noreturn]] void die(const std::string & msg); void runTest(unsigned int num, const std::function & func); std::string createTmpFile(); std::string generateString(size_t n); diff --git a/dbms/src/Interpreters/tests/hash_map_string.cpp b/dbms/src/Interpreters/tests/hash_map_string.cpp index 9076a1e582e..cad701b49ca 100644 --- a/dbms/src/Interpreters/tests/hash_map_string.cpp +++ b/dbms/src/Interpreters/tests/hash_map_string.cpp @@ -277,7 +277,7 @@ struct Grower : public HashTableGrower<> } /// Set the buffer size by the number of elements in the hash table. Used when deserializing a hash table. - void set(size_t /*num_elems*/) + [[noreturn]] void set(size_t /*num_elems*/) { throw Poco::Exception(__PRETTY_FUNCTION__); } From 7736bb012cbdcdce4a97cb9428a9c21589a2500c Mon Sep 17 00:00:00 2001 From: alexey-milovidov Date: Mon, 8 Jul 2019 22:23:26 +0300 Subject: [PATCH 46/62] Update CHANGELOG.md --- CHANGELOG.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 61a7f3ae39b..ae3703d788d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,14 +1,14 @@ ## ClickHouse release 19.9.4.1, 2019-07-05 ### Bug Fix -* Fix potential infinite sleeping of low-priority queries. [#5842](https://github.com/yandex/ClickHouse/pull/5842) ([alexey-milovidov](https://github.com/alexey-milovidov)) -* Fix rare bug in checking of part with LowCardinality column. [#5832](https://github.com/yandex/ClickHouse/pull/5832) ([alesapin](https://github.com/alesapin)) -* Fix ClickHouse determines default time zone as UCT instead of UTC. [#5828](https://github.com/yandex/ClickHouse/pull/5828) ([alexey-milovidov](https://github.com/alexey-milovidov)) -* Fix bug about executing distributed DROP/ALTER/TRUNCATE/OPTIMIZE ON CLUSTER queries on follower replica before leader replica. Now they will be executed directly on leader replica. [#5757](https://github.com/yandex/ClickHouse/pull/5757) ([alesapin](https://github.com/alesapin)) * Fix segfault in Delta codec which affects columns with values less than 32 bits size. The bug led to random memory corruption. [#5786](https://github.com/yandex/ClickHouse/pull/5786) ([alesapin](https://github.com/alesapin)) -* Fix race condition, which cause that some queries may not appear in query_log after SYSTEM FLUSH LOGS query. [#5685](https://github.com/yandex/ClickHouse/pull/5685) ([Anton Popov](https://github.com/CurtizJ)) +* Fix rare bug in checking of part with LowCardinality column. [#5832](https://github.com/yandex/ClickHouse/pull/5832) ([alesapin](https://github.com/alesapin)) * Fix segfault in TTL merge with non-physical columns in block. [#5819](https://github.com/yandex/ClickHouse/pull/5819) ([Anton Popov](https://github.com/CurtizJ)) -* Added missing support for constant arguments to evalMLModel function. [#5820](https://github.com/yandex/ClickHouse/pull/5820) ([alexey-milovidov](https://github.com/alexey-milovidov)) +* Fix potential infinite sleeping of low-priority queries. [#5842](https://github.com/yandex/ClickHouse/pull/5842) ([alexey-milovidov](https://github.com/alexey-milovidov)) +* Fix how ClickHouse determines default time zone as UCT instead of UTC. [#5828](https://github.com/yandex/ClickHouse/pull/5828) ([alexey-milovidov](https://github.com/alexey-milovidov)) +* Fix bug about executing distributed DROP/ALTER/TRUNCATE/OPTIMIZE ON CLUSTER queries on follower replica before leader replica. Now they will be executed directly on leader replica. [#5757](https://github.com/yandex/ClickHouse/pull/5757) ([alesapin](https://github.com/alesapin)) +* Fix race condition, which cause that some queries may not appear in query_log instantly after SYSTEM FLUSH LOGS query. [#5685](https://github.com/yandex/ClickHouse/pull/5685) ([Anton Popov](https://github.com/CurtizJ)) +* Added missing support for constant arguments to `evalMLModel` function. [#5820](https://github.com/yandex/ClickHouse/pull/5820) ([alexey-milovidov](https://github.com/alexey-milovidov)) ## ClickHouse release 19.7.6.1, 2019-07-05 From aa21e0108b2cc91f2f32339753d58839fa46968a Mon Sep 17 00:00:00 2001 From: alexey-milovidov Date: Mon, 8 Jul 2019 22:25:01 +0300 Subject: [PATCH 47/62] Update CHANGELOG_RU.md --- CHANGELOG_RU.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/CHANGELOG_RU.md b/CHANGELOG_RU.md index 1c03f95dc17..2fa1f34ce04 100644 --- a/CHANGELOG_RU.md +++ b/CHANGELOG_RU.md @@ -1,14 +1,14 @@ ## ClickHouse release 19.9.4.1, 2019-07-05 ### Исправления ошибок +* Исправлен segmentation fault в кодеке сжатия Delta в колонках с величинами размером меньше 32 бит. Ошибка могла приводить к повреждениям памяти. [#5786](https://github.com/yandex/ClickHouse/pull/5786) ([alesapin](https://github.com/alesapin)) +* Исправлена ошибка в проверке кусков в LowCardinality колонках. [#5832](https://github.com/yandex/ClickHouse/pull/5832) ([alesapin](https://github.com/alesapin)) +* Исправлен segmentation fault при слиянии кусков с истекшим TTL в случае, когда в блоке присутствуют столбцы, не входящие в структуру таблицы. [#5819](https://github.com/yandex/ClickHouse/pull/5819) ([Anton Popov](https://github.com/CurtizJ)) * Исправлена существовавшая возможность ухода в бесконечное ожидание на низко-приоритетных запросах. [#5842](https://github.com/yandex/ClickHouse/pull/5842) ([alexey-milovidov](https://github.com/alexey-milovidov)) -* Исправлена ошибка в проверке частей в LowCardinality колонках. [#5832](https://github.com/yandex/ClickHouse/pull/5832) ([alesapin](https://github.com/alesapin)) -* Исправлена ошибка определения формата таймзоны по умолчанию (UCT вместо UTC). [#5828](https://github.com/yandex/ClickHouse/pull/5828) ([alexey-milovidov](https://github.com/alexey-milovidov)) +* Исправлена ошибка определения таймзоны по умолчанию (UCT вместо UTC). [#5828](https://github.com/yandex/ClickHouse/pull/5828) ([alexey-milovidov](https://github.com/alexey-milovidov)) * Исправлена ошибка в распределенных запросах вида DROP/ALTER/TRUNCATE/OPTIMIZE ON CLUSTER. [#5757](https://github.com/yandex/ClickHouse/pull/5757) ([alesapin](https://github.com/alesapin)) -* Исправлена ошибка сегментации в кодеке сжатия Delta в колонках с величинами размером меньше 32 бит. Ошибка могла приводить к повреждениям памяти. [#5786](https://github.com/yandex/ClickHouse/pull/5786) ([alesapin](https://github.com/alesapin)) -* Исправлена ошибка, которая при распределенных запросах могла привести к тому, что некоторые запросы не появлялись в query_log после SYSTEM FLUSH LOGS запроса. [#5685](https://github.com/yandex/ClickHouse/pull/5685) ([Anton Popov](https://github.com/CurtizJ)) -* Исправлена ошибка сегментации при слиянии кусков с истекшим TTL в случае, когда в блоке присутствуют столбцы, не входящие в структуру таблицы. [#5819](https://github.com/yandex/ClickHouse/pull/5819) ([Anton Popov](https://github.com/CurtizJ)) -* Добавлена отсутствовавшая поддержка константных аргументов для функции evalMLModel. [#5820](https://github.com/yandex/ClickHouse/pull/5820) ([alexey-milovidov](https://github.com/alexey-milovidov)) +* Исправлена ошибка, которая при распределенных запросах могла привести к тому, что некоторые запросы не появлялись в query_log сразу после SYSTEM FLUSH LOGS запроса. [#5685](https://github.com/yandex/ClickHouse/pull/5685) ([Anton Popov](https://github.com/CurtizJ)) +* Добавлена отсутствовавшая поддержка константных аргументов для функции `evalMLModel`. [#5820](https://github.com/yandex/ClickHouse/pull/5820) ([alexey-milovidov](https://github.com/alexey-milovidov)) ## ClickHouse release 19.7.6.1, 2019-07-05 From 6515445a05e081ca8f62a092ee4d1dc07e4b46c9 Mon Sep 17 00:00:00 2001 From: alexey-milovidov Date: Mon, 8 Jul 2019 22:25:38 +0300 Subject: [PATCH 48/62] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ae3703d788d..caf1f4fd7f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ ## ClickHouse release 19.7.6.1, 2019-07-05 ### Bug Fix -* Fix push require columns with join. [#5192](https://github.com/yandex/ClickHouse/pull/5192) ([Winter Zhang](https://github.com/zhang2014)) +* Fix performance regression in some queries with JOIN. [#5192](https://github.com/yandex/ClickHouse/pull/5192) ([Winter Zhang](https://github.com/zhang2014)) ## ClickHouse release 19.9.2.4, 2019-06-24 From 02eada5b35b4a52a2210792621b65954ac69deeb Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Mon, 8 Jul 2019 22:41:11 +0300 Subject: [PATCH 49/62] Fixed error in prev. revision --- dbms/programs/server/HTTPHandler.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dbms/programs/server/HTTPHandler.cpp b/dbms/programs/server/HTTPHandler.cpp index 18ca9b84518..5db7ce1a3ae 100644 --- a/dbms/programs/server/HTTPHandler.cpp +++ b/dbms/programs/server/HTTPHandler.cpp @@ -211,7 +211,6 @@ void HTTPHandler::processQuery( Output & used_output) { Context context = server.context(); - context.makeGlobalContext(); CurrentThread::QueryScope query_scope(context); @@ -285,7 +284,7 @@ void HTTPHandler::processQuery( session = context.acquireSession(session_id, session_timeout, session_check == "1"); context = *session; - context.makeSessionContext(); + context.setSessionContext(*session); } SCOPE_EXIT({ From a1e7d12f683c9784e368a3ff325210a24528ee1d Mon Sep 17 00:00:00 2001 From: alexey-milovidov Date: Mon, 8 Jul 2019 22:55:36 +0300 Subject: [PATCH 50/62] Update ParserQueryWithOutput.cpp --- dbms/src/Parsers/ParserQueryWithOutput.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/dbms/src/Parsers/ParserQueryWithOutput.cpp b/dbms/src/Parsers/ParserQueryWithOutput.cpp index 23c89ae20f9..c41e0946a96 100644 --- a/dbms/src/Parsers/ParserQueryWithOutput.cpp +++ b/dbms/src/Parsers/ParserQueryWithOutput.cpp @@ -15,7 +15,6 @@ #include - namespace DB { From 6088aa233f1b0569820d580a8cc92598ea70efc9 Mon Sep 17 00:00:00 2001 From: alexey-milovidov Date: Mon, 8 Jul 2019 22:55:54 +0300 Subject: [PATCH 51/62] Update ASTCheckQuery.h --- dbms/src/Parsers/ASTCheckQuery.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/Parsers/ASTCheckQuery.h b/dbms/src/Parsers/ASTCheckQuery.h index 89b9035ae9c..40665f6f2b6 100644 --- a/dbms/src/Parsers/ASTCheckQuery.h +++ b/dbms/src/Parsers/ASTCheckQuery.h @@ -2,7 +2,7 @@ #include #include -#include + namespace DB { From c491cddb789e13226524b88f5f28c7b6b70e2b3c Mon Sep 17 00:00:00 2001 From: alexey-milovidov Date: Mon, 8 Jul 2019 22:56:48 +0300 Subject: [PATCH 52/62] Update CheckResults.h --- dbms/src/Storages/CheckResults.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/Storages/CheckResults.h b/dbms/src/Storages/CheckResults.h index 53f1e7c0ca2..0f895fba3bc 100644 --- a/dbms/src/Storages/CheckResults.h +++ b/dbms/src/Storages/CheckResults.h @@ -12,7 +12,7 @@ struct CheckResult /// Part name for merge tree or file name for simplier tables String fs_path; /// Does check passed - bool success; + bool success = false; /// Failure message if any String failure_message; From ef41b16a449c8a81edcd50b7380a5e86e076dbcf Mon Sep 17 00:00:00 2001 From: chertus Date: Mon, 8 Jul 2019 23:06:17 +0300 Subject: [PATCH 53/62] fix wrong ExpressionAnalyzer.array_join_columns calculation --- dbms/src/Interpreters/ExpressionAnalyzer.cpp | 6 +++++- .../0_stateless/00876_wrong_arraj_join_column.reference | 0 .../0_stateless/00876_wrong_arraj_join_column.sql | 9 +++++++++ 3 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 dbms/tests/queries/0_stateless/00876_wrong_arraj_join_column.reference create mode 100644 dbms/tests/queries/0_stateless/00876_wrong_arraj_join_column.sql diff --git a/dbms/src/Interpreters/ExpressionAnalyzer.cpp b/dbms/src/Interpreters/ExpressionAnalyzer.cpp index a1345cd7ba9..f0afe55379f 100644 --- a/dbms/src/Interpreters/ExpressionAnalyzer.cpp +++ b/dbms/src/Interpreters/ExpressionAnalyzer.cpp @@ -144,7 +144,11 @@ void ExpressionAnalyzer::analyzeAggregation() { getRootActions(array_join_expression_list, true, temp_actions); addMultipleArrayJoinAction(temp_actions, is_array_join_left); - array_join_columns = temp_actions->getSampleBlock().getNamesAndTypesList(); + + array_join_columns.clear(); + for (auto & column : temp_actions->getSampleBlock().getNamesAndTypesList()) + if (syntax->array_join_result_to_source.count(column.name)) + array_join_columns.emplace_back(column); } const ASTTablesInSelectQueryElement * join = select_query->join(); diff --git a/dbms/tests/queries/0_stateless/00876_wrong_arraj_join_column.reference b/dbms/tests/queries/0_stateless/00876_wrong_arraj_join_column.reference new file mode 100644 index 00000000000..e69de29bb2d diff --git a/dbms/tests/queries/0_stateless/00876_wrong_arraj_join_column.sql b/dbms/tests/queries/0_stateless/00876_wrong_arraj_join_column.sql new file mode 100644 index 00000000000..0e72f9a67ce --- /dev/null +++ b/dbms/tests/queries/0_stateless/00876_wrong_arraj_join_column.sql @@ -0,0 +1,9 @@ +DROP TABLE IF EXISTS visits; +CREATE TABLE visits (str String) ENGINE = MergeTree ORDER BY (str); + +SELECT 1 +FROM visits +ARRAY JOIN arrayFilter(t -> 1, arrayMap(x -> tuple(x), [42])) AS i +WHERE ((str, i.1) IN ('x', 0)); + +DROP TABLE visits; From f748efbb5b234d8dfe9c587edf8d79d606f2aba5 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Tue, 9 Jul 2019 01:30:30 +0300 Subject: [PATCH 54/62] Fixed ubsan report in fuzz test --- dbms/src/Functions/bitTest.cpp | 4 +++- dbms/tests/queries/0_stateless/00967_ubsan_bit_test.reference | 1 + dbms/tests/queries/0_stateless/00967_ubsan_bit_test.sql | 1 + 3 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 dbms/tests/queries/0_stateless/00967_ubsan_bit_test.reference create mode 100644 dbms/tests/queries/0_stateless/00967_ubsan_bit_test.sql diff --git a/dbms/src/Functions/bitTest.cpp b/dbms/src/Functions/bitTest.cpp index 1818f7af479..ebb52a4dacf 100644 --- a/dbms/src/Functions/bitTest.cpp +++ b/dbms/src/Functions/bitTest.cpp @@ -1,5 +1,7 @@ #include #include +#include + namespace DB { @@ -10,7 +12,7 @@ struct BitTestImpl using ResultType = UInt8; template - static inline Result apply(A a, B b) + NO_SANITIZE_UNDEFINED static inline Result apply(A a, B b) { return (typename NumberTraits::ToInteger::Type(a) >> typename NumberTraits::ToInteger::Type(b)) & 1; } diff --git a/dbms/tests/queries/0_stateless/00967_ubsan_bit_test.reference b/dbms/tests/queries/0_stateless/00967_ubsan_bit_test.reference new file mode 100644 index 00000000000..573541ac970 --- /dev/null +++ b/dbms/tests/queries/0_stateless/00967_ubsan_bit_test.reference @@ -0,0 +1 @@ +0 diff --git a/dbms/tests/queries/0_stateless/00967_ubsan_bit_test.sql b/dbms/tests/queries/0_stateless/00967_ubsan_bit_test.sql new file mode 100644 index 00000000000..1682e725670 --- /dev/null +++ b/dbms/tests/queries/0_stateless/00967_ubsan_bit_test.sql @@ -0,0 +1 @@ +SELECT sum(ignore(bitTest(number, 65))) FROM numbers(10); From ea295347a20217188e91d6dd1919f76062944947 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Tue, 9 Jul 2019 03:08:04 +0300 Subject: [PATCH 55/62] Fixed race condition in build --- CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 2d3b37ce366..18b177e5a16 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -471,4 +471,5 @@ if (GLIBC_COMPATIBILITY OR USE_INTERNAL_UNWIND_LIBRARY_FOR_EXCEPTION_HANDLING) add_default_dependencies(brotli) add_default_dependencies(libprotobuf) add_default_dependencies(base64) + add_default_dependencies(readpassphrase) endif () From 9a17a461ea6a54068e492842ea2d0120e4cb4358 Mon Sep 17 00:00:00 2001 From: alesapin Date: Tue, 9 Jul 2019 12:02:52 +0300 Subject: [PATCH 56/62] Review fixes and better tests --- dbms/src/Storages/StorageMergeTree.cpp | 17 ++++++--- .../Storages/StorageReplicatedMergeTree.cpp | 4 +- .../integration/test_check_table/test.py | 14 ++++--- .../0_stateless/00961_check_table.reference | 11 ++++++ .../queries/0_stateless/00961_check_table.sql | 37 +++++++++++++++++++ 5 files changed, 69 insertions(+), 14 deletions(-) create mode 100644 dbms/tests/queries/0_stateless/00961_check_table.reference create mode 100644 dbms/tests/queries/0_stateless/00961_check_table.sql diff --git a/dbms/src/Storages/StorageMergeTree.cpp b/dbms/src/Storages/StorageMergeTree.cpp index 092573b604b..6b32a99c167 100644 --- a/dbms/src/Storages/StorageMergeTree.cpp +++ b/dbms/src/Storages/StorageMergeTree.cpp @@ -1141,19 +1141,24 @@ CheckResults StorageMergeTree::checkData(const ASTPtr & query, const Context & c String full_part_path = part->getFullPath(); /// If the checksums file is not present, calculate the checksums and write them to disk. String checksums_path = full_part_path + "checksums.txt"; + String tmp_checksums_path = full_part_path + "checksums.txt.tmp"; if (!Poco::File(checksums_path).exists()) { try { - auto counted_checksums = checkDataPart(part, false, primary_key_data_types, skip_indices); - counted_checksums.checkEqual(part->checksums, true); - WriteBufferFromFile out(full_part_path + "checksums.txt.tmp", 4096); + auto calculated_checksums = checkDataPart(part, false, primary_key_data_types, skip_indices); + calculated_checksums.checkEqual(part->checksums, true); + WriteBufferFromFile out(tmp_checksums_path, 4096); part->checksums.write(out); - Poco::File(full_part_path + "checksums.txt.tmp").renameTo(full_part_path + "checksums.txt"); + Poco::File(tmp_checksums_path).renameTo(checksums_path); results.emplace_back(part->name, true, "Checksums recounted and written to disk."); } - catch (Exception & ex) + catch (const Exception & ex) { + Poco::File tmp_file(tmp_checksums_path); + if (tmp_file.exists()) + tmp_file.remove(); + results.emplace_back(part->name, false, "Check of part finished with error: '" + ex.message() + "'"); } @@ -1165,7 +1170,7 @@ CheckResults StorageMergeTree::checkData(const ASTPtr & query, const Context & c checkDataPart(part, true, primary_key_data_types, skip_indices); results.emplace_back(part->name, true, ""); } - catch (Exception & ex) + catch (const Exception & ex) { results.emplace_back(part->name, false, ex.message()); } diff --git a/dbms/src/Storages/StorageReplicatedMergeTree.cpp b/dbms/src/Storages/StorageReplicatedMergeTree.cpp index 5fd42086e1d..b51da168192 100644 --- a/dbms/src/Storages/StorageReplicatedMergeTree.cpp +++ b/dbms/src/Storages/StorageReplicatedMergeTree.cpp @@ -5127,9 +5127,9 @@ CheckResults StorageReplicatedMergeTree::checkData(const ASTPtr & query, const C { results.push_back(part_check_thread.checkPart(part->name)); } - catch (Exception & ex) + catch (const Exception & ex) { - results.emplace_back(part->name, false, "Error during check:" + ex.message()); + results.emplace_back(part->name, false, "Check of part finished with error: '" + ex.message() + "'"); } } return results; diff --git a/dbms/tests/integration/test_check_table/test.py b/dbms/tests/integration/test_check_table/test.py index 195c421f854..6d53b423896 100644 --- a/dbms/tests/integration/test_check_table/test.py +++ b/dbms/tests/integration/test_check_table/test.py @@ -112,16 +112,18 @@ def test_check_replicated_table_corruption(started_cluster): assert node1.query("SELECT count() from replicated_mt") == "4\n" assert node2.query("SELECT count() from replicated_mt") == "4\n" - corrupt_data_part_on_disk(node1, "replicated_mt", "201901_0_0_0") - assert node1.query("CHECK TABLE replicated_mt PARTITION 201901") == "201901_0_0_0\t0\tPart 201901_0_0_0 looks broken. Removing it and queueing a fetch.\n" + part_name = node1.query("SELECT name from system.parts where table = 'replicated_mt' and partition_id = '201901' and active = 1").strip() + + corrupt_data_part_on_disk(node1, "replicated_mt", part_name) + assert node1.query("CHECK TABLE replicated_mt PARTITION 201901") == "{p}\t0\tPart {p} looks broken. Removing it and queueing a fetch.\n".format(p=part_name) node1.query("SYSTEM SYNC REPLICA replicated_mt") - assert node1.query("CHECK TABLE replicated_mt PARTITION 201901") == "201901_0_0_0\t1\t\n" + assert node1.query("CHECK TABLE replicated_mt PARTITION 201901") == "{}\t1\t\n".format(part_name) assert node1.query("SELECT count() from replicated_mt") == "4\n" - remove_part_from_disk(node2, "replicated_mt", "201901_0_0_0") - assert node2.query("CHECK TABLE replicated_mt PARTITION 201901") == "201901_0_0_0\t0\tPart 201901_0_0_0 looks broken. Removing it and queueing a fetch.\n" + remove_part_from_disk(node2, "replicated_mt", part_name) + assert node2.query("CHECK TABLE replicated_mt PARTITION 201901") == "{p}\t0\tPart {p} looks broken. Removing it and queueing a fetch.\n".format(p=part_name) node1.query("SYSTEM SYNC REPLICA replicated_mt") - assert node1.query("CHECK TABLE replicated_mt PARTITION 201901") == "201901_0_0_0\t1\t\n" + assert node1.query("CHECK TABLE replicated_mt PARTITION 201901") == "{}\t1\t\n".format(part_name) assert node1.query("SELECT count() from replicated_mt") == "4\n" diff --git a/dbms/tests/queries/0_stateless/00961_check_table.reference b/dbms/tests/queries/0_stateless/00961_check_table.reference new file mode 100644 index 00000000000..d85c66db622 --- /dev/null +++ b/dbms/tests/queries/0_stateless/00961_check_table.reference @@ -0,0 +1,11 @@ +201901_1_1_0 1 +======== +201901_1_1_0 1 +201901_2_2_0 1 +======== +201901_1_2_1 1 +======== +201901_1_2_1 1 +201902_3_3_0 1 +======== +201902_3_4_1 1 diff --git a/dbms/tests/queries/0_stateless/00961_check_table.sql b/dbms/tests/queries/0_stateless/00961_check_table.sql new file mode 100644 index 00000000000..9752ffc3974 --- /dev/null +++ b/dbms/tests/queries/0_stateless/00961_check_table.sql @@ -0,0 +1,37 @@ +DROP TABLE IF EXISTS mt_table; + +CREATE TABLE mt_table (d Date, key UInt64, data String) ENGINE = MergeTree() PARTITION BY toYYYYMM(d) ORDER BY key; + +CHECK TABLE mt_table; + +INSERT INTO mt_table VALUES (toDate('2019-01-02'), 1, 'Hello'), (toDate('2019-01-02'), 2, 'World'); + +CHECK TABLE mt_table; + +INSERT INTO mt_table VALUES (toDate('2019-01-02'), 3, 'quick'), (toDate('2019-01-02'), 4, 'brown'); + +SELECT '========'; + +CHECK TABLE mt_table; + +OPTIMIZE TABLE mt_table FINAL; + +SELECT '========'; + +CHECK TABLE mt_table; + +SELECT '========'; + +INSERT INTO mt_table VALUES (toDate('2019-02-03'), 5, '!'), (toDate('2019-02-03'), 6, '?'); + +CHECK TABLE mt_table; + +SELECT '========'; + +INSERT INTO mt_table VALUES (toDate('2019-02-03'), 7, 'jump'), (toDate('2019-02-03'), 8, 'around'); + +OPTIMIZE TABLE mt_table FINAL; + +CHECK TABLE mt_table PARTITION 201902; + +DROP TABLE IF EXISTS mt_table; From 74b2440f665159ecb873540130c61a6c3147195b Mon Sep 17 00:00:00 2001 From: akonyaev Date: Tue, 9 Jul 2019 13:26:06 +0300 Subject: [PATCH 57/62] add user parsing in HDFS URI --- dbms/src/IO/HDFSCommon.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/dbms/src/IO/HDFSCommon.cpp b/dbms/src/IO/HDFSCommon.cpp index 34cef18068f..e813a367e9f 100644 --- a/dbms/src/IO/HDFSCommon.cpp +++ b/dbms/src/IO/HDFSCommon.cpp @@ -2,6 +2,7 @@ #if USE_HDFS #include +#include namespace DB { namespace ErrorCodes @@ -25,6 +26,13 @@ HDFSBuilderPtr createHDFSBuilder(const Poco::URI & uri) hdfsBuilderConfSetStr(builder.get(), "input.write.timeout", "60000"); // 1 min hdfsBuilderConfSetStr(builder.get(), "input.connect.timeout", "60000"); // 1 min + std::string userInfo = uri.getUserInfo(); + if (!userInfo.empty() && userInfo.front() != ':') { + char *user = strtok(const_cast (userInfo.c_str()), ":"); + if (user != NULL) { + hdfsBuilderSetUserName(builder.get(), user); + } + } hdfsBuilderSetNameNode(builder.get(), host.c_str()); hdfsBuilderSetNameNodePort(builder.get(), port); return builder; From f5f6e73c84e74082f64a490a7069d178d9db05d0 Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov Date: Tue, 9 Jul 2019 15:12:51 +0300 Subject: [PATCH 58/62] Ignore non-instrumented modules in ThreadSanitizer. libc++ seems not to be instrumented, which is why we get false positives about data races in normal destruction of weak_ptr. In passing, remove redundant configuration of llvm-symbolizer. One symlink to llvm-8 should suffice. --- docker/test/stress/Dockerfile | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/docker/test/stress/Dockerfile b/docker/test/stress/Dockerfile index 0ee49d64d51..b0b94ccc579 100644 --- a/docker/test/stress/Dockerfile +++ b/docker/test/stress/Dockerfile @@ -34,13 +34,8 @@ CMD dpkg -i package_folder/clickhouse-common-static_*.deb; \ ln -s /usr/share/clickhouse-test/config/log_queries.xml /etc/clickhouse-server/users.d/; \ ln -s /usr/share/clickhouse-test/config/part_log.xml /etc/clickhouse-server/config.d/; \ ln -s /usr/lib/llvm-8/bin/llvm-symbolizer /usr/bin/llvm-symbolizer; \ - echo "TSAN_OPTIONS='halt_on_error=1 history_size=7'" >> /etc/environment; \ - echo "TSAN_SYMBOLIZER_PATH=/usr/lib/llvm-8/bin/llvm-symbolizer" >> /etc/environment; \ + echo "TSAN_OPTIONS='halt_on_error=1 history_size=7 ignore_noninstrumented_modules=1 verbosity=1'" >> /etc/environment; \ echo "UBSAN_OPTIONS='print_stacktrace=1'" >> /etc/environment; \ - echo "ASAN_SYMBOLIZER_PATH=/usr/lib/llvm-6.0/bin/llvm-symbolizer" >> /etc/environment; \ - echo "UBSAN_SYMBOLIZER_PATH=/usr/lib/llvm-6.0/bin/llvm-symbolizer" >> /etc/environment; \ - echo "TSAN_SYMBOLIZER_PATH=/usr/lib/llvm-6.0/bin/llvm-symbolizer" >> /etc/environment; \ - echo "LLVM_SYMBOLIZER_PATH=/usr/lib/llvm-6.0/bin/llvm-symbolizer" >> /etc/environment; \ service clickhouse-server start && sleep 5 \ && /s3downloader --dataset-names $DATASETS \ && chmod 777 -R /var/lib/clickhouse \ From c0dc8fc9d3132ed0c4ea9e9a5f73227520126e02 Mon Sep 17 00:00:00 2001 From: akonyaev Date: Tue, 9 Jul 2019 16:16:04 +0300 Subject: [PATCH 59/62] add user parsing in HDFS URI (fixes after review, style fixes) --- dbms/src/IO/HDFSCommon.cpp | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/dbms/src/IO/HDFSCommon.cpp b/dbms/src/IO/HDFSCommon.cpp index e813a367e9f..1747a64932f 100644 --- a/dbms/src/IO/HDFSCommon.cpp +++ b/dbms/src/IO/HDFSCommon.cpp @@ -2,7 +2,9 @@ #if USE_HDFS #include -#include +#include +#include + namespace DB { namespace ErrorCodes @@ -27,9 +29,13 @@ HDFSBuilderPtr createHDFSBuilder(const Poco::URI & uri) hdfsBuilderConfSetStr(builder.get(), "input.connect.timeout", "60000"); // 1 min std::string userInfo = uri.getUserInfo(); - if (!userInfo.empty() && userInfo.front() != ':') { - char *user = strtok(const_cast (userInfo.c_str()), ":"); - if (user != NULL) { + if (!userInfo.empty() && userInfo.front() != ':') + { + std::vector splitRes; + boost::split(splitRes, userInfo, boost::algorithm::is_any_of(":"), boost::algorithm::token_compress_on); + const char *user = splitRes.front().c_str(); + if (user) + { hdfsBuilderSetUserName(builder.get(), user); } } From 54d890b9849e737563a43e61c79570674334bf6f Mon Sep 17 00:00:00 2001 From: akonyaev Date: Tue, 9 Jul 2019 17:09:56 +0300 Subject: [PATCH 60/62] add user parsing in HDFS URI (rewrite split boost->std::string) --- dbms/src/IO/HDFSCommon.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/dbms/src/IO/HDFSCommon.cpp b/dbms/src/IO/HDFSCommon.cpp index 1747a64932f..80e6a0de555 100644 --- a/dbms/src/IO/HDFSCommon.cpp +++ b/dbms/src/IO/HDFSCommon.cpp @@ -2,8 +2,6 @@ #if USE_HDFS #include -#include -#include namespace DB { @@ -31,13 +29,17 @@ HDFSBuilderPtr createHDFSBuilder(const Poco::URI & uri) std::string userInfo = uri.getUserInfo(); if (!userInfo.empty() && userInfo.front() != ':') { - std::vector splitRes; - boost::split(splitRes, userInfo, boost::algorithm::is_any_of(":"), boost::algorithm::token_compress_on); - const char *user = splitRes.front().c_str(); - if (user) + std::string user; + size_t delimPos = userInfo.find(":"); + if (delimPos != std::string::npos) { - hdfsBuilderSetUserName(builder.get(), user); + user = userInfo.substr(0, delimPos); } + else + { + user = userInfo; + } + hdfsBuilderSetUserName(builder.get(), user.c_str()); } hdfsBuilderSetNameNode(builder.get(), host.c_str()); hdfsBuilderSetNameNodePort(builder.get(), port); From 81ee9a368379a01147ef7b86f864ac0568e03f55 Mon Sep 17 00:00:00 2001 From: alesapin Date: Tue, 9 Jul 2019 19:42:37 +0300 Subject: [PATCH 61/62] Log to stderr for external_dictionaries tests --- .../test_external_dictionaries/configs/config.xml | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/dbms/tests/integration/test_external_dictionaries/configs/config.xml b/dbms/tests/integration/test_external_dictionaries/configs/config.xml index 97cbaf97d4a..fb8f0e1312e 100644 --- a/dbms/tests/integration/test_external_dictionaries/configs/config.xml +++ b/dbms/tests/integration/test_external_dictionaries/configs/config.xml @@ -1,12 +1,14 @@ - trace - /var/log/clickhouse-server/clickhouse-server.log - /var/log/clickhouse-server/clickhouse-server.err.log - 1000M - 10 - + trace + /var/log/clickhouse-server/clickhouse-server.log + /var/log/clickhouse-server/clickhouse-server.err.log + 1000M + 10 + /var/log/clickhouse-server/stderr.log + /var/log/clickhouse-server/stdout.log + 9000 127.0.0.1 From f6df9113079b1bf6a782ba5577718e0ee9f86bfd Mon Sep 17 00:00:00 2001 From: alexey-milovidov Date: Tue, 9 Jul 2019 20:35:47 +0300 Subject: [PATCH 62/62] Update HDFSCommon.cpp --- dbms/src/IO/HDFSCommon.cpp | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/dbms/src/IO/HDFSCommon.cpp b/dbms/src/IO/HDFSCommon.cpp index 80e6a0de555..4be67c7a0de 100644 --- a/dbms/src/IO/HDFSCommon.cpp +++ b/dbms/src/IO/HDFSCommon.cpp @@ -10,6 +10,7 @@ namespace ErrorCodes extern const int BAD_ARGUMENTS; extern const int NETWORK_ERROR; } + HDFSBuilderPtr createHDFSBuilder(const Poco::URI & uri) { auto & host = uri.getHost(); @@ -26,19 +27,16 @@ HDFSBuilderPtr createHDFSBuilder(const Poco::URI & uri) hdfsBuilderConfSetStr(builder.get(), "input.write.timeout", "60000"); // 1 min hdfsBuilderConfSetStr(builder.get(), "input.connect.timeout", "60000"); // 1 min - std::string userInfo = uri.getUserInfo(); - if (!userInfo.empty() && userInfo.front() != ':') + std::string user_info = uri.getUserInfo(); + if (!user_info.empty() && user_info.front() != ':') { std::string user; - size_t delimPos = userInfo.find(":"); - if (delimPos != std::string::npos) - { - user = userInfo.substr(0, delimPos); - } + size_t delim_pos = user_info.find(":"); + if (delim_pos != std::string::npos) + user = user_info.substr(0, delim_pos); else - { - user = userInfo; - } + user = user_info; + hdfsBuilderSetUserName(builder.get(), user.c_str()); } hdfsBuilderSetNameNode(builder.get(), host.c_str());