From 125b2ab214aadcaebe166a871a44587e0f1ee712 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sat, 8 May 2021 17:01:25 +0300 Subject: [PATCH 01/26] Suppressions for PVS-Studio --- src/AggregateFunctions/AggregateFunctionBoundingRatio.h | 2 +- src/DataTypes/getLeastSupertype.cpp | 2 +- src/Storages/StorageMergeTree.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/AggregateFunctions/AggregateFunctionBoundingRatio.h b/src/AggregateFunctions/AggregateFunctionBoundingRatio.h index 0cfb33efc10..15b355f51a6 100644 --- a/src/AggregateFunctions/AggregateFunctionBoundingRatio.h +++ b/src/AggregateFunctions/AggregateFunctionBoundingRatio.h @@ -20,7 +20,7 @@ namespace ErrorCodes /** Tracks the leftmost and rightmost (x, y) data points. */ -struct AggregateFunctionBoundingRatioData +struct AggregateFunctionBoundingRatioData //-V730 { struct Point { diff --git a/src/DataTypes/getLeastSupertype.cpp b/src/DataTypes/getLeastSupertype.cpp index 6710313349b..31e8613a88e 100644 --- a/src/DataTypes/getLeastSupertype.cpp +++ b/src/DataTypes/getLeastSupertype.cpp @@ -423,7 +423,7 @@ DataTypePtr getLeastSupertype(const DataTypes & types) size_t min_bit_width_of_integer = std::max(max_bits_of_signed_integer, max_bits_of_unsigned_integer); /// If unsigned is not covered by signed. - if (max_bits_of_signed_integer && max_bits_of_unsigned_integer >= max_bits_of_signed_integer) + if (max_bits_of_signed_integer && max_bits_of_unsigned_integer >= max_bits_of_signed_integer) //-V1051 { // Because 128 and 256 bit integers are significantly slower, we should not promote to them. // But if we already have wide numbers, promotion is necessary. diff --git a/src/Storages/StorageMergeTree.cpp b/src/Storages/StorageMergeTree.cpp index b3165febd7c..7eadfc5142d 100644 --- a/src/Storages/StorageMergeTree.cpp +++ b/src/Storages/StorageMergeTree.cpp @@ -976,7 +976,7 @@ bool StorageMergeTree::mutateSelectedPart(const StorageMetadataPtr & metadata_sn return true; } -std::optional StorageMergeTree::getDataProcessingJob() +std::optional StorageMergeTree::getDataProcessingJob() //-V657 { if (shutdown_called) return {}; From 78a2df590093bc75b4729cadc71b917bd8e737f5 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sat, 8 May 2021 17:05:58 +0300 Subject: [PATCH 02/26] Fix low quality code in Qualtile (found by PVS-Studio) --- src/AggregateFunctions/QuantileExact.h | 50 +++++++++++++++++--------- 1 file changed, 33 insertions(+), 17 deletions(-) diff --git a/src/AggregateFunctions/QuantileExact.h b/src/AggregateFunctions/QuantileExact.h index aa7b4bbe250..cca16aac610 100644 --- a/src/AggregateFunctions/QuantileExact.h +++ b/src/AggregateFunctions/QuantileExact.h @@ -263,7 +263,7 @@ struct QuantileExactLow : public QuantileExactBase((floor(s / 2)) - 1)]; } } - // else quantile is the nth index of the sorted array obtained by multiplying - // level and size of array. Example if level = 0.1 and size of array is 10, - // then return array[1]. - return array[n]; + else + { + // else quantile is the nth index of the sorted array obtained by multiplying + // level and size of array. Example if level = 0.1 and size of array is 10, + // then return array[1]. + size_t n = level < 1 ? level * array.size() : (array.size() - 1); + return array[n]; + } } return std::numeric_limits::quiet_NaN(); } @@ -295,7 +299,7 @@ struct QuantileExactLow : public QuantileExactBase(floor((s / 2) - 1))]; } } - // else quantile is the nth index of the sorted array obtained by multiplying - // level and size of array. Example if level = 0.1 and size of array is 10. - result[indices[i]] = array[n]; + else + { + // else quantile is the nth index of the sorted array obtained by multiplying + // level and size of array. Example if level = 0.1 and size of array is 10. + size_t n = level < 1 ? level * array.size() : (array.size() - 1); + result[indices[i]] = array[n]; + } } } else @@ -337,7 +345,7 @@ struct QuantileExactHigh : public QuantileExactBase(floor(s / 2))]; } - // else quantile is the nth index of the sorted array obtained by multiplying - // level and size of array. Example if level = 0.1 and size of array is 10. - return array[n]; + else + { + // else quantile is the nth index of the sorted array obtained by multiplying + // level and size of array. Example if level = 0.1 and size of array is 10. + size_t n = level < 1 ? level * array.size() : (array.size() - 1); + return array[n]; + } } return std::numeric_limits::quiet_NaN(); } @@ -361,7 +373,7 @@ struct QuantileExactHigh : public QuantileExactBase(floor(s / 2))]; } - // else quantile is the nth index of the sorted array obtained by multiplying - // level and size of array. Example if level = 0.1 and size of array is 10. - result[indices[i]] = array[n]; + else + { + // else quantile is the nth index of the sorted array obtained by multiplying + // level and size of array. Example if level = 0.1 and size of array is 10. + size_t n = level < 1 ? level * array.size() : (array.size() - 1); + result[indices[i]] = array[n]; + } } } else From 7ca0f46708700a3d46e440d1d2eb03ac290a321b Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sat, 8 May 2021 17:09:40 +0300 Subject: [PATCH 03/26] Add suppressions for PVS-Studio --- src/Functions/runningAccumulate.cpp | 4 ++-- src/Parsers/ASTCreateQuery.cpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Functions/runningAccumulate.cpp b/src/Functions/runningAccumulate.cpp index 7c1b1f7162e..0fe354e30c0 100644 --- a/src/Functions/runningAccumulate.cpp +++ b/src/Functions/runningAccumulate.cpp @@ -120,8 +120,8 @@ public: state_created = false; } - agg_func.create(place.data()); - state_created = true; + agg_func.create(place.data()); /// This function can throw. + state_created = true; //-V519 } agg_func.merge(place.data(), state_to_add, arena.get()); diff --git a/src/Parsers/ASTCreateQuery.cpp b/src/Parsers/ASTCreateQuery.cpp index 1192fcc6ebd..d4c7312f437 100644 --- a/src/Parsers/ASTCreateQuery.cpp +++ b/src/Parsers/ASTCreateQuery.cpp @@ -327,7 +327,7 @@ void ASTCreateQuery::formatQueryImpl(const FormatSettings & settings, FormatStat FormatStateStacked frame_nested = frame; columns_list->formatImpl(settings, state, frame_nested); settings.ostr << (settings.one_line ? ")" : "\n)"); - frame.expression_list_always_start_on_new_line = false; + frame.expression_list_always_start_on_new_line = false; //-V519 } settings.ostr << (settings.hilite ? hilite_keyword : "") << " AS " << (settings.hilite ? hilite_none : ""); @@ -355,7 +355,7 @@ void ASTCreateQuery::formatQueryImpl(const FormatSettings & settings, FormatStat settings.ostr << (settings.one_line ? ")" : "\n)"); } - frame.expression_list_always_start_on_new_line = false; + frame.expression_list_always_start_on_new_line = false; //-V519 if (storage) storage->formatImpl(settings, state, frame); From 241c7a94da483b8261ba74abcee522cac78a042c Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sat, 8 May 2021 17:10:06 +0300 Subject: [PATCH 04/26] Fix bad code in Keeper (found by PVS-Studio) --- src/Coordination/KeeperStorage.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Coordination/KeeperStorage.cpp b/src/Coordination/KeeperStorage.cpp index 197cc4323cf..5f2d6141be9 100644 --- a/src/Coordination/KeeperStorage.cpp +++ b/src/Coordination/KeeperStorage.cpp @@ -103,7 +103,8 @@ struct KeeperStorageSyncRequest final : public KeeperStorageRequest std::pair process(KeeperStorage::Container & /* container */, KeeperStorage::Ephemerals & /* ephemerals */, int64_t /* zxid */, int64_t /* session_id */) const override { auto response = zk_request->makeResponse(); - dynamic_cast(response.get())->path = dynamic_cast(zk_request.get())->path; + dynamic_cast(*response).path + = dynamic_cast(*zk_request).path; return {response, {}}; } }; From 8b9c058141a7490021b30433853d9cdadb9a3eeb Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sat, 8 May 2021 17:13:32 +0300 Subject: [PATCH 05/26] Fix bad code in Keeper (found by PVS-Studio) --- src/Coordination/tests/gtest_for_build.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Coordination/tests/gtest_for_build.cpp b/src/Coordination/tests/gtest_for_build.cpp index 515565a3b9f..a0313651c21 100644 --- a/src/Coordination/tests/gtest_for_build.cpp +++ b/src/Coordination/tests/gtest_for_build.cpp @@ -64,7 +64,7 @@ TEST(CoordinationTest, BufferSerde) { Coordination::ZooKeeperRequestPtr request = Coordination::ZooKeeperRequestFactory::instance().get(Coordination::OpNum::Get); request->xid = 3; - dynamic_cast(request.get())->path = "/path/value"; + dynamic_cast(*request).path = "/path/value"; DB::WriteBufferFromNuraftBuffer wbuf; request->write(wbuf); @@ -90,7 +90,7 @@ TEST(CoordinationTest, BufferSerde) EXPECT_EQ(request_read->getOpNum(), Coordination::OpNum::Get); EXPECT_EQ(request_read->xid, 3); - EXPECT_EQ(dynamic_cast(request_read.get())->path, "/path/value"); + EXPECT_EQ(dynamic_cast(*request_read).path, "/path/value"); } template From bdc372cb9a11603c50d5f850ccc3d06cf304e6e4 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sat, 8 May 2021 17:21:36 +0300 Subject: [PATCH 06/26] Fix bad code (found by PVS-Studio) --- src/Dictionaries/tests/gtest_dictionary_configuration.cpp | 4 ++-- src/IO/tests/gtest_cascade_and_memory_write_buffer.cpp | 5 ++--- src/Interpreters/DatabaseCatalog.cpp | 2 +- src/Parsers/ExpressionElementParsers.cpp | 7 ++++--- src/Parsers/New/ParserErrorListener.cpp | 3 ++- 5 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/Dictionaries/tests/gtest_dictionary_configuration.cpp b/src/Dictionaries/tests/gtest_dictionary_configuration.cpp index 389199bfe61..2b16228b59e 100644 --- a/src/Dictionaries/tests/gtest_dictionary_configuration.cpp +++ b/src/Dictionaries/tests/gtest_dictionary_configuration.cpp @@ -19,10 +19,10 @@ static bool registered = false; #pragma GCC diagnostic ignored "-Wunused-function" static std::string configurationToString(const DictionaryConfigurationPtr & config) { - const Poco::Util::XMLConfiguration * xml_config = dynamic_cast(config.get()); + const Poco::Util::XMLConfiguration & xml_config = dynamic_cast(*config); std::ostringstream oss; // STYLE_CHECK_ALLOW_STD_STRING_STREAM oss.exceptions(std::ios::failbit); - xml_config->save(oss); + xml_config.save(oss); return oss.str(); } diff --git a/src/IO/tests/gtest_cascade_and_memory_write_buffer.cpp b/src/IO/tests/gtest_cascade_and_memory_write_buffer.cpp index 4936307a5e3..53248467418 100644 --- a/src/IO/tests/gtest_cascade_and_memory_write_buffer.cpp +++ b/src/IO/tests/gtest_cascade_and_memory_write_buffer.cpp @@ -41,10 +41,9 @@ static void testCascadeBufferRedability( if (!wbuf) continue; - auto * wbuf_readable = dynamic_cast(wbuf.get()); - ASSERT_FALSE(!wbuf_readable); + auto & wbuf_readable = dynamic_cast(*wbuf); - auto rbuf = wbuf_readable->tryGetReadBuffer(); + auto rbuf = wbuf_readable.tryGetReadBuffer(); ASSERT_FALSE(!rbuf); read_buffers.emplace_back(rbuf); diff --git a/src/Interpreters/DatabaseCatalog.cpp b/src/Interpreters/DatabaseCatalog.cpp index 23e147bcc3b..190620e87ff 100644 --- a/src/Interpreters/DatabaseCatalog.cpp +++ b/src/Interpreters/DatabaseCatalog.cpp @@ -56,7 +56,7 @@ TemporaryTableHolder::TemporaryTableHolder(ContextPtr context_, const TemporaryT ASTPtr original_create; ASTCreateQuery * create = dynamic_cast(query.get()); String global_name; - if (query) + if (create) { original_create = create->clone(); if (create->uuid == UUIDHelpers::Nil) diff --git a/src/Parsers/ExpressionElementParsers.cpp b/src/Parsers/ExpressionElementParsers.cpp index 7c91d4ee89b..9c20cfd0327 100644 --- a/src/Parsers/ExpressionElementParsers.cpp +++ b/src/Parsers/ExpressionElementParsers.cpp @@ -497,7 +497,8 @@ bool ParserTableFunctionView::parseImpl(Pos & pos, ASTPtr & node, Expected & exp bool ParserWindowReference::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) { - ASTFunction * function = dynamic_cast(node.get()); + assert(node); + ASTFunction & function = dynamic_cast(*node); // Variant 1: // function_name ( * ) OVER window_name @@ -510,7 +511,7 @@ bool ParserWindowReference::parseImpl(Pos & pos, ASTPtr & node, Expected & expec ParserIdentifier window_name_parser; if (window_name_parser.parse(pos, window_name_ast, expected)) { - function->window_name = getIdentifierName(window_name_ast); + function.window_name = getIdentifierName(window_name_ast); return true; } else @@ -522,7 +523,7 @@ bool ParserWindowReference::parseImpl(Pos & pos, ASTPtr & node, Expected & expec // Variant 2: // function_name ( * ) OVER ( window_definition ) ParserWindowDefinition parser_definition; - return parser_definition.parse(pos, function->window_definition, expected); + return parser_definition.parse(pos, function.window_definition, expected); } static bool tryParseFrameDefinition(ASTWindowDefinition * node, IParser::Pos & pos, diff --git a/src/Parsers/New/ParserErrorListener.cpp b/src/Parsers/New/ParserErrorListener.cpp index 5c1b9238d15..bc5ee84cba9 100644 --- a/src/Parsers/New/ParserErrorListener.cpp +++ b/src/Parsers/New/ParserErrorListener.cpp @@ -23,7 +23,8 @@ extern int SYNTAX_ERROR; void ParserErrorListener::syntaxError( Recognizer * recognizer, Token * token, size_t, size_t, const std::string & message, std::exception_ptr) { - auto * parser = dynamic_cast(recognizer); + auto * parser = dynamic_cast(recognizer); + assert(parser); LOG_ERROR(&Poco::Logger::get("ClickHouseParser"), "Last element parsed so far:\n" From 8d62c42eb9afee75575f4b533cfba47786346f87 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sat, 8 May 2021 17:43:03 +0300 Subject: [PATCH 07/26] A bunch of changes for PVS-Studio --- base/daemon/SentryWriter.cpp | 2 +- src/Core/MySQL/MySQLReplication.cpp | 10 +++++----- src/DataTypes/convertMySQLDataType.cpp | 4 ++-- .../PostgreSQL/fetchPostgreSQLTableStructure.cpp | 6 +++--- src/Functions/neighbor.cpp | 2 +- src/Interpreters/DuplicateOrderByVisitor.h | 2 +- src/Interpreters/GroupByFunctionKeysVisitor.h | 2 +- src/Interpreters/InJoinSubqueriesPreprocessor.cpp | 4 ++-- .../InterpreterShowAccessEntitiesQuery.cpp | 8 ++++++-- src/Parsers/ParserGrantQuery.cpp | 3 +-- src/Processors/Formats/IRowOutputFormat.cpp | 2 +- .../MergingAggregatedMemoryEfficientTransform.cpp | 5 +++-- src/Storages/MergeTree/DataPartsExchange.cpp | 4 +++- src/Storages/StorageBuffer.cpp | 2 +- 14 files changed, 31 insertions(+), 25 deletions(-) diff --git a/base/daemon/SentryWriter.cpp b/base/daemon/SentryWriter.cpp index 1b7d0064b99..1028dc7d2dc 100644 --- a/base/daemon/SentryWriter.cpp +++ b/base/daemon/SentryWriter.cpp @@ -102,7 +102,7 @@ void SentryWriter::initialize(Poco::Util::LayeredConfiguration & config) auto * logger = &Poco::Logger::get("SentryWriter"); if (config.getBool("send_crash_reports.enabled", false)) { - if (debug || (strlen(VERSION_OFFICIAL) > 0)) + if (debug || (strlen(VERSION_OFFICIAL) > 0)) //-V560 { enabled = true; } diff --git a/src/Core/MySQL/MySQLReplication.cpp b/src/Core/MySQL/MySQLReplication.cpp index 7e3f68983be..402b026736c 100644 --- a/src/Core/MySQL/MySQLReplication.cpp +++ b/src/Core/MySQL/MySQLReplication.cpp @@ -474,19 +474,19 @@ namespace MySQLReplication } case MYSQL_TYPE_NEWDECIMAL: { - const auto & dispatch = [](const size_t & precision, const size_t & scale, const auto & function) -> Field + const auto & dispatch = [](size_t precision, size_t scale, const auto & function) -> Field { if (precision <= DecimalUtils::max_precision) return Field(function(precision, scale, Decimal32())); - else if (precision <= DecimalUtils::max_precision) + else if (precision <= DecimalUtils::max_precision) //-V547 return Field(function(precision, scale, Decimal64())); - else if (precision <= DecimalUtils::max_precision) + else if (precision <= DecimalUtils::max_precision) //-V547 return Field(function(precision, scale, Decimal128())); return Field(function(precision, scale, Decimal256())); }; - const auto & read_decimal = [&](const size_t & precision, const size_t & scale, auto decimal) + const auto & read_decimal = [&](size_t precision, size_t scale, auto decimal) { using DecimalType = decltype(decimal); static constexpr size_t digits_per_integer = 9; @@ -543,7 +543,7 @@ namespace MySQLReplication UInt32 val = 0; size_t to_read = compressed_bytes_map[compressed_decimals]; - if (to_read) + if (to_read) //-V547 { readBigEndianStrict(payload, reinterpret_cast(&val), to_read); res *= intExp10OfSize(compressed_decimals); diff --git a/src/DataTypes/convertMySQLDataType.cpp b/src/DataTypes/convertMySQLDataType.cpp index 5684503e34a..bcccb58966a 100644 --- a/src/DataTypes/convertMySQLDataType.cpp +++ b/src/DataTypes/convertMySQLDataType.cpp @@ -105,9 +105,9 @@ DataTypePtr convertMySQLDataType(MultiEnum type_support, { if (precision <= DecimalUtils::max_precision) res = std::make_shared>(precision, scale); - else if (precision <= DecimalUtils::max_precision) + else if (precision <= DecimalUtils::max_precision) //-V547 res = std::make_shared>(precision, scale); - else if (precision <= DecimalUtils::max_precision) + else if (precision <= DecimalUtils::max_precision) //-V547 res = std::make_shared>(precision, scale); } diff --git a/src/Databases/PostgreSQL/fetchPostgreSQLTableStructure.cpp b/src/Databases/PostgreSQL/fetchPostgreSQLTableStructure.cpp index a1dd74e0a73..2eb10bf273f 100644 --- a/src/Databases/PostgreSQL/fetchPostgreSQLTableStructure.cpp +++ b/src/Databases/PostgreSQL/fetchPostgreSQLTableStructure.cpp @@ -67,11 +67,11 @@ static DataTypePtr convertPostgreSQLDataType(String & type, bool is_nullable, ui if (precision <= DecimalUtils::max_precision) res = std::make_shared>(precision, scale); - else if (precision <= DecimalUtils::max_precision) + else if (precision <= DecimalUtils::max_precision) //-V547 res = std::make_shared>(precision, scale); - else if (precision <= DecimalUtils::max_precision) + else if (precision <= DecimalUtils::max_precision) //-V547 res = std::make_shared>(precision, scale); - else if (precision <= DecimalUtils::max_precision) + else if (precision <= DecimalUtils::max_precision) //-V547 res = std::make_shared>(precision, scale); else throw Exception(ErrorCodes::BAD_ARGUMENTS, "Precision {} and scale {} are too big and not supported", precision, scale); diff --git a/src/Functions/neighbor.cpp b/src/Functions/neighbor.cpp index e841fe9e79b..0de7c480b18 100644 --- a/src/Functions/neighbor.cpp +++ b/src/Functions/neighbor.cpp @@ -178,7 +178,7 @@ public: for (size_t row = 0; row < input_rows_count; ++row) { - Int64 offset = offset_column->getInt(offset_is_constant ? 0 : row); + Int64 offset = offset_column->getInt(row); /// Protection from possible overflow. if (unlikely(offset > (1 << 30) || offset < -(1 << 30))) diff --git a/src/Interpreters/DuplicateOrderByVisitor.h b/src/Interpreters/DuplicateOrderByVisitor.h index 4f95854e17b..4231b2600af 100644 --- a/src/Interpreters/DuplicateOrderByVisitor.h +++ b/src/Interpreters/DuplicateOrderByVisitor.h @@ -93,7 +93,7 @@ public: bool is_stateful = false; ASTFunctionStatefulVisitor::Data data{context, is_stateful}; ASTFunctionStatefulVisitor(data).visit(elem); - if (is_stateful) + if (is_stateful) //-V547 return; } } diff --git a/src/Interpreters/GroupByFunctionKeysVisitor.h b/src/Interpreters/GroupByFunctionKeysVisitor.h index 0c745be33ed..855723307d6 100644 --- a/src/Interpreters/GroupByFunctionKeysVisitor.h +++ b/src/Interpreters/GroupByFunctionKeysVisitor.h @@ -97,7 +97,7 @@ public: KeepFunctionVisitor::Data keep_data{data.key_names_to_keep, keep_key}; KeepFunctionVisitor(keep_data).visit(function_node->arguments); - if (!keep_key) + if (!keep_key) //-V547 (data.key_names_to_keep).erase(function_node->getColumnName()); } diff --git a/src/Interpreters/InJoinSubqueriesPreprocessor.cpp b/src/Interpreters/InJoinSubqueriesPreprocessor.cpp index 8dc5d95bd89..e173f1554f4 100644 --- a/src/Interpreters/InJoinSubqueriesPreprocessor.cpp +++ b/src/Interpreters/InJoinSubqueriesPreprocessor.cpp @@ -178,7 +178,7 @@ private: std::vector renamed; NonGlobalTableVisitor::Data table_data(data.getContext(), data.checker, renamed, &node, nullptr); NonGlobalTableVisitor(table_data).visit(subquery); - if (!renamed.empty()) + if (!renamed.empty()) //-V547 data.renamed_tables.emplace_back(subquery, std::move(renamed)); } } @@ -196,7 +196,7 @@ private: std::vector renamed; NonGlobalTableVisitor::Data table_data(data.getContext(), data.checker, renamed, nullptr, table_join); NonGlobalTableVisitor(table_data).visit(subquery); - if (!renamed.empty()) + if (!renamed.empty()) //-V547 data.renamed_tables.emplace_back(subquery, std::move(renamed)); } } diff --git a/src/Interpreters/InterpreterShowAccessEntitiesQuery.cpp b/src/Interpreters/InterpreterShowAccessEntitiesQuery.cpp index afd685ea983..c2c2305f976 100644 --- a/src/Interpreters/InterpreterShowAccessEntitiesQuery.cpp +++ b/src/Interpreters/InterpreterShowAccessEntitiesQuery.cpp @@ -33,9 +33,11 @@ String InterpreterShowAccessEntitiesQuery::getRewrittenQuery() const { auto & query = query_ptr->as(); query.replaceEmptyDatabase(getContext()->getCurrentDatabase()); + String origin; String expr = "*"; - String filter, order; + String filter; + String order; switch (query.type) { @@ -43,8 +45,10 @@ String InterpreterShowAccessEntitiesQuery::getRewrittenQuery() const { origin = "row_policies"; expr = "name"; + if (!query.short_name.empty()) - filter += String{filter.empty() ? "" : " AND "} + "short_name = " + quoteString(query.short_name); + filter = "short_name = " + quoteString(query.short_name); + if (query.database_and_table_name) { const String & database = query.database_and_table_name->first; diff --git a/src/Parsers/ParserGrantQuery.cpp b/src/Parsers/ParserGrantQuery.cpp index d3aa62e73da..9411fa93892 100644 --- a/src/Parsers/ParserGrantQuery.cpp +++ b/src/Parsers/ParserGrantQuery.cpp @@ -238,8 +238,7 @@ bool ParserGrantQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) return false; String cluster; - if (cluster.empty()) - parseOnCluster(pos, expected, cluster); + parseOnCluster(pos, expected, cluster); bool grant_option = false; bool admin_option = false; diff --git a/src/Processors/Formats/IRowOutputFormat.cpp b/src/Processors/Formats/IRowOutputFormat.cpp index b714844feea..6b7a9a46eaa 100644 --- a/src/Processors/Formats/IRowOutputFormat.cpp +++ b/src/Processors/Formats/IRowOutputFormat.cpp @@ -103,7 +103,7 @@ void IRowOutputFormat::writeMinExtreme(const DB::Columns & columns, size_t row_n write(columns, row_num); } -void IRowOutputFormat::writeMaxExtreme(const DB::Columns & columns, size_t row_num) +void IRowOutputFormat::writeMaxExtreme(const DB::Columns & columns, size_t row_num) //-V524 { write(columns, row_num); } diff --git a/src/Processors/Transforms/MergingAggregatedMemoryEfficientTransform.cpp b/src/Processors/Transforms/MergingAggregatedMemoryEfficientTransform.cpp index 83c1e35ad12..df2ea4b03f0 100644 --- a/src/Processors/Transforms/MergingAggregatedMemoryEfficientTransform.cpp +++ b/src/Processors/Transforms/MergingAggregatedMemoryEfficientTransform.cpp @@ -228,13 +228,14 @@ IProcessor::Status GroupingAggregatedTransform::prepare() return Status::PortFull; /// Sanity check. If new bucket was read, we should be able to push it. - if (!all_inputs_finished) + /// This is always false, but we still keep this condition in case the code will be changed. + if (!all_inputs_finished) // -V547 throw Exception("GroupingAggregatedTransform has read new two-level bucket, but couldn't push it.", ErrorCodes::LOGICAL_ERROR); } else { - if (!all_inputs_finished) + if (!all_inputs_finished) // -V547 throw Exception("GroupingAggregatedTransform should have read all chunks for single level aggregation, " "but not all of the inputs are finished.", ErrorCodes::LOGICAL_ERROR); diff --git a/src/Storages/MergeTree/DataPartsExchange.cpp b/src/Storages/MergeTree/DataPartsExchange.cpp index 205d57f533e..620dd2de278 100644 --- a/src/Storages/MergeTree/DataPartsExchange.cpp +++ b/src/Storages/MergeTree/DataPartsExchange.cpp @@ -419,7 +419,9 @@ MergeTreeData::MutableDataPartPtr Fetcher::fetchPart( throw Exception("Got 'send_s3_metadata' cookie for in-memory part", ErrorCodes::INCORRECT_PART_TYPE); UUID part_uuid = UUIDHelpers::Nil; - if (server_protocol_version >= REPLICATION_PROTOCOL_VERSION_WITH_PARTS_UUID) + + /// Always true due to values of constants. But we keep this condition just in case. + if (server_protocol_version >= REPLICATION_PROTOCOL_VERSION_WITH_PARTS_UUID) //-V547 readUUIDText(part_uuid, in); try diff --git a/src/Storages/StorageBuffer.cpp b/src/Storages/StorageBuffer.cpp index 58161a94f45..08c55c79775 100644 --- a/src/Storages/StorageBuffer.cpp +++ b/src/Storages/StorageBuffer.cpp @@ -859,7 +859,7 @@ void StorageBuffer::flushBuffer(Buffer & buffer, bool check_thresholds, bool loc buffer.data.swap(block_to_write); - if (!buffer.first_write_time) + if (!buffer.first_write_time) // -V547 buffer.first_write_time = current_time; /// After a while, the next write attempt will happen. From 022667a190fae935509f0a9f5dcf5a2e6c1fd0b3 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sat, 8 May 2021 17:54:08 +0300 Subject: [PATCH 08/26] Fix some PVS-Studio warnings --- src/Functions/greatCircleDistance.cpp | 4 ++-- .../Transforms/FillingTransform.cpp | 6 +++--- src/Storages/System/StorageSystemTables.cpp | 21 ++++++------------- 3 files changed, 11 insertions(+), 20 deletions(-) diff --git a/src/Functions/greatCircleDistance.cpp b/src/Functions/greatCircleDistance.cpp index 93ec7c9de4c..095edd25e40 100644 --- a/src/Functions/greatCircleDistance.cpp +++ b/src/Functions/greatCircleDistance.cpp @@ -187,8 +187,8 @@ float distance(float lon1deg, float lat1deg, float lon2deg, float lat2deg) /// This is linear interpolation between two table items at index "latitude_midpoint_index" and "latitude_midpoint_index + 1". - float k_lat; - float k_lon; + float k_lat{}; + float k_lon{}; if constexpr (method == Method::SPHERE_DEGREES) { diff --git a/src/Processors/Transforms/FillingTransform.cpp b/src/Processors/Transforms/FillingTransform.cpp index 3f3a0e75223..085e21b1766 100644 --- a/src/Processors/Transforms/FillingTransform.cpp +++ b/src/Processors/Transforms/FillingTransform.cpp @@ -36,6 +36,8 @@ FillingTransform::FillingTransform( auto max_type = Field::Types::Null; WhichDataType which(type); DataTypePtr to_type; + + /// TODO Wrong results for big integers. if (isInteger(type) || which.isDateOrDateTime()) { max_type = Field::Types::Int64; @@ -46,9 +48,7 @@ FillingTransform::FillingTransform( max_type = Field::Types::Float64; to_type = std::make_shared(); } - - if (descr.fill_from.getType() > max_type || descr.fill_to.getType() > max_type - || descr.fill_step.getType() > max_type) + else return false; descr.fill_from = convertFieldToType(descr.fill_from, *to_type); diff --git a/src/Storages/System/StorageSystemTables.cpp b/src/Storages/System/StorageSystemTables.cpp index 9602339f381..7046a37766c 100644 --- a/src/Storages/System/StorageSystemTables.cpp +++ b/src/Storages/System/StorageSystemTables.cpp @@ -389,14 +389,13 @@ protected: src_index += 2; StorageMetadataPtr metadata_snapshot; - if (table != nullptr) - metadata_snapshot = table->getInMemoryMetadataPtr(); + assert(table != nullptr); + metadata_snapshot = table->getInMemoryMetadataPtr(); ASTPtr expression_ptr; if (columns_mask[src_index++]) { - assert(metadata_snapshot != nullptr); - if ((expression_ptr = metadata_snapshot->getPartitionKeyAST())) + if (metadata_snapshot && (expression_ptr = metadata_snapshot->getPartitionKeyAST())) res_columns[res_index++]->insert(queryToString(expression_ptr)); else res_columns[res_index++]->insertDefault(); @@ -404,8 +403,7 @@ protected: if (columns_mask[src_index++]) { - assert(metadata_snapshot != nullptr); - if ((expression_ptr = metadata_snapshot->getSortingKey().expression_list_ast)) + if (metadata_snapshot && (expression_ptr = metadata_snapshot->getSortingKey().expression_list_ast)) res_columns[res_index++]->insert(queryToString(expression_ptr)); else res_columns[res_index++]->insertDefault(); @@ -413,8 +411,7 @@ protected: if (columns_mask[src_index++]) { - assert(metadata_snapshot != nullptr); - if ((expression_ptr = metadata_snapshot->getPrimaryKey().expression_list_ast)) + if (metadata_snapshot && (expression_ptr = metadata_snapshot->getPrimaryKey().expression_list_ast)) res_columns[res_index++]->insert(queryToString(expression_ptr)); else res_columns[res_index++]->insertDefault(); @@ -422,8 +419,7 @@ protected: if (columns_mask[src_index++]) { - assert(metadata_snapshot != nullptr); - if ((expression_ptr = metadata_snapshot->getSamplingKeyAST())) + if (metadata_snapshot && (expression_ptr = metadata_snapshot->getSamplingKeyAST())) res_columns[res_index++]->insert(queryToString(expression_ptr)); else res_columns[res_index++]->insertDefault(); @@ -431,7 +427,6 @@ protected: if (columns_mask[src_index++]) { - assert(table != nullptr); auto policy = table->getStoragePolicy(); if (policy) res_columns[res_index++]->insert(policy->getName()); @@ -441,7 +436,6 @@ protected: if (columns_mask[src_index++]) { - assert(table != nullptr); auto total_rows = table->totalRows(context->getSettingsRef()); if (total_rows) res_columns[res_index++]->insert(*total_rows); @@ -451,7 +445,6 @@ protected: if (columns_mask[src_index++]) { - assert(table != nullptr); auto total_bytes = table->totalBytes(context->getSettingsRef()); if (total_bytes) res_columns[res_index++]->insert(*total_bytes); @@ -461,7 +454,6 @@ protected: if (columns_mask[src_index++]) { - assert(table != nullptr); auto lifetime_rows = table->lifetimeRows(); if (lifetime_rows) res_columns[res_index++]->insert(*lifetime_rows); @@ -471,7 +463,6 @@ protected: if (columns_mask[src_index++]) { - assert(table != nullptr); auto lifetime_bytes = table->lifetimeBytes(); if (lifetime_bytes) res_columns[res_index++]->insert(*lifetime_bytes); From e905883c759b84916f8e2af16cabf1945109715b Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sat, 8 May 2021 18:20:40 +0300 Subject: [PATCH 09/26] More fixes for PVS-Studio --- .../library-bridge/SharedLibraryHandler.h | 2 ++ src/Access/Quota.h | 2 +- .../AggregateFunctionHistogram.h | 2 +- .../AggregateFunctionMinMaxAny.h | 2 +- src/Common/CompactArray.h | 6 ++--- src/Common/ErrorCodes.h | 2 +- src/Common/HashTable/ClearableHashSet.h | 2 +- src/Common/HashTable/FixedClearableHashSet.h | 2 +- src/Common/HashTable/FixedHashMap.h | 6 ++--- src/Common/HashTable/FixedHashTable.h | 4 ++-- src/Common/HashTable/HashSet.h | 4 ++-- src/Common/HashTable/HashTable.h | 2 +- src/Common/HashTable/SmallTable.h | 24 +++++++++---------- src/Common/HashTable/StringHashTable.h | 2 +- src/Common/HashTable/TwoLevelHashTable.h | 12 +++++----- src/Common/MemoryTracker.h | 2 +- src/Common/SharedBlockRowRef.h | 2 +- src/Common/ThreadStatus.h | 2 +- src/Common/ZooKeeper/ZooKeeperCommon.h | 2 +- src/Coordination/Changelog.h | 6 ++--- src/Core/PostgreSQLProtocol.h | 6 +++-- src/Core/SortCursor.h | 2 +- .../tests/gtest_data_type_get_common_type.cpp | 2 +- src/Functions/isDecimalOverflow.cpp | 2 +- src/Interpreters/ActionsDAG.h | 2 +- src/Interpreters/DDLTask.h | 4 ++-- src/Interpreters/DatabaseCatalog.h | 2 +- src/Interpreters/TextLog.h | 6 ++--- src/Interpreters/sortBlock.h | 2 +- src/Processors/Executors/PollingQueue.h | 2 +- src/Storages/AlterCommands.cpp | 4 +++- src/Storages/AlterCommands.h | 3 ++- src/Storages/MergeTree/MergeSelector.h | 8 +++---- src/Storages/MergeTree/MergeTreeData.cpp | 3 +++ .../MergeTree/SimpleMergeSelector.cpp | 2 ++ src/Storages/PartitionCommands.cpp | 9 ++++++- src/Storages/PartitionCommands.h | 4 +++- 37 files changed, 86 insertions(+), 65 deletions(-) diff --git a/programs/library-bridge/SharedLibraryHandler.h b/programs/library-bridge/SharedLibraryHandler.h index 5c0334ac89f..fa476995e32 100644 --- a/programs/library-bridge/SharedLibraryHandler.h +++ b/programs/library-bridge/SharedLibraryHandler.h @@ -23,6 +23,8 @@ public: SharedLibraryHandler(const SharedLibraryHandler & other); + SharedLibraryHandler & operator=(const SharedLibraryHandler & other) = delete; + ~SharedLibraryHandler(); BlockInputStreamPtr loadAll(); diff --git a/src/Access/Quota.h b/src/Access/Quota.h index 430bdca29b0..b7970b2583b 100644 --- a/src/Access/Quota.h +++ b/src/Access/Quota.h @@ -45,7 +45,7 @@ struct Quota : public IAccessEntity struct ResourceTypeInfo { - const char * const raw_name; + const char * const raw_name = ""; const String name; /// Lowercased with underscores, e.g. "result_rows". const String keyword; /// Uppercased with spaces, e.g. "RESULT ROWS". const bool output_as_float = false; diff --git a/src/AggregateFunctions/AggregateFunctionHistogram.h b/src/AggregateFunctions/AggregateFunctionHistogram.h index c44cb61b275..a5a6920ce33 100644 --- a/src/AggregateFunctions/AggregateFunctionHistogram.h +++ b/src/AggregateFunctions/AggregateFunctionHistogram.h @@ -220,7 +220,7 @@ private: } public: - AggregateFunctionHistogramData() + AggregateFunctionHistogramData() //-V730 : size(0) , lower_bound(std::numeric_limits::max()) , upper_bound(std::numeric_limits::lowest()) diff --git a/src/AggregateFunctions/AggregateFunctionMinMaxAny.h b/src/AggregateFunctions/AggregateFunctionMinMaxAny.h index 919026a78c1..f4561660ad5 100644 --- a/src/AggregateFunctions/AggregateFunctionMinMaxAny.h +++ b/src/AggregateFunctions/AggregateFunctionMinMaxAny.h @@ -181,7 +181,7 @@ public: /** For strings. Short strings are stored in the object itself, and long strings are allocated separately. * NOTE It could also be suitable for arrays of numbers. */ -struct SingleValueDataString +struct SingleValueDataString //-V730 { private: using Self = SingleValueDataString; diff --git a/src/Common/CompactArray.h b/src/Common/CompactArray.h index 785cd04b4d0..7291fe3f18d 100644 --- a/src/Common/CompactArray.h +++ b/src/Common/CompactArray.h @@ -159,12 +159,12 @@ private: /// The number of bytes read. size_t read_count = 0; /// The content in the current position. - UInt8 value_l; - UInt8 value_r; + UInt8 value_l = 0; + UInt8 value_r = 0; /// bool is_eof = false; /// Does the cell fully fit into one byte? - bool fits_in_byte; + bool fits_in_byte = false; }; /** TODO This code looks very suboptimal. diff --git a/src/Common/ErrorCodes.h b/src/Common/ErrorCodes.h index ffd0b8b8619..cefd77df868 100644 --- a/src/Common/ErrorCodes.h +++ b/src/Common/ErrorCodes.h @@ -29,7 +29,7 @@ namespace ErrorCodes struct Error { /// Number of times Exception with this ErrorCode had been throw. - Value count; + Value count = 0; /// Time of the last error. UInt64 error_time_ms = 0; /// Message for the last error. diff --git a/src/Common/HashTable/ClearableHashSet.h b/src/Common/HashTable/ClearableHashSet.h index dc057afacd8..7da0469a99a 100644 --- a/src/Common/HashTable/ClearableHashSet.h +++ b/src/Common/HashTable/ClearableHashSet.h @@ -44,7 +44,7 @@ struct ClearableHashTableCell : public BaseCell /// Do I need to store the zero key separately (that is, can a zero key be inserted into the hash table). static constexpr bool need_zero_value_storage = false; - ClearableHashTableCell() {} + ClearableHashTableCell() {} //-V730 ClearableHashTableCell(const Key & key_, const State & state) : BaseCell(key_, state), version(state.version) {} }; diff --git a/src/Common/HashTable/FixedClearableHashSet.h b/src/Common/HashTable/FixedClearableHashSet.h index 19d4669831f..56c7afd250e 100644 --- a/src/Common/HashTable/FixedClearableHashSet.h +++ b/src/Common/HashTable/FixedClearableHashSet.h @@ -13,7 +13,7 @@ struct FixedClearableHashTableCell using mapped_type = VoidMapped; UInt32 version; - FixedClearableHashTableCell() {} + FixedClearableHashTableCell() {} //-V730 FixedClearableHashTableCell(const Key &, const State & state) : version(state.version) {} const VoidKey getKey() const { return {}; } diff --git a/src/Common/HashTable/FixedHashMap.h b/src/Common/HashTable/FixedHashMap.h index 9fc331e09e4..37bd81c8b4f 100644 --- a/src/Common/HashTable/FixedHashMap.h +++ b/src/Common/HashTable/FixedHashMap.h @@ -16,7 +16,7 @@ struct FixedHashMapCell bool full; Mapped mapped; - FixedHashMapCell() {} + FixedHashMapCell() {} //-V730 FixedHashMapCell(const Key &, const State &) : full(true) {} FixedHashMapCell(const value_type & value_, const State &) : full(true), mapped(value_.second) {} @@ -31,7 +31,7 @@ struct FixedHashMapCell /// Note that we have to assemble a continuous layout for the value_type on each call of getValue(). struct CellExt { - CellExt() {} + CellExt() {} //-V730 CellExt(Key && key_, const FixedHashMapCell * ptr_) : key(key_), ptr(const_cast(ptr_)) {} void update(Key && key_, const FixedHashMapCell * ptr_) { @@ -76,7 +76,7 @@ struct FixedHashMapImplicitZeroCell /// Note that we have to assemble a continuous layout for the value_type on each call of getValue(). struct CellExt { - CellExt() {} + CellExt() {} //-V730 CellExt(Key && key_, const FixedHashMapImplicitZeroCell * ptr_) : key(key_), ptr(const_cast(ptr_)) {} void update(Key && key_, const FixedHashMapImplicitZeroCell * ptr_) { diff --git a/src/Common/HashTable/FixedHashTable.h b/src/Common/HashTable/FixedHashTable.h index e4715a6c1da..c1d2c8fe6ee 100644 --- a/src/Common/HashTable/FixedHashTable.h +++ b/src/Common/HashTable/FixedHashTable.h @@ -19,7 +19,7 @@ struct FixedHashTableCell using mapped_type = VoidMapped; bool full; - FixedHashTableCell() {} + FixedHashTableCell() {} //-V730 FixedHashTableCell(const Key &, const State &) : full(true) {} const VoidKey getKey() const { return {}; } @@ -267,7 +267,7 @@ public: DB::ReadBuffer & in; Cell cell; size_t read_count = 0; - size_t size; + size_t size = 0; bool is_eof = false; bool is_initialized = false; }; diff --git a/src/Common/HashTable/HashSet.h b/src/Common/HashTable/HashSet.h index 19dba189ddc..d06fc761362 100644 --- a/src/Common/HashTable/HashSet.h +++ b/src/Common/HashTable/HashSet.h @@ -73,8 +73,8 @@ struct HashSetCellWithSavedHash : public HashTableCell size_t saved_hash; - HashSetCellWithSavedHash() : Base() {} - HashSetCellWithSavedHash(const Key & key_, const typename Base::State & state) : Base(key_, state) {} + HashSetCellWithSavedHash() : Base() {} //-V730 + HashSetCellWithSavedHash(const Key & key_, const typename Base::State & state) : Base(key_, state) {} //-V730 bool keyEquals(const Key & key_) const { return bitEquals(this->key, key_); } bool keyEquals(const Key & key_, size_t hash_) const { return saved_hash == hash_ && bitEquals(this->key, key_); } diff --git a/src/Common/HashTable/HashTable.h b/src/Common/HashTable/HashTable.h index 0f394db5938..60271aabba7 100644 --- a/src/Common/HashTable/HashTable.h +++ b/src/Common/HashTable/HashTable.h @@ -305,7 +305,7 @@ template struct ZeroValueStorage; template -struct ZeroValueStorage +struct ZeroValueStorage //-V308 { private: bool has_zero = false; diff --git a/src/Common/HashTable/SmallTable.h b/src/Common/HashTable/SmallTable.h index 25f29ae869e..895d2adc83e 100644 --- a/src/Common/HashTable/SmallTable.h +++ b/src/Common/HashTable/SmallTable.h @@ -80,7 +80,7 @@ public: { public: Reader(DB::ReadBuffer & in_) - : in(in_) + : in(in_) { } @@ -124,15 +124,15 @@ public: DB::ReadBuffer & in; Cell cell; size_t read_count = 0; - size_t size; + size_t size = 0; bool is_eof = false; bool is_initialized = false; }; class iterator { - Self * container; - Cell * ptr; + Self * container = nullptr; + Cell * ptr = nullptr; friend class SmallTable; @@ -158,8 +158,8 @@ public: class const_iterator { - const Self * container; - const Cell * ptr; + const Self * container = nullptr; + const Cell * ptr = nullptr; friend class SmallTable; @@ -184,16 +184,16 @@ public: }; - const_iterator begin() const { return iteratorTo(buf); } - iterator begin() { return iteratorTo(buf); } + const_iterator begin() const { return iteratorTo(buf); } + iterator begin() { return iteratorTo(buf); } - const_iterator end() const { return iteratorTo(buf + m_size); } - iterator end() { return iteratorTo(buf + m_size); } + const_iterator end() const { return iteratorTo(buf + m_size); } + iterator end() { return iteratorTo(buf + m_size); } protected: - const_iterator iteratorTo(const Cell * ptr) const { return const_iterator(this, ptr); } - iterator iteratorTo(Cell * ptr) { return iterator(this, ptr); } + const_iterator iteratorTo(const Cell * ptr) const { return const_iterator(this, ptr); } + iterator iteratorTo(Cell * ptr) { return iterator(this, ptr); } public: diff --git a/src/Common/HashTable/StringHashTable.h b/src/Common/HashTable/StringHashTable.h index 9f91de5585b..3a96d0cefde 100644 --- a/src/Common/HashTable/StringHashTable.h +++ b/src/Common/HashTable/StringHashTable.h @@ -79,7 +79,7 @@ struct StringHashTableHash }; template -struct StringHashTableEmpty +struct StringHashTableEmpty //-V730 { using Self = StringHashTableEmpty; diff --git a/src/Common/HashTable/TwoLevelHashTable.h b/src/Common/HashTable/TwoLevelHashTable.h index 7eabb5fa138..14afb91c071 100644 --- a/src/Common/HashTable/TwoLevelHashTable.h +++ b/src/Common/HashTable/TwoLevelHashTable.h @@ -119,9 +119,9 @@ public: class iterator { - Self * container; - size_t bucket; - typename Impl::iterator current_it; + Self * container{}; + size_t bucket{}; + typename Impl::iterator current_it{}; friend class TwoLevelHashTable; @@ -156,9 +156,9 @@ public: class const_iterator { - Self * container; - size_t bucket; - typename Impl::const_iterator current_it; + Self * container{}; + size_t bucket{}; + typename Impl::const_iterator current_it{}; friend class TwoLevelHashTable; diff --git a/src/Common/MemoryTracker.h b/src/Common/MemoryTracker.h index 9475b555975..4dada6f4ef7 100644 --- a/src/Common/MemoryTracker.h +++ b/src/Common/MemoryTracker.h @@ -208,7 +208,7 @@ public: static bool isBlocked(VariableContext current_level, bool fault_injection) { - return counter > 0 && current_level >= level && (!fault_injection || (fault_injection && block_fault_injections)); + return counter > 0 && current_level >= level && (!fault_injection || block_fault_injections); } }; }; diff --git a/src/Common/SharedBlockRowRef.h b/src/Common/SharedBlockRowRef.h index 193f7e4dd05..46c35c6ac47 100644 --- a/src/Common/SharedBlockRowRef.h +++ b/src/Common/SharedBlockRowRef.h @@ -44,7 +44,7 @@ using SharedBlockPtr = boost::intrusive_ptr; struct SharedBlockRowRef { ColumnRawPtrs * columns = nullptr; - size_t row_num; + size_t row_num = 0; SharedBlockPtr shared_block; void swap(SharedBlockRowRef & other) diff --git a/src/Common/ThreadStatus.h b/src/Common/ThreadStatus.h index 3b39e462fa6..6fc43114621 100644 --- a/src/Common/ThreadStatus.h +++ b/src/Common/ThreadStatus.h @@ -71,7 +71,7 @@ public: LogsLevel client_logs_level = LogsLevel::none; String query; - UInt64 normalized_query_hash; + UInt64 normalized_query_hash = 0; }; using ThreadGroupStatusPtr = std::shared_ptr; diff --git a/src/Common/ZooKeeper/ZooKeeperCommon.h b/src/Common/ZooKeeper/ZooKeeperCommon.h index dd95eaa6b67..2a317da2622 100644 --- a/src/Common/ZooKeeper/ZooKeeperCommon.h +++ b/src/Common/ZooKeeper/ZooKeeperCommon.h @@ -25,7 +25,7 @@ namespace Coordination struct ZooKeeperResponse : virtual Response { XID xid = 0; - int64_t zxid; + int64_t zxid = 0; virtual ~ZooKeeperResponse() override = default; virtual void readImpl(ReadBuffer &) = 0; diff --git a/src/Coordination/Changelog.h b/src/Coordination/Changelog.h index d669f56aded..856e6035164 100644 --- a/src/Coordination/Changelog.h +++ b/src/Coordination/Changelog.h @@ -30,10 +30,10 @@ static constexpr auto CURRENT_CHANGELOG_VERSION = ChangelogVersion::V0; struct ChangelogRecordHeader { ChangelogVersion version = CURRENT_CHANGELOG_VERSION; - uint64_t index; /// entry log number - uint64_t term; + uint64_t index = 0; /// entry log number + uint64_t term = 0; nuraft::log_val_type value_type; - uint64_t blob_size; + uint64_t blob_size = 0; }; /// Changelog record on disk diff --git a/src/Core/PostgreSQLProtocol.h b/src/Core/PostgreSQLProtocol.h index 4c41ead7a02..9e1afcd187c 100644 --- a/src/Core/PostgreSQLProtocol.h +++ b/src/Core/PostgreSQLProtocol.h @@ -257,6 +257,7 @@ class FirstMessage : public FrontMessage { public: Int32 payload_size; + FirstMessage() = delete; FirstMessage(int payload_size_) : payload_size(payload_size_) {} }; @@ -264,8 +265,9 @@ public: class CancelRequest : public FirstMessage { public: - Int32 process_id; - Int32 secret_key; + Int32 process_id = 0; + Int32 secret_key = 0; + CancelRequest(int payload_size_) : FirstMessage(payload_size_) {} void deserialize(ReadBuffer & in) override diff --git a/src/Core/SortCursor.h b/src/Core/SortCursor.h index 79730e9697e..dd804bd4675 100644 --- a/src/Core/SortCursor.h +++ b/src/Core/SortCursor.h @@ -126,7 +126,7 @@ struct SortCursorImpl /// Prevent using pos instead of getRow() private: - size_t pos; + size_t pos = 0; }; using SortCursorImpls = std::vector; diff --git a/src/DataTypes/tests/gtest_data_type_get_common_type.cpp b/src/DataTypes/tests/gtest_data_type_get_common_type.cpp index 5c258f0b42e..2a77237e982 100644 --- a/src/DataTypes/tests/gtest_data_type_get_common_type.cpp +++ b/src/DataTypes/tests/gtest_data_type_get_common_type.cpp @@ -37,7 +37,7 @@ static auto typesFromString(const std::string & str) struct TypesTestCase { - const char * from_types; + const char * from_types = nullptr; const char * expected_type = nullptr; }; diff --git a/src/Functions/isDecimalOverflow.cpp b/src/Functions/isDecimalOverflow.cpp index 16d6752e6a5..0cac3675d12 100644 --- a/src/Functions/isDecimalOverflow.cpp +++ b/src/Functions/isDecimalOverflow.cpp @@ -85,7 +85,7 @@ public: auto result_column = ColumnUInt8::create(); - auto call = [&](const auto & types) -> bool + auto call = [&](const auto & types) -> bool //-V567 { using Types = std::decay_t; using Type = typename Types::RightType; diff --git a/src/Interpreters/ActionsDAG.h b/src/Interpreters/ActionsDAG.h index 645f1aaeeb7..049cce69da3 100644 --- a/src/Interpreters/ActionsDAG.h +++ b/src/Interpreters/ActionsDAG.h @@ -73,7 +73,7 @@ public: { NodeRawConstPtrs children; - ActionType type; + ActionType type{}; std::string result_name; DataTypePtr result_type; diff --git a/src/Interpreters/DDLTask.h b/src/Interpreters/DDLTask.h index 874a29f051d..5fafb42bfd2 100644 --- a/src/Interpreters/DDLTask.h +++ b/src/Interpreters/DDLTask.h @@ -126,8 +126,8 @@ private: String cluster_name; ClusterPtr cluster; Cluster::Address address_in_cluster; - size_t host_shard_num; - size_t host_replica_num; + size_t host_shard_num = 0; + size_t host_replica_num = 0; }; struct DatabaseReplicatedTask : public DDLTaskBase diff --git a/src/Interpreters/DatabaseCatalog.h b/src/Interpreters/DatabaseCatalog.h index 388079b54f2..4fa6c5c0a10 100644 --- a/src/Interpreters/DatabaseCatalog.h +++ b/src/Interpreters/DatabaseCatalog.h @@ -234,7 +234,7 @@ private: StorageID table_id = StorageID::createEmpty(); StoragePtr table; String metadata_path; - time_t drop_time; + time_t drop_time{}; }; using TablesMarkedAsDropped = std::list; diff --git a/src/Interpreters/TextLog.h b/src/Interpreters/TextLog.h index 7ff55128a90..0133d5e4eb6 100644 --- a/src/Interpreters/TextLog.h +++ b/src/Interpreters/TextLog.h @@ -10,10 +10,10 @@ struct TextLogElement { time_t event_time{}; Decimal64 event_time_microseconds{}; - UInt32 microseconds; + UInt32 microseconds{}; String thread_name; - UInt64 thread_id; + UInt64 thread_id{}; Message::Priority level = Message::PRIO_TRACE; @@ -22,7 +22,7 @@ struct TextLogElement String message; String source_file; - UInt64 source_line; + UInt64 source_line{}; static std::string name() { return "TextLog"; } static Block createBlock(); diff --git a/src/Interpreters/sortBlock.h b/src/Interpreters/sortBlock.h index a3b0a10b8f9..faf9384901b 100644 --- a/src/Interpreters/sortBlock.h +++ b/src/Interpreters/sortBlock.h @@ -32,7 +32,7 @@ bool isAlreadySorted(const Block & block, const SortDescription & description); /// Column with description for sort struct ColumnWithSortDescription { - const IColumn * column; + const IColumn * column = nullptr; SortColumnDescription description; /// It means, that this column is ColumnConst diff --git a/src/Processors/Executors/PollingQueue.h b/src/Processors/Executors/PollingQueue.h index 0d306ddf2f7..100d762b731 100644 --- a/src/Processors/Executors/PollingQueue.h +++ b/src/Processors/Executors/PollingQueue.h @@ -17,7 +17,7 @@ class PollingQueue public: struct TaskData { - size_t thread_num; + size_t thread_num = 0; void * data = nullptr; int fd = -1; diff --git a/src/Storages/AlterCommands.cpp b/src/Storages/AlterCommands.cpp index e3177c167c5..249b81fab3d 100644 --- a/src/Storages/AlterCommands.cpp +++ b/src/Storages/AlterCommands.cpp @@ -784,8 +784,10 @@ String alterTypeToString(const AlterCommand::Type type) return "RENAME COLUMN"; case AlterCommand::Type::REMOVE_TTL: return "REMOVE TTL"; + default: + throw Exception("Uninitialized ALTER command", ErrorCodes::LOGICAL_ERROR); } - __builtin_unreachable(); + } void AlterCommands::apply(StorageInMemoryMetadata & metadata, ContextPtr context) const diff --git a/src/Storages/AlterCommands.h b/src/Storages/AlterCommands.h index d6c80bc5ed4..c01536116b2 100644 --- a/src/Storages/AlterCommands.h +++ b/src/Storages/AlterCommands.h @@ -23,6 +23,7 @@ struct AlterCommand enum Type { + UNKNOWN, ADD_COLUMN, DROP_COLUMN, MODIFY_COLUMN, @@ -55,7 +56,7 @@ struct AlterCommand TTL }; - Type type; + Type type = UNKNOWN; String column_name; diff --git a/src/Storages/MergeTree/MergeSelector.h b/src/Storages/MergeTree/MergeSelector.h index 5a92b4c5dd6..aac805823a9 100644 --- a/src/Storages/MergeTree/MergeSelector.h +++ b/src/Storages/MergeTree/MergeSelector.h @@ -31,16 +31,16 @@ public: struct Part { /// Size of data part in bytes. - size_t size; + size_t size = 0; /// How old this data part in seconds. - time_t age; + time_t age = 0; /// Depth of tree of merges by which this part was created. New parts has zero level. - unsigned level; + unsigned level = 0; /// Opaque pointer to avoid dependencies (it is not possible to do forward declaration of typedef). - const void * data; + const void * data = nullptr; /// Information about different TTLs for part. Can be used by /// TTLSelector to assign merges with TTL. diff --git a/src/Storages/MergeTree/MergeTreeData.cpp b/src/Storages/MergeTree/MergeTreeData.cpp index f677f745080..dad7f021491 100644 --- a/src/Storages/MergeTree/MergeTreeData.cpp +++ b/src/Storages/MergeTree/MergeTreeData.cpp @@ -3022,6 +3022,9 @@ Pipe MergeTreeData::alterPartition( } break; + + default: + throw Exception("Uninitialized partition command", ErrorCodes::LOGICAL_ERROR); } for (auto & command_result : current_command_results) command_result.command_type = command.typeToString(); diff --git a/src/Storages/MergeTree/SimpleMergeSelector.cpp b/src/Storages/MergeTree/SimpleMergeSelector.cpp index 972c6ea6ecb..8ab01fb4b6c 100644 --- a/src/Storages/MergeTree/SimpleMergeSelector.cpp +++ b/src/Storages/MergeTree/SimpleMergeSelector.cpp @@ -3,6 +3,7 @@ #include #include +#include #include @@ -168,6 +169,7 @@ void selectWithinPartition( for (size_t end = begin + 2; end <= parts_count; ++end) { + assert(end > begin); if (settings.max_parts_to_merge_at_once && end - begin > settings.max_parts_to_merge_at_once) break; diff --git a/src/Storages/PartitionCommands.cpp b/src/Storages/PartitionCommands.cpp index f09f60887e8..7cee3a59345 100644 --- a/src/Storages/PartitionCommands.cpp +++ b/src/Storages/PartitionCommands.cpp @@ -13,6 +13,12 @@ namespace DB { +namespace ErrorCodes +{ + extern const int LOGICAL_ERROR; +} + + std::optional PartitionCommand::parse(const ASTAlterCommand * command_ast) { if (command_ast->type == ASTAlterCommand::DROP_PARTITION) @@ -155,8 +161,9 @@ std::string PartitionCommand::typeToString() const return "UNFREEZE ALL"; case PartitionCommand::Type::REPLACE_PARTITION: return "REPLACE PARTITION"; + default: + throw Exception("Uninitialized partition command", ErrorCodes::LOGICAL_ERROR); } - __builtin_unreachable(); } Pipe convertCommandsResultToSource(const PartitionCommandsResultInfo & commands_result) diff --git a/src/Storages/PartitionCommands.h b/src/Storages/PartitionCommands.h index 9f89d44bd4e..b1ff77a758b 100644 --- a/src/Storages/PartitionCommands.h +++ b/src/Storages/PartitionCommands.h @@ -20,6 +20,8 @@ struct PartitionCommand { enum Type { + UNKNOWN, + ATTACH_PARTITION, MOVE_PARTITION, DROP_PARTITION, @@ -32,7 +34,7 @@ struct PartitionCommand REPLACE_PARTITION, }; - Type type; + Type type = UNKNOWN; ASTPtr partition; From 3025f9e141fca2e247048498799378e56e35a7d1 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sat, 8 May 2021 18:35:09 +0300 Subject: [PATCH 10/26] More fixes for PVS-Studio --- src/Compression/tests/gtest_compressionCodec.cpp | 2 +- src/Databases/DatabaseAtomic.cpp | 2 +- src/Dictionaries/SSDCacheDictionaryStorage.h | 3 +++ src/IO/MySQLBinlogEventReadBuffer.h | 4 ++-- src/IO/ReadBufferAIO.cpp | 2 +- src/Interpreters/ActionsVisitor.cpp | 2 ++ src/Processors/Formats/IRowInputFormat.h | 6 +++--- src/Processors/Formats/Impl/CapnProtoRowInputFormat.h | 6 +++--- src/Processors/QueryPlan/Optimizations/Optimizations.h | 4 ++-- src/Processors/QueryPlan/Optimizations/optimizeTree.cpp | 2 +- src/Processors/QueryPlan/QueryPlan.cpp | 8 ++++---- src/Processors/Transforms/AggregatingTransform.cpp | 2 +- src/Server/PostgreSQLHandler.h | 6 +++--- src/Storages/RabbitMQ/ReadBufferFromRabbitMQConsumer.h | 6 +++--- 14 files changed, 30 insertions(+), 25 deletions(-) diff --git a/src/Compression/tests/gtest_compressionCodec.cpp b/src/Compression/tests/gtest_compressionCodec.cpp index 0acd15e48c3..bfcd1d05607 100644 --- a/src/Compression/tests/gtest_compressionCodec.cpp +++ b/src/Compression/tests/gtest_compressionCodec.cpp @@ -1129,7 +1129,7 @@ template auto DDCompatibilityTestSequence() { // Generates sequences with double delta in given range. - auto dd_generator = [prev_delta = static_cast(0), prev = static_cast(0)](auto dd) mutable + auto dd_generator = [prev_delta = static_cast(0), prev = static_cast(0)](auto dd) mutable //-V788 { const auto curr = dd + prev + prev_delta; prev = curr; diff --git a/src/Databases/DatabaseAtomic.cpp b/src/Databases/DatabaseAtomic.cpp index 9b9ea572c3d..e939f76f372 100644 --- a/src/Databases/DatabaseAtomic.cpp +++ b/src/Databases/DatabaseAtomic.cpp @@ -102,7 +102,7 @@ StoragePtr DatabaseAtomic::detachTable(const String & name) auto table = DatabaseOrdinary::detachTableUnlocked(name, lock); table_name_to_path.erase(name); detached_tables.emplace(table->getStorageID().uuid, table); - not_in_use = cleanupDetachedTables(); + not_in_use = cleanupDetachedTables(); //-V1001 return table; } diff --git a/src/Dictionaries/SSDCacheDictionaryStorage.h b/src/Dictionaries/SSDCacheDictionaryStorage.h index 7d72beca35e..fcfb5803a18 100644 --- a/src/Dictionaries/SSDCacheDictionaryStorage.h +++ b/src/Dictionaries/SSDCacheDictionaryStorage.h @@ -744,6 +744,9 @@ private: FileDescriptor & operator=(FileDescriptor && rhs) { + if (this == &rhs) + return *this; + close(fd); fd = rhs.fd; diff --git a/src/IO/MySQLBinlogEventReadBuffer.h b/src/IO/MySQLBinlogEventReadBuffer.h index d31e19d82b8..7212a548844 100644 --- a/src/IO/MySQLBinlogEventReadBuffer.h +++ b/src/IO/MySQLBinlogEventReadBuffer.h @@ -11,11 +11,11 @@ protected: static const size_t MAX_CHECKSUM_SIGNATURE_LENGTH = 4; ReadBuffer & in; - size_t checksum_signature_length; + size_t checksum_signature_length = 0; size_t checksum_buff_size = 0; size_t checksum_buff_limit = 0; - char checksum_buf[MAX_CHECKSUM_SIGNATURE_LENGTH]; + char checksum_buf[MAX_CHECKSUM_SIGNATURE_LENGTH]{}; bool nextImpl() override; diff --git a/src/IO/ReadBufferAIO.cpp b/src/IO/ReadBufferAIO.cpp index 39e3c26826a..c064e0d4ed9 100644 --- a/src/IO/ReadBufferAIO.cpp +++ b/src/IO/ReadBufferAIO.cpp @@ -106,7 +106,7 @@ bool ReadBufferAIO::nextImpl() ProfileInfo info; info.bytes_requested = requested_byte_count; info.bytes_read = bytes_read; - info.nanoseconds = watch->elapsed(); + info.nanoseconds = watch->elapsed(); //-V1007 profile_callback(info); } diff --git a/src/Interpreters/ActionsVisitor.cpp b/src/Interpreters/ActionsVisitor.cpp index b7d679a81f1..b463ab0a586 100644 --- a/src/Interpreters/ActionsVisitor.cpp +++ b/src/Interpreters/ActionsVisitor.cpp @@ -205,6 +205,8 @@ static Block createBlockFromAST(const ASTPtr & node, const DataTypes & types, Co tuple = &literal->value.get(); } + assert(tuple || func); + size_t tuple_size = tuple ? tuple->size() : func->arguments->children.size(); if (tuple_size != num_columns) throw Exception("Incorrect size of tuple in set: " + toString(tuple_size) + " instead of " + toString(num_columns), diff --git a/src/Processors/Formats/IRowInputFormat.h b/src/Processors/Formats/IRowInputFormat.h index 8c600ad7285..2ca182b7ffe 100644 --- a/src/Processors/Formats/IRowInputFormat.h +++ b/src/Processors/Formats/IRowInputFormat.h @@ -22,10 +22,10 @@ struct RowReadExtension /// Common parameters for generating blocks. struct RowInputFormatParams { - size_t max_block_size; + size_t max_block_size = 0; - UInt64 allow_errors_num; - Float64 allow_errors_ratio; + UInt64 allow_errors_num = 0; + Float64 allow_errors_ratio = 0; Poco::Timespan max_execution_time = 0; OverflowMode timeout_overflow_mode = OverflowMode::THROW; diff --git a/src/Processors/Formats/Impl/CapnProtoRowInputFormat.h b/src/Processors/Formats/Impl/CapnProtoRowInputFormat.h index 51ed451417c..0957cd1d681 100644 --- a/src/Processors/Formats/Impl/CapnProtoRowInputFormat.h +++ b/src/Processors/Formats/Impl/CapnProtoRowInputFormat.h @@ -50,9 +50,9 @@ private: struct Action { enum Type { POP, PUSH, READ }; - Type type; - capnp::StructSchema::Field field = {}; - BlockPositionList columns = {}; + Type type{}; + capnp::StructSchema::Field field{}; + BlockPositionList columns{}; }; // Wrapper for classes that could throw in destructor diff --git a/src/Processors/QueryPlan/Optimizations/Optimizations.h b/src/Processors/QueryPlan/Optimizations/Optimizations.h index 7e946a71fad..10bc6293537 100644 --- a/src/Processors/QueryPlan/Optimizations/Optimizations.h +++ b/src/Processors/QueryPlan/Optimizations/Optimizations.h @@ -23,8 +23,8 @@ struct Optimization { using Function = size_t (*)(QueryPlan::Node *, QueryPlan::Nodes &); const Function apply = nullptr; - const char * name; - const bool QueryPlanOptimizationSettings::* const is_enabled; + const char * name = ""; + const bool QueryPlanOptimizationSettings::* const is_enabled{}; }; /// Move ARRAY JOIN up if possible. diff --git a/src/Processors/QueryPlan/Optimizations/optimizeTree.cpp b/src/Processors/QueryPlan/Optimizations/optimizeTree.cpp index da9b1e26f68..9a8dd151830 100644 --- a/src/Processors/QueryPlan/Optimizations/optimizeTree.cpp +++ b/src/Processors/QueryPlan/Optimizations/optimizeTree.cpp @@ -23,7 +23,7 @@ void optimizeTree(const QueryPlanOptimizationSettings & settings, QueryPlan::Nod struct Frame { - QueryPlan::Node * node; + QueryPlan::Node * node = nullptr; /// If not zero, traverse only depth_limit layers of tree (if no other optimizations happen). /// Otherwise, traverse all children. diff --git a/src/Processors/QueryPlan/QueryPlan.cpp b/src/Processors/QueryPlan/QueryPlan.cpp index 3e46adb9d9c..d04d98c9d97 100644 --- a/src/Processors/QueryPlan/QueryPlan.cpp +++ b/src/Processors/QueryPlan/QueryPlan.cpp @@ -142,7 +142,7 @@ QueryPipelinePtr QueryPlan::buildQueryPipeline( struct Frame { - Node * node; + Node * node = {}; QueryPipelines pipelines = {}; }; @@ -242,7 +242,7 @@ JSONBuilder::ItemPtr QueryPlan::explainPlan(const ExplainPlanOptions & options) struct Frame { - Node * node; + Node * node = {}; size_t next_child = 0; std::unique_ptr node_map = {}; std::unique_ptr children_array = {}; @@ -352,7 +352,7 @@ void QueryPlan::explainPlan(WriteBuffer & buffer, const ExplainPlanOptions & opt struct Frame { - Node * node; + Node * node = {}; bool is_description_printed = false; size_t next_child = 0; }; @@ -398,7 +398,7 @@ void QueryPlan::explainPipeline(WriteBuffer & buffer, const ExplainPipelineOptio struct Frame { - Node * node; + Node * node = {}; size_t offset = 0; bool is_description_printed = false; size_t next_child = 0; diff --git a/src/Processors/Transforms/AggregatingTransform.cpp b/src/Processors/Transforms/AggregatingTransform.cpp index 3400d06dae3..77e603103a6 100644 --- a/src/Processors/Transforms/AggregatingTransform.cpp +++ b/src/Processors/Transforms/AggregatingTransform.cpp @@ -95,7 +95,7 @@ public: struct SharedData { std::atomic next_bucket_to_merge = 0; - std::array, NUM_BUCKETS> is_bucket_processed; + std::array, NUM_BUCKETS> is_bucket_processed{}; std::atomic is_cancelled = false; SharedData() diff --git a/src/Server/PostgreSQLHandler.h b/src/Server/PostgreSQLHandler.h index cc30c85d8bb..0f114d388fb 100644 --- a/src/Server/PostgreSQLHandler.h +++ b/src/Server/PostgreSQLHandler.h @@ -38,9 +38,9 @@ private: IServer & server; ContextPtr connection_context; - bool ssl_enabled; - Int32 connection_id; - Int32 secret_key; + bool ssl_enabled = false; + Int32 connection_id = 0; + Int32 secret_key = 0; std::shared_ptr in; std::shared_ptr out; diff --git a/src/Storages/RabbitMQ/ReadBufferFromRabbitMQConsumer.h b/src/Storages/RabbitMQ/ReadBufferFromRabbitMQConsumer.h index 23e1d3f0649..9b8a73fc4fb 100644 --- a/src/Storages/RabbitMQ/ReadBufferFromRabbitMQConsumer.h +++ b/src/Storages/RabbitMQ/ReadBufferFromRabbitMQConsumer.h @@ -48,9 +48,9 @@ public: { String message; String message_id; - uint64_t timestamp; - bool redelivered; - AckTracker track; + uint64_t timestamp = 0; + bool redelivered = false; + AckTracker track{}; }; ChannelPtr & getChannel() { return consumer_channel; } From f805f4aa4f4dcc2c3f31b367929ca787abd3a58d Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sat, 8 May 2021 18:35:38 +0300 Subject: [PATCH 11/26] Fix error found by PVS-Studio --- src/Storages/System/StorageSystemNumbers.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Storages/System/StorageSystemNumbers.cpp b/src/Storages/System/StorageSystemNumbers.cpp index f8a0e94bf98..545f2c8be9a 100644 --- a/src/Storages/System/StorageSystemNumbers.cpp +++ b/src/Storages/System/StorageSystemNumbers.cpp @@ -144,7 +144,7 @@ Pipe StorageSystemNumbers::read( Pipe pipe; - if (num_streams > 1 && !even_distribution && *limit) + if (num_streams > 1 && !even_distribution && limit) { auto state = std::make_shared(offset); UInt64 max_counter = offset + *limit; From ad88819ee4e8f65caf6f2cd343a2402d96e0e00b Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sat, 8 May 2021 19:09:17 +0300 Subject: [PATCH 12/26] Fix a bunch of warnings from PVS-Studio --- base/loggers/Loggers.cpp | 2 +- src/Access/ExternalAuthenticators.cpp | 6 +- src/Access/GrantedRoles.cpp | 2 +- .../AggregateFunctionStatistics.h | 6 +- src/Common/HyperLogLogCounter.h | 10 +- src/Common/PoolWithFailoverBase.h | 2 +- src/Common/ZooKeeper/TestKeeper.cpp | 16 +- src/Common/filesystemHelpers.cpp | 1 - .../tests/gtest_compressionCodec.cpp | 47 ++--- src/DataStreams/IBlockInputStream.h | 2 +- src/DataStreams/NativeBlockOutputStream.cpp | 2 +- .../PostgreSQLBlockInputStream.cpp | 3 +- .../gtest_DataType_deserializeAsText.cpp | 4 - src/Dictionaries/Embedded/RegionsNames.h | 2 +- src/Dictionaries/MySQLDictionarySource.cpp | 2 +- .../PostgreSQLDictionarySource.cpp | 2 +- src/Dictionaries/XDBCDictionarySource.cpp | 2 +- src/Functions/FunctionBase64Conversion.h | 2 +- src/Functions/isValidUTF8.cpp | 2 +- .../ConvertStringsToEnumVisitor.cpp | 186 +++++++++++++++++ .../ConvertStringsToEnumVisitor.h | 194 ++---------------- src/Interpreters/InterpreterWatchQuery.cpp | 2 +- src/Interpreters/PartLog.cpp | 2 +- src/Interpreters/executeQuery.cpp | 4 +- src/Interpreters/join_common.cpp | 2 +- src/Parsers/ParserExplainQuery.cpp | 2 +- .../PipelineExecutingBlockInputStream.cpp | 7 - .../PipelineExecutingBlockInputStream.h | 1 - src/Processors/QueryPipeline.h | 2 +- .../QueryPlan/PartialSortingStep.cpp | 2 +- src/Processors/QueryPlan/QueryPlan.cpp | 2 +- src/Server/TCPHandler.cpp | 2 +- src/Storages/MergeTree/MergeTreeData.cpp | 2 +- .../MergeTreeDataPartWriterCompact.cpp | 4 +- src/Storages/MergeTree/MergeTreeIndexSet.cpp | 2 +- src/Storages/RabbitMQ/StorageRabbitMQ.cpp | 4 +- 36 files changed, 260 insertions(+), 275 deletions(-) create mode 100644 src/Interpreters/ConvertStringsToEnumVisitor.cpp diff --git a/base/loggers/Loggers.cpp b/base/loggers/Loggers.cpp index f66b1eb5b91..4a66d43606b 100644 --- a/base/loggers/Loggers.cpp +++ b/base/loggers/Loggers.cpp @@ -40,7 +40,7 @@ void Loggers::buildLoggers(Poco::Util::AbstractConfiguration & config, Poco::Log split->addTextLog(log, text_log_max_priority); auto current_logger = config.getString("logger", ""); - if (config_logger == current_logger) + if (config_logger == current_logger) //-V1051 return; config_logger = current_logger; diff --git a/src/Access/ExternalAuthenticators.cpp b/src/Access/ExternalAuthenticators.cpp index 1cade973724..0c4d2f417c9 100644 --- a/src/Access/ExternalAuthenticators.cpp +++ b/src/Access/ExternalAuthenticators.cpp @@ -77,7 +77,7 @@ auto parseLDAPServer(const Poco::Util::AbstractConfiguration & config, const Str if (enable_tls_lc_str == "starttls") params.enable_tls = LDAPClient::Params::TLSEnable::YES_STARTTLS; else if (config.getBool(ldap_server_config + ".enable_tls")) - params.enable_tls = LDAPClient::Params::TLSEnable::YES; + params.enable_tls = LDAPClient::Params::TLSEnable::YES; //-V1048 else params.enable_tls = LDAPClient::Params::TLSEnable::NO; } @@ -96,7 +96,7 @@ auto parseLDAPServer(const Poco::Util::AbstractConfiguration & config, const Str else if (tls_minimum_protocol_version_lc_str == "tls1.1") params.tls_minimum_protocol_version = LDAPClient::Params::TLSProtocolVersion::TLS1_1; else if (tls_minimum_protocol_version_lc_str == "tls1.2") - params.tls_minimum_protocol_version = LDAPClient::Params::TLSProtocolVersion::TLS1_2; + params.tls_minimum_protocol_version = LDAPClient::Params::TLSProtocolVersion::TLS1_2; //-V1048 else throw Exception("Bad value for 'tls_minimum_protocol_version' entry, allowed values are: 'ssl2', 'ssl3', 'tls1.0', 'tls1.1', 'tls1.2'", ErrorCodes::BAD_ARGUMENTS); } @@ -113,7 +113,7 @@ auto parseLDAPServer(const Poco::Util::AbstractConfiguration & config, const Str else if (tls_require_cert_lc_str == "try") params.tls_require_cert = LDAPClient::Params::TLSRequireCert::TRY; else if (tls_require_cert_lc_str == "demand") - params.tls_require_cert = LDAPClient::Params::TLSRequireCert::DEMAND; + params.tls_require_cert = LDAPClient::Params::TLSRequireCert::DEMAND; //-V1048 else throw Exception("Bad value for 'tls_require_cert' entry, allowed values are: 'never', 'allow', 'try', 'demand'", ErrorCodes::BAD_ARGUMENTS); } diff --git a/src/Access/GrantedRoles.cpp b/src/Access/GrantedRoles.cpp index 7930b56e44d..2659f8a3ec9 100644 --- a/src/Access/GrantedRoles.cpp +++ b/src/Access/GrantedRoles.cpp @@ -136,7 +136,7 @@ GrantedRoles::Elements GrantedRoles::getElements() const boost::range::set_difference(roles, roles_with_admin_option, std::back_inserter(element.ids)); if (!element.empty()) { - element.admin_option = false; + element.admin_option = false; //-V1048 elements.emplace_back(std::move(element)); } diff --git a/src/AggregateFunctions/AggregateFunctionStatistics.h b/src/AggregateFunctions/AggregateFunctionStatistics.h index 473bce1d89a..2b778f85d99 100644 --- a/src/AggregateFunctions/AggregateFunctionStatistics.h +++ b/src/AggregateFunctions/AggregateFunctionStatistics.h @@ -13,7 +13,7 @@ namespace DB { -namespace +namespace detail { /// This function returns true if both values are large and comparable. @@ -72,7 +72,7 @@ public: Float64 factor = static_cast(count * source.count) / total_count; Float64 delta = mean - source.mean; - if (areComparable(count, source.count)) + if (detail::areComparable(count, source.count)) mean = (source.count * source.mean + count * mean) / total_count; else mean = source.mean + delta * (static_cast(count) / total_count); @@ -302,7 +302,7 @@ public: Float64 left_delta = left_mean - source.left_mean; Float64 right_delta = right_mean - source.right_mean; - if (areComparable(count, source.count)) + if (detail::areComparable(count, source.count)) { left_mean = (source.count * source.left_mean + count * left_mean) / total_count; right_mean = (source.count * source.right_mean + count * right_mean) / total_count; diff --git a/src/Common/HyperLogLogCounter.h b/src/Common/HyperLogLogCounter.h index 0acffaaaf33..4f44e0c3c55 100644 --- a/src/Common/HyperLogLogCounter.h +++ b/src/Common/HyperLogLogCounter.h @@ -82,17 +82,12 @@ template class __attribute__ ((packed)) Denominator; -namespace -{ - /// Returns true if rank storage is big. constexpr bool isBigRankStore(UInt8 precision) { return precision >= 12; } -} - /// Used to deduce denominator type depending on options provided. template struct IntermediateDenominator; @@ -120,7 +115,7 @@ struct IntermediateDenominator -class __attribute__ ((packed)) Denominator> { @@ -164,7 +159,7 @@ private: /// Used when rank storage is big. template -class __attribute__ ((packed)) Denominator> { @@ -252,6 +247,7 @@ struct RankWidth } + /// Sets behavior of HyperLogLog class. enum class HyperLogLogMode { diff --git a/src/Common/PoolWithFailoverBase.h b/src/Common/PoolWithFailoverBase.h index 141ac7a7e49..78d45a93b13 100644 --- a/src/Common/PoolWithFailoverBase.h +++ b/src/Common/PoolWithFailoverBase.h @@ -388,7 +388,7 @@ void PoolWithFailoverBase::updateErrorCounts(PoolWithFailoverBase TestKeeperCreateRequest::process(TestKeeper::Contai else { TestKeeper::Node created_node; - created_node.seq_num = 0; + created_node.seq_num = 0; //-V1044 created_node.stat.czxid = zxid; created_node.stat.mzxid = zxid; created_node.stat.ctime = std::chrono::system_clock::now().time_since_epoch() / std::chrono::milliseconds(1); @@ -271,7 +271,7 @@ std::pair TestKeeperRemoveRequest::process(TestKeeper::Contai auto & parent = container.at(parentPath(path)); --parent.stat.numChildren; ++parent.stat.cversion; - response.error = Error::ZOK; + response.error = Error::ZOK; //-V1044 undo = [prev_node, &container, path = path] { @@ -293,7 +293,7 @@ std::pair TestKeeperExistsRequest::process(TestKeeper::Contai if (it != container.end()) { response.stat = it->second.stat; - response.error = Error::ZOK; + response.error = Error::ZOK; //-V1044 } else { @@ -316,7 +316,7 @@ std::pair TestKeeperGetRequest::process(TestKeeper::Container { response.stat = it->second.stat; response.data = it->second.data; - response.error = Error::ZOK; + response.error = Error::ZOK; //-V1044 } return { std::make_shared(response), {} }; @@ -343,7 +343,7 @@ std::pair TestKeeperSetRequest::process(TestKeeper::Container it->second.data = data; ++container.at(parentPath(path)).stat.cversion; response.stat = it->second.stat; - response.error = Error::ZOK; + response.error = Error::ZOK; //-V1044 undo = [prev_node, &container, path = path] { @@ -387,7 +387,7 @@ std::pair TestKeeperListRequest::process(TestKeeper::Containe } response.stat = it->second.stat; - response.error = Error::ZOK; + response.error = Error::ZOK; //-V1044 } return { std::make_shared(response), {} }; @@ -407,7 +407,7 @@ std::pair TestKeeperCheckRequest::process(TestKeeper::Contain } else { - response.error = Error::ZOK; + response.error = Error::ZOK; //-V1044 } return { std::make_shared(response), {} }; @@ -422,7 +422,7 @@ std::pair TestKeeperMultiRequest::process(TestKeeper::Contain try { auto request_it = requests.begin(); - response.error = Error::ZOK; + response.error = Error::ZOK; //-V1044 while (request_it != requests.end()) { const TestKeeperRequest & concrete_request = dynamic_cast(**request_it); diff --git a/src/Common/filesystemHelpers.cpp b/src/Common/filesystemHelpers.cpp index 51a66fba3aa..3d8eec63471 100644 --- a/src/Common/filesystemHelpers.cpp +++ b/src/Common/filesystemHelpers.cpp @@ -79,7 +79,6 @@ std::filesystem::path getMountPoint(std::filesystem::path absolute_path) if (device_id != parent_device_id) return absolute_path; absolute_path = parent; - device_id = parent_device_id; } return absolute_path; diff --git a/src/Compression/tests/gtest_compressionCodec.cpp b/src/Compression/tests/gtest_compressionCodec.cpp index bfcd1d05607..20fe5476807 100644 --- a/src/Compression/tests/gtest_compressionCodec.cpp +++ b/src/Compression/tests/gtest_compressionCodec.cpp @@ -35,29 +35,6 @@ using namespace DB; -namespace std -{ -template -std::ostream & operator<<(std::ostream & ostr, const std::optional & opt) -{ - if (!opt) - { - return ostr << ""; - } - - return ostr << *opt; -} - -template -std::vector operator+(std::vector && left, std::vector && right) -{ - std::vector result(std::move(left)); - std::move(std::begin(right), std::end(right), std::back_inserter(result)); - - return result; -} - -} namespace { @@ -337,6 +314,14 @@ CodecTestSequence operator+(CodecTestSequence && left, const CodecTestSequence & return left.append(right); } +std::vector operator+(const std::vector & left, const std::vector & right) +{ + std::vector result(std::move(left)); + std::move(std::begin(right), std::end(right), std::back_inserter(result)); + + return result; +} + template CodecTestSequence operator*(CodecTestSequence && left, T times) { @@ -362,7 +347,7 @@ std::ostream & operator<<(std::ostream & ostr, const Codec & codec) { return ostr << "Codec{" << "name: " << codec.codec_statement - << ", expected_compression_ratio: " << codec.expected_compression_ratio + << ", expected_compression_ratio: " << *codec.expected_compression_ratio << "}"; } @@ -775,15 +760,13 @@ auto FFand0Generator = []() return [step = 0](auto i) mutable { decltype(i) result; - if (step++ % 2 == 0) - { - memset(&result, 0, sizeof(result)); - } - else - { - memset(&result, 0xFF, sizeof(result)); - } + if (step % 2 == 0) + memset(&result, 0, sizeof(result)); + else + memset(&result, 0xFF, sizeof(result)); + + ++step; return result; }; }; diff --git a/src/DataStreams/IBlockInputStream.h b/src/DataStreams/IBlockInputStream.h index b077f87e8a8..090ea394fd6 100644 --- a/src/DataStreams/IBlockInputStream.h +++ b/src/DataStreams/IBlockInputStream.h @@ -158,7 +158,7 @@ public: /** Set the approximate total number of rows to read. */ - virtual void addTotalRowsApprox(size_t value) { total_rows_approx += value; } + void addTotalRowsApprox(size_t value) { total_rows_approx += value; } /** Ask to abort the receipt of data as soon as possible. diff --git a/src/DataStreams/NativeBlockOutputStream.cpp b/src/DataStreams/NativeBlockOutputStream.cpp index 2a016c9a0c8..a5d8c15986b 100644 --- a/src/DataStreams/NativeBlockOutputStream.cpp +++ b/src/DataStreams/NativeBlockOutputStream.cpp @@ -51,7 +51,7 @@ static void writeData(const IDataType & type, const ColumnPtr & column, WriteBuf ISerialization::SerializeBinaryBulkSettings settings; settings.getter = [&ostr](ISerialization::SubstreamPath) -> WriteBuffer * { return &ostr; }; settings.position_independent_encoding = false; - settings.low_cardinality_max_dictionary_size = 0; + settings.low_cardinality_max_dictionary_size = 0; //-V1048 auto serialization = type.getDefaultSerialization(); diff --git a/src/DataStreams/PostgreSQLBlockInputStream.cpp b/src/DataStreams/PostgreSQLBlockInputStream.cpp index f134ed0c693..8897cd13a2e 100644 --- a/src/DataStreams/PostgreSQLBlockInputStream.cpp +++ b/src/DataStreams/PostgreSQLBlockInputStream.cpp @@ -210,7 +210,8 @@ void PostgreSQLBlockInputStream::insertValue(IColumn & column, std::string_view { max_dimension = std::max(max_dimension, dimension); - if (--dimension == 0) + --dimension; + if (dimension == 0) break; dimensions[dimension].emplace_back(Array(dimensions[dimension + 1].begin(), dimensions[dimension + 1].end())); diff --git a/src/DataTypes/tests/gtest_DataType_deserializeAsText.cpp b/src/DataTypes/tests/gtest_DataType_deserializeAsText.cpp index 9d8c32c92b6..675ecfbe6e7 100644 --- a/src/DataTypes/tests/gtest_DataType_deserializeAsText.cpp +++ b/src/DataTypes/tests/gtest_DataType_deserializeAsText.cpp @@ -15,8 +15,6 @@ #include -namespace std -{ template inline std::ostream& operator<<(std::ostream & ostr, const std::vector & v) @@ -29,8 +27,6 @@ inline std::ostream& operator<<(std::ostream & ostr, const std::vector & v) return ostr << "] (" << v.size() << ") items"; } -} - using namespace DB; struct ParseDataTypeTestCase diff --git a/src/Dictionaries/Embedded/RegionsNames.h b/src/Dictionaries/Embedded/RegionsNames.h index 7216f238156..91830490325 100644 --- a/src/Dictionaries/Embedded/RegionsNames.h +++ b/src/Dictionaries/Embedded/RegionsNames.h @@ -84,7 +84,7 @@ public: { size_t language_id = static_cast(language); - if (region_id >= names_refs[language_id].size()) + if (region_id >= names_refs[language_id].size()) //-V1051 return StringRef("", 0); StringRef ref = names_refs[language_id][region_id]; diff --git a/src/Dictionaries/MySQLDictionarySource.cpp b/src/Dictionaries/MySQLDictionarySource.cpp index 676863ae588..e440ed59fb0 100644 --- a/src/Dictionaries/MySQLDictionarySource.cpp +++ b/src/Dictionaries/MySQLDictionarySource.cpp @@ -164,7 +164,7 @@ bool MySQLDictionarySource::isModified() const if (!invalidate_query.empty()) { auto response = doInvalidateQuery(invalidate_query); - if (response == invalidate_query_response) + if (response == invalidate_query_response) //-V1051 return false; invalidate_query_response = response; return true; diff --git a/src/Dictionaries/PostgreSQLDictionarySource.cpp b/src/Dictionaries/PostgreSQLDictionarySource.cpp index 051ee94ef3e..af7aef45acc 100644 --- a/src/Dictionaries/PostgreSQLDictionarySource.cpp +++ b/src/Dictionaries/PostgreSQLDictionarySource.cpp @@ -103,7 +103,7 @@ bool PostgreSQLDictionarySource::isModified() const if (!invalidate_query.empty()) { auto response = doInvalidateQuery(invalidate_query); - if (response == invalidate_query_response) + if (response == invalidate_query_response) //-V1051 return false; invalidate_query_response = response; } diff --git a/src/Dictionaries/XDBCDictionarySource.cpp b/src/Dictionaries/XDBCDictionarySource.cpp index 5774641a90f..618e5282068 100644 --- a/src/Dictionaries/XDBCDictionarySource.cpp +++ b/src/Dictionaries/XDBCDictionarySource.cpp @@ -225,7 +225,7 @@ bool XDBCDictionarySource::isModified() const if (!invalidate_query.empty()) { auto response = doInvalidateQuery(invalidate_query); - if (invalidate_query_response == response) + if (invalidate_query_response == response) //-V1051 return false; invalidate_query_response = response; } diff --git a/src/Functions/FunctionBase64Conversion.h b/src/Functions/FunctionBase64Conversion.h index d060b86c54b..29aa5913b83 100644 --- a/src/Functions/FunctionBase64Conversion.h +++ b/src/Functions/FunctionBase64Conversion.h @@ -146,7 +146,7 @@ public: if (!outlen) { outlen = 0; - dst_pos = savepoint; + dst_pos = savepoint; //-V1048 // clean the symbol dst_pos[0] = 0; } diff --git a/src/Functions/isValidUTF8.cpp b/src/Functions/isValidUTF8.cpp index d8c5d3cc580..e3158bb709c 100644 --- a/src/Functions/isValidUTF8.cpp +++ b/src/Functions/isValidUTF8.cpp @@ -277,7 +277,7 @@ SOFTWARE. len -= 16; }; - while (len >= 16) // NOLINT + while (len >= 16) // NOLINT //-V1044 check_packed(_mm_loadu_si128(reinterpret_cast(data))); /// 0 <= len <= 15 for now. Reading data from data - 1 because of right padding of 15 and left padding diff --git a/src/Interpreters/ConvertStringsToEnumVisitor.cpp b/src/Interpreters/ConvertStringsToEnumVisitor.cpp new file mode 100644 index 00000000000..85971457c36 --- /dev/null +++ b/src/Interpreters/ConvertStringsToEnumVisitor.cpp @@ -0,0 +1,186 @@ +#include +#include +#include +#include +#include + + +namespace DB +{ + +namespace +{ + +/// @note We place strings in ascending order here under the assumption it colud speed up String to Enum conversion. +String makeStringsEnum(const std::set & values) +{ + String enum_string = "Enum8("; + if (values.size() >= 255) + enum_string = "Enum16("; + + size_t number = 1; + for (const auto & item : values) + { + enum_string += "\'" + item + "\' = " + std::to_string(number++); + + if (number <= values.size()) + enum_string += ", "; + } + + enum_string += ")"; + return enum_string; +} + +void changeIfArguments(ASTPtr & first, ASTPtr & second) +{ + String first_value = first->as()->value.get(); + String second_value = second->as()->value.get(); + + std::set values; + values.insert(first_value); + values.insert(second_value); + + String enum_string = makeStringsEnum(values); + auto enum_literal = std::make_shared(enum_string); + + auto first_cast = makeASTFunction("CAST"); + first_cast->arguments->children.push_back(first); + first_cast->arguments->children.push_back(enum_literal); + + auto second_cast = makeASTFunction("CAST"); + second_cast->arguments->children.push_back(second); + second_cast->arguments->children.push_back(enum_literal); + + first = first_cast; + second = second_cast; +} + +void changeTransformArguments(ASTPtr & array_to, ASTPtr & other) +{ + std::set values; + + for (const auto & item : array_to->as()->value.get()) + values.insert(item.get()); + values.insert(other->as()->value.get()); + + String enum_string = makeStringsEnum(values); + + auto array_cast = makeASTFunction("CAST"); + array_cast->arguments->children.push_back(array_to); + array_cast->arguments->children.push_back(std::make_shared("Array(" + enum_string + ")")); + array_to = array_cast; + + auto other_cast = makeASTFunction("CAST"); + other_cast->arguments->children.push_back(other); + other_cast->arguments->children.push_back(std::make_shared(enum_string)); + other = other_cast; +} + +bool checkSameType(const Array & array, const String & type) +{ + for (const auto & item : array) + if (item.getTypeName() != type) + return false; + return true; +} + +} + + +bool FindUsedFunctionsMatcher::needChildVisit(const ASTPtr & node, const ASTPtr &) +{ + return !(node->as()); +} + +void FindUsedFunctionsMatcher::visit(const ASTPtr & ast, Data & data) +{ + if (auto * func = ast->as()) + visit(*func, data); +} + +void FindUsedFunctionsMatcher::visit(const ASTFunction & func, Data & data) +{ + if (data.names.count(func.name) && !data.call_stack.empty()) + { + String alias = func.tryGetAlias(); + if (!alias.empty()) + { + data.used_functions.insert(alias); + } + } + + data.call_stack.push_back(func.name); + + /// Visit children with known call stack + Visitor(data).visit(func.arguments); + + data.call_stack.pop_back(); +} + + +bool ConvertStringsToEnumMatcher::needChildVisit(const ASTPtr & node, const ASTPtr &) +{ + return !(node->as()); +} + +void ConvertStringsToEnumMatcher::visit(ASTPtr & ast, Data & data) +{ + if (auto * func = ast->as()) + visit(*func, data); +} + +void ConvertStringsToEnumMatcher::visit(ASTFunction & function_node, Data & data) +{ + if (!function_node.arguments) + return; + + /// We are not sure we could change the type of function result + /// cause it is present in other function as argument + if (data.used_functions.count(function_node.tryGetAlias())) + return; + + if (function_node.name == "if") + { + if (function_node.arguments->children.size() != 2) + return; + + auto literal1 = function_node.arguments->children[1]->as(); + auto literal2 = function_node.arguments->children[2]->as(); + if (!literal1 || !literal2) + return; + + if (String(literal1->value.getTypeName()) != "String" || + String(literal2->value.getTypeName()) != "String") + return; + + changeIfArguments(function_node.arguments->children[1], + function_node.arguments->children[2]); + } + else if (function_node.name == "transform") + { + if (function_node.arguments->children.size() != 4) + return; + + auto literal_to = function_node.arguments->children[2]->as(); + auto literal_other = function_node.arguments->children[3]->as(); + if (!literal_to || !literal_other) + return; + + if (String(literal_to->value.getTypeName()) != "Array" || + String(literal_other->value.getTypeName()) != "String") + return; + + Array array_to = literal_to->value.get(); + if (array_to.size() == 0) + return; + + bool to_strings = checkSameType(array_to, "String"); + if (!to_strings) + return; + + changeTransformArguments(function_node.arguments->children[2], function_node.arguments->children[3]); + } +} + +} + diff --git a/src/Interpreters/ConvertStringsToEnumVisitor.h b/src/Interpreters/ConvertStringsToEnumVisitor.h index adf408f87bd..b1389f40654 100644 --- a/src/Interpreters/ConvertStringsToEnumVisitor.h +++ b/src/Interpreters/ConvertStringsToEnumVisitor.h @@ -1,98 +1,17 @@ #pragma once -#include -#include +#include +#include +#include + #include -#include -#include -#include -#include -#include -#include -#include -#include -#include +#include + namespace DB { -namespace -{ - -/// @note We place strings in ascending order here under the assumption it colud speed up String to Enum conversion. -String makeStringsEnum(const std::set & values) -{ - String enum_string = "Enum8("; - if (values.size() >= 255) - enum_string = "Enum16("; - - size_t number = 1; - for (const auto & item : values) - { - enum_string += "\'" + item + "\' = " + std::to_string(number++); - - if (number <= values.size()) - enum_string += ", "; - } - - enum_string += ")"; - return enum_string; -} - -void changeIfArguments(ASTPtr & first, ASTPtr & second) -{ - String first_value = first->as()->value.get(); - String second_value = second->as()->value.get(); - - std::set values; - values.insert(first_value); - values.insert(second_value); - - String enum_string = makeStringsEnum(values); - auto enum_literal = std::make_shared(enum_string); - - auto first_cast = makeASTFunction("CAST"); - first_cast->arguments->children.push_back(first); - first_cast->arguments->children.push_back(enum_literal); - - auto second_cast = makeASTFunction("CAST"); - second_cast->arguments->children.push_back(second); - second_cast->arguments->children.push_back(enum_literal); - - first = first_cast; - second = second_cast; -} - -void changeTransformArguments(ASTPtr & array_to, ASTPtr & other) -{ - std::set values; - - for (const auto & item : array_to->as()->value.get()) - values.insert(item.get()); - values.insert(other->as()->value.get()); - - String enum_string = makeStringsEnum(values); - - auto array_cast = makeASTFunction("CAST"); - array_cast->arguments->children.push_back(array_to); - array_cast->arguments->children.push_back(std::make_shared("Array(" + enum_string + ")")); - array_to = array_cast; - - auto other_cast = makeASTFunction("CAST"); - other_cast->arguments->children.push_back(other); - other_cast->arguments->children.push_back(std::make_shared(enum_string)); - other = other_cast; -} - -bool checkSameType(const Array & array, const String & type) -{ - for (const auto & item : array) - if (item.getTypeName() != type) - return false; - return true; -} - -} +class ASTFunction; struct FindUsedFunctionsMatcher { @@ -105,35 +24,9 @@ struct FindUsedFunctionsMatcher std::vector call_stack = {}; }; - static bool needChildVisit(const ASTPtr & node, const ASTPtr &) - { - return !(node->as()); - } - - static void visit(const ASTPtr & ast, Data & data) - { - if (auto * func = ast->as()) - visit(*func, data); - } - - static void visit(const ASTFunction & func, Data & data) - { - if (data.names.count(func.name) && !data.call_stack.empty()) - { - String alias = func.tryGetAlias(); - if (!alias.empty()) - { - data.used_functions.insert(alias); - } - } - - data.call_stack.push_back(func.name); - - /// Visit children with known call stack - Visitor(data).visit(func.arguments); - - data.call_stack.pop_back(); - } + static bool needChildVisit(const ASTPtr & node, const ASTPtr &); + static void visit(const ASTPtr & ast, Data & data); + static void visit(const ASTFunction & func, Data & data); }; using FindUsedFunctionsVisitor = FindUsedFunctionsMatcher::Visitor; @@ -145,70 +38,9 @@ struct ConvertStringsToEnumMatcher std::unordered_set & used_functions; }; - static bool needChildVisit(const ASTPtr & node, const ASTPtr &) - { - return !(node->as()); - } - - static void visit(ASTPtr & ast, Data & data) - { - if (auto * func = ast->as()) - visit(*func, data); - } - - static void visit(ASTFunction & function_node, Data & data) - { - if (!function_node.arguments) - return; - - /// We are not sure we could change the type of function result - /// cause it is present in other function as argument - if (data.used_functions.count(function_node.tryGetAlias())) - return; - - if (function_node.name == "if") - { - if (function_node.arguments->children.size() != 2) - return; - - auto literal1 = function_node.arguments->children[1]->as(); - auto literal2 = function_node.arguments->children[2]->as(); - if (!literal1 || !literal2) - return; - - if (String(literal1->value.getTypeName()) != "String" || - String(literal2->value.getTypeName()) != "String") - return; - - changeIfArguments(function_node.arguments->children[1], - function_node.arguments->children[2]); - } - else if (function_node.name == "transform") - { - if (function_node.arguments->children.size() != 4) - return; - - auto literal_to = function_node.arguments->children[2]->as(); - auto literal_other = function_node.arguments->children[3]->as(); - if (!literal_to || !literal_other) - return; - - if (String(literal_to->value.getTypeName()) != "Array" || - String(literal_other->value.getTypeName()) != "String") - return; - - Array array_to = literal_to->value.get(); - if (array_to.size() == 0) - return; - - bool to_strings = checkSameType(array_to, "String"); - if (!to_strings) - return; - - changeTransformArguments(function_node.arguments->children[2], - function_node.arguments->children[3]); - } - } + static bool needChildVisit(const ASTPtr & node, const ASTPtr &); + static void visit(ASTPtr & ast, Data & data); + static void visit(ASTFunction & function_node, Data & data); }; using ConvertStringsToEnumVisitor = InDepthNodeVisitor; diff --git a/src/Interpreters/InterpreterWatchQuery.cpp b/src/Interpreters/InterpreterWatchQuery.cpp index 84959f2f624..edf0f37c00e 100644 --- a/src/Interpreters/InterpreterWatchQuery.cpp +++ b/src/Interpreters/InterpreterWatchQuery.cpp @@ -77,7 +77,7 @@ BlockIO InterpreterWatchQuery::execute() if (IBlockInputStream * stream = dynamic_cast(streams[0].get())) { StreamLocalLimits limits; - limits.mode = LimitsMode::LIMITS_CURRENT; + limits.mode = LimitsMode::LIMITS_CURRENT; //-V1048 limits.size_limits.max_rows = settings.max_result_rows; limits.size_limits.max_bytes = settings.max_result_bytes; limits.size_limits.overflow_mode = settings.result_overflow_mode; diff --git a/src/Interpreters/PartLog.cpp b/src/Interpreters/PartLog.cpp index e4459399336..ad4fb60f00c 100644 --- a/src/Interpreters/PartLog.cpp +++ b/src/Interpreters/PartLog.cpp @@ -143,7 +143,7 @@ bool PartLog::addNewParts( if (query_id.data && query_id.size) elem.query_id.insert(0, query_id.data, query_id.size); - elem.event_type = PartLogElement::NEW_PART; + elem.event_type = PartLogElement::NEW_PART; //-V1048 // construct event_time and event_time_microseconds using the same time point // so that the two times will always be equal up to a precision of a second. diff --git a/src/Interpreters/executeQuery.cpp b/src/Interpreters/executeQuery.cpp index 5df245f9f26..f310ce0d529 100644 --- a/src/Interpreters/executeQuery.cpp +++ b/src/Interpreters/executeQuery.cpp @@ -552,7 +552,7 @@ static std::tuple executeQueryImpl( StreamLocalLimits limits; if (!interpreter->ignoreLimits()) { - limits.mode = LimitsMode::LIMITS_CURRENT; + limits.mode = LimitsMode::LIMITS_CURRENT; //-V1048 limits.size_limits = SizeLimits(settings.max_result_rows, settings.max_result_bytes, settings.result_overflow_mode); } @@ -632,7 +632,7 @@ static std::tuple executeQueryImpl( { QueryLogElement elem; - elem.type = QueryLogElementType::QUERY_START; + elem.type = QueryLogElementType::QUERY_START; //-V1048 elem.event_time = time_in_seconds(current_time); elem.event_time_microseconds = time_in_microseconds(current_time); diff --git a/src/Interpreters/join_common.cpp b/src/Interpreters/join_common.cpp index cc51848b4a4..80299610a44 100644 --- a/src/Interpreters/join_common.cpp +++ b/src/Interpreters/join_common.cpp @@ -281,7 +281,7 @@ void createMissedColumns(Block & block) for (size_t i = 0; i < block.columns(); ++i) { auto & column = block.getByPosition(i); - if (!column.column) + if (!column.column) //-V1051 column.column = column.type->createColumn(); } } diff --git a/src/Parsers/ParserExplainQuery.cpp b/src/Parsers/ParserExplainQuery.cpp index 7a7abba94a9..c8d8dc10a7f 100644 --- a/src/Parsers/ParserExplainQuery.cpp +++ b/src/Parsers/ParserExplainQuery.cpp @@ -31,7 +31,7 @@ bool ParserExplainQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected else if (s_pipeline.ignore(pos, expected)) kind = ASTExplainQuery::ExplainKind::QueryPipeline; else if (s_plan.ignore(pos, expected)) - kind = ASTExplainQuery::ExplainKind::QueryPlan; + kind = ASTExplainQuery::ExplainKind::QueryPlan; //-V1048 } else return false; diff --git a/src/Processors/Executors/PipelineExecutingBlockInputStream.cpp b/src/Processors/Executors/PipelineExecutingBlockInputStream.cpp index 458fa0a0a90..736ad1ecefe 100644 --- a/src/Processors/Executors/PipelineExecutingBlockInputStream.cpp +++ b/src/Processors/Executors/PipelineExecutingBlockInputStream.cpp @@ -121,11 +121,4 @@ void PipelineExecutingBlockInputStream::setQuota(const std::shared_ptr & quota_) final; - void addTotalRowsApprox(size_t value) final; protected: void readPrefixImpl() override; diff --git a/src/Processors/QueryPipeline.h b/src/Processors/QueryPipeline.h index 0c8caa93539..90c7e370880 100644 --- a/src/Processors/QueryPipeline.h +++ b/src/Processors/QueryPipeline.h @@ -135,7 +135,7 @@ public: { auto num_threads = pipe.maxParallelStreams(); - if (max_threads) + if (max_threads) //-V1051 num_threads = std::min(num_threads, max_threads); return std::max(1, num_threads); diff --git a/src/Processors/QueryPlan/PartialSortingStep.cpp b/src/Processors/QueryPlan/PartialSortingStep.cpp index f4abea440fe..d713a63028a 100644 --- a/src/Processors/QueryPlan/PartialSortingStep.cpp +++ b/src/Processors/QueryPlan/PartialSortingStep.cpp @@ -58,7 +58,7 @@ void PartialSortingStep::transformPipeline(QueryPipeline & pipeline, const Build }); StreamLocalLimits limits; - limits.mode = LimitsMode::LIMITS_CURRENT; + limits.mode = LimitsMode::LIMITS_CURRENT; //-V1048 limits.size_limits = size_limits; pipeline.addSimpleTransform([&](const Block & header, QueryPipeline::StreamType stream_type) -> ProcessorPtr diff --git a/src/Processors/QueryPlan/QueryPlan.cpp b/src/Processors/QueryPlan/QueryPlan.cpp index d04d98c9d97..44c5c48975c 100644 --- a/src/Processors/QueryPlan/QueryPlan.cpp +++ b/src/Processors/QueryPlan/QueryPlan.cpp @@ -158,7 +158,7 @@ QueryPipelinePtr QueryPlan::buildQueryPipeline( if (last_pipeline) { frame.pipelines.emplace_back(std::move(last_pipeline)); - last_pipeline = nullptr; + last_pipeline = nullptr; //-V1048 } size_t next_child = frame.pipelines.size(); diff --git a/src/Server/TCPHandler.cpp b/src/Server/TCPHandler.cpp index 916f29ba1d4..9ef0292ee06 100644 --- a/src/Server/TCPHandler.cpp +++ b/src/Server/TCPHandler.cpp @@ -745,7 +745,7 @@ void TCPHandler::processTablesStatusRequest() status.absolute_delay = replicated_table->getAbsoluteDelay(); } else - status.is_replicated = false; + status.is_replicated = false; //-V1048 response.table_states_by_id.emplace(table_name, std::move(status)); } diff --git a/src/Storages/MergeTree/MergeTreeData.cpp b/src/Storages/MergeTree/MergeTreeData.cpp index dad7f021491..854b64181cc 100644 --- a/src/Storages/MergeTree/MergeTreeData.cpp +++ b/src/Storages/MergeTree/MergeTreeData.cpp @@ -1179,7 +1179,7 @@ void MergeTreeData::removePartsFinally(const MergeTreeData::DataPartsVector & pa part_log_elem.event_type = PartLogElement::REMOVE_PART; part_log_elem.event_time = time(nullptr); - part_log_elem.duration_ms = 0; + part_log_elem.duration_ms = 0; //-V1048 part_log_elem.database_name = table_id.database_name; part_log_elem.table_name = table_id.table_name; diff --git a/src/Storages/MergeTree/MergeTreeDataPartWriterCompact.cpp b/src/Storages/MergeTree/MergeTreeDataPartWriterCompact.cpp index 2efda206cf9..5e85f88a487 100644 --- a/src/Storages/MergeTree/MergeTreeDataPartWriterCompact.cpp +++ b/src/Storages/MergeTree/MergeTreeDataPartWriterCompact.cpp @@ -114,8 +114,8 @@ void writeColumnSingleGranule( ISerialization::SerializeBinaryBulkSettings serialize_settings; serialize_settings.getter = stream_getter; - serialize_settings.position_independent_encoding = true; - serialize_settings.low_cardinality_max_dictionary_size = 0; + serialize_settings.position_independent_encoding = true; //-V1048 + serialize_settings.low_cardinality_max_dictionary_size = 0; //-V1048 serialization->serializeBinaryBulkStatePrefix(serialize_settings, state); serialization->serializeBinaryBulkWithMultipleStreams(*column.column, from_row, number_of_rows, serialize_settings, state); diff --git a/src/Storages/MergeTree/MergeTreeIndexSet.cpp b/src/Storages/MergeTree/MergeTreeIndexSet.cpp index ff875b185e9..6cee80983d6 100644 --- a/src/Storages/MergeTree/MergeTreeIndexSet.cpp +++ b/src/Storages/MergeTree/MergeTreeIndexSet.cpp @@ -69,7 +69,7 @@ void MergeTreeIndexGranuleSet::serializeBinary(WriteBuffer & ostr) const ISerialization::SerializeBinaryBulkSettings settings; settings.getter = [&ostr](ISerialization::SubstreamPath) -> WriteBuffer * { return &ostr; }; settings.position_independent_encoding = false; - settings.low_cardinality_max_dictionary_size = 0; + settings.low_cardinality_max_dictionary_size = 0; //-V1048 auto serialization = type->getDefaultSerialization(); ISerialization::SerializeBinaryBulkStatePtr state; diff --git a/src/Storages/RabbitMQ/StorageRabbitMQ.cpp b/src/Storages/RabbitMQ/StorageRabbitMQ.cpp index de923390300..6f2d4813f05 100644 --- a/src/Storages/RabbitMQ/StorageRabbitMQ.cpp +++ b/src/Storages/RabbitMQ/StorageRabbitMQ.cpp @@ -402,7 +402,7 @@ void StorageRabbitMQ::bindExchange() } } - while (!binding_created) + while (!binding_created) //-V1044 { event_handler->iterateLoop(); } @@ -463,7 +463,7 @@ void StorageRabbitMQ::bindQueue(size_t queue_id) const String queue_name = !hash_exchange ? queue_base : std::to_string(queue_id) + "_" + queue_base; setup_channel->declareQueue(queue_name, AMQP::durable, queue_settings).onSuccess(success_callback).onError(error_callback); - while (!binding_created) + while (!binding_created) //-V1044 { event_handler->iterateLoop(); } From 8e8160be28f4d0017280ba70de680b9c39a698eb Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sat, 8 May 2021 21:11:31 +0300 Subject: [PATCH 13/26] Fix error --- src/Storages/System/StorageSystemTables.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Storages/System/StorageSystemTables.cpp b/src/Storages/System/StorageSystemTables.cpp index 7046a37766c..9d00a9b71c4 100644 --- a/src/Storages/System/StorageSystemTables.cpp +++ b/src/Storages/System/StorageSystemTables.cpp @@ -389,8 +389,8 @@ protected: src_index += 2; StorageMetadataPtr metadata_snapshot; - assert(table != nullptr); - metadata_snapshot = table->getInMemoryMetadataPtr(); + if (table) + metadata_snapshot = table->getInMemoryMetadataPtr(); ASTPtr expression_ptr; if (columns_mask[src_index++]) @@ -427,7 +427,7 @@ protected: if (columns_mask[src_index++]) { - auto policy = table->getStoragePolicy(); + auto policy = table ? table->getStoragePolicy() : nullptr; if (policy) res_columns[res_index++]->insert(policy->getName()); else @@ -436,7 +436,7 @@ protected: if (columns_mask[src_index++]) { - auto total_rows = table->totalRows(context->getSettingsRef()); + auto total_rows = table ? table->totalRows(context->getSettingsRef()) : std::nullopt; if (total_rows) res_columns[res_index++]->insert(*total_rows); else @@ -445,7 +445,7 @@ protected: if (columns_mask[src_index++]) { - auto total_bytes = table->totalBytes(context->getSettingsRef()); + auto total_bytes = table ? table->totalBytes(context->getSettingsRef()) : std::nullopt; if (total_bytes) res_columns[res_index++]->insert(*total_bytes); else @@ -454,7 +454,7 @@ protected: if (columns_mask[src_index++]) { - auto lifetime_rows = table->lifetimeRows(); + auto lifetime_rows = table ? table->lifetimeRows() : std::nullopt; if (lifetime_rows) res_columns[res_index++]->insert(*lifetime_rows); else @@ -463,7 +463,7 @@ protected: if (columns_mask[src_index++]) { - auto lifetime_bytes = table->lifetimeBytes(); + auto lifetime_bytes = table ? table->lifetimeBytes() : std::nullopt; if (lifetime_bytes) res_columns[res_index++]->insert(*lifetime_bytes); else From aa25ffa9e3cfa86c59198570855f174be571ef32 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sat, 8 May 2021 21:15:00 +0300 Subject: [PATCH 14/26] Fix error --- src/Processors/Transforms/FillingTransform.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Processors/Transforms/FillingTransform.cpp b/src/Processors/Transforms/FillingTransform.cpp index 085e21b1766..317453f272a 100644 --- a/src/Processors/Transforms/FillingTransform.cpp +++ b/src/Processors/Transforms/FillingTransform.cpp @@ -51,6 +51,11 @@ FillingTransform::FillingTransform( else return false; + if (descr.fill_from.getType() > max_type + || descr.fill_to.getType() > max_type + || descr.fill_step.getType() > max_type) + return false; + descr.fill_from = convertFieldToType(descr.fill_from, *to_type); descr.fill_to = convertFieldToType(descr.fill_to, *to_type); descr.fill_step = convertFieldToType(descr.fill_step, *to_type); From ac9b43f10322731a6751fca634cd646c87db0bb1 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sat, 8 May 2021 21:16:25 +0300 Subject: [PATCH 15/26] Fix error --- src/Common/HyperLogLogCounter.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Common/HyperLogLogCounter.h b/src/Common/HyperLogLogCounter.h index 4f44e0c3c55..e32619b509e 100644 --- a/src/Common/HyperLogLogCounter.h +++ b/src/Common/HyperLogLogCounter.h @@ -115,7 +115,7 @@ struct IntermediateDenominator -class __attribute__ ((packed)) details::Denominator> { From c412820e9c49f6e4bc6ec2c8ca8d585deda95421 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sat, 8 May 2021 22:30:58 +0300 Subject: [PATCH 16/26] Fix warnings by PVS-Studio --- src/Common/ZooKeeper/TestKeeper.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Common/ZooKeeper/TestKeeper.cpp b/src/Common/ZooKeeper/TestKeeper.cpp index ddd077f7117..49eb7ad764d 100644 --- a/src/Common/ZooKeeper/TestKeeper.cpp +++ b/src/Common/ZooKeeper/TestKeeper.cpp @@ -198,7 +198,7 @@ std::pair TestKeeperCreateRequest::process(TestKeeper::Contai else { TestKeeper::Node created_node; - created_node.seq_num = 0; //-V1044 + created_node.seq_num = 0; //-V1048 created_node.stat.czxid = zxid; created_node.stat.mzxid = zxid; created_node.stat.ctime = std::chrono::system_clock::now().time_since_epoch() / std::chrono::milliseconds(1); @@ -271,7 +271,7 @@ std::pair TestKeeperRemoveRequest::process(TestKeeper::Contai auto & parent = container.at(parentPath(path)); --parent.stat.numChildren; ++parent.stat.cversion; - response.error = Error::ZOK; //-V1044 + response.error = Error::ZOK; //-V1048 undo = [prev_node, &container, path = path] { @@ -293,7 +293,7 @@ std::pair TestKeeperExistsRequest::process(TestKeeper::Contai if (it != container.end()) { response.stat = it->second.stat; - response.error = Error::ZOK; //-V1044 + response.error = Error::ZOK; //-V1048 } else { @@ -316,7 +316,7 @@ std::pair TestKeeperGetRequest::process(TestKeeper::Container { response.stat = it->second.stat; response.data = it->second.data; - response.error = Error::ZOK; //-V1044 + response.error = Error::ZOK; //-V1048 } return { std::make_shared(response), {} }; @@ -343,7 +343,7 @@ std::pair TestKeeperSetRequest::process(TestKeeper::Container it->second.data = data; ++container.at(parentPath(path)).stat.cversion; response.stat = it->second.stat; - response.error = Error::ZOK; //-V1044 + response.error = Error::ZOK; //-V1048 undo = [prev_node, &container, path = path] { @@ -387,7 +387,7 @@ std::pair TestKeeperListRequest::process(TestKeeper::Containe } response.stat = it->second.stat; - response.error = Error::ZOK; //-V1044 + response.error = Error::ZOK; //-V1048 } return { std::make_shared(response), {} }; @@ -407,7 +407,7 @@ std::pair TestKeeperCheckRequest::process(TestKeeper::Contain } else { - response.error = Error::ZOK; //-V1044 + response.error = Error::ZOK; //-V1048 } return { std::make_shared(response), {} }; @@ -422,7 +422,7 @@ std::pair TestKeeperMultiRequest::process(TestKeeper::Contain try { auto request_it = requests.begin(); - response.error = Error::ZOK; //-V1044 + response.error = Error::ZOK; //-V1048 while (request_it != requests.end()) { const TestKeeperRequest & concrete_request = dynamic_cast(**request_it); From 7f349cd286971e8938d5aad386733447d4f60b94 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sat, 8 May 2021 22:31:11 +0300 Subject: [PATCH 17/26] Fix warnings by PVS-Studio in some third-party code --- base/glibc-compatibility/musl/lgamma.c | 3 ++ base/glibc-compatibility/musl/lgammal.c | 3 ++ base/glibc-compatibility/musl/libm.h | 3 ++ base/glibc-compatibility/musl/powf.c | 3 ++ base/pcg-random/pcg_extras.hpp | 65 +------------------------ base/pcg-random/pcg_random.hpp | 8 +-- 6 files changed, 17 insertions(+), 68 deletions(-) diff --git a/base/glibc-compatibility/musl/lgamma.c b/base/glibc-compatibility/musl/lgamma.c index fb9d105d0fa..5e959504e29 100644 --- a/base/glibc-compatibility/musl/lgamma.c +++ b/base/glibc-compatibility/musl/lgamma.c @@ -78,6 +78,9 @@ * */ +// Disable warnings by PVS-Studio +//-V::GA + static const double pi = 3.14159265358979311600e+00, /* 0x400921FB, 0x54442D18 */ a0 = 7.72156649015328655494e-02, /* 0x3FB3C467, 0xE37DB0C8 */ diff --git a/base/glibc-compatibility/musl/lgammal.c b/base/glibc-compatibility/musl/lgammal.c index b158748ce1f..775559f13b6 100644 --- a/base/glibc-compatibility/musl/lgammal.c +++ b/base/glibc-compatibility/musl/lgammal.c @@ -85,6 +85,9 @@ * */ +// Disable warnings by PVS-Studio +//-V::GA + #include #include #include "libm.h" diff --git a/base/glibc-compatibility/musl/libm.h b/base/glibc-compatibility/musl/libm.h index 55520c2fb03..cdf766b49f3 100644 --- a/base/glibc-compatibility/musl/libm.h +++ b/base/glibc-compatibility/musl/libm.h @@ -1,6 +1,9 @@ #ifndef _LIBM_H #define _LIBM_H +// Disable warnings by PVS-Studio +//-V::GA + #include #include #include diff --git a/base/glibc-compatibility/musl/powf.c b/base/glibc-compatibility/musl/powf.c index de8fab54554..35dc3611b94 100644 --- a/base/glibc-compatibility/musl/powf.c +++ b/base/glibc-compatibility/musl/powf.c @@ -3,6 +3,9 @@ * SPDX-License-Identifier: MIT */ +// Disable warnings by PVS-Studio +//-V::GA + #include #include #include "libm.h" diff --git a/base/pcg-random/pcg_extras.hpp b/base/pcg-random/pcg_extras.hpp index b71e859a25f..39c91c4ecfa 100644 --- a/base/pcg-random/pcg_extras.hpp +++ b/base/pcg-random/pcg_extras.hpp @@ -447,69 +447,6 @@ inline SrcIter uneven_copy(SrcIter src_first, std::integral_constant{}); } -/* generate_to, fill in a fixed-size array of integral type using a SeedSeq - * (actually works for any random-access iterator) - */ - -template -inline void generate_to_impl(SeedSeq&& generator, DestIter dest, - std::true_type) -{ - generator.generate(dest, dest+size); -} - -template -void generate_to_impl(SeedSeq&& generator, DestIter dest, - std::false_type) -{ - typedef typename std::iterator_traits::value_type dest_t; - constexpr auto DEST_SIZE = sizeof(dest_t); - constexpr auto GEN_SIZE = sizeof(uint32_t); - - constexpr bool GEN_IS_SMALLER = GEN_SIZE < DEST_SIZE; - constexpr size_t FROM_ELEMS = - GEN_IS_SMALLER - ? size * ((DEST_SIZE+GEN_SIZE-1) / GEN_SIZE) - : (size + (GEN_SIZE / DEST_SIZE) - 1) - / ((GEN_SIZE / DEST_SIZE) + GEN_IS_SMALLER); - // this odd code ^^^^^^^^^^^^^^^^^ is work-around for - // a bug: http://llvm.org/bugs/show_bug.cgi?id=21287 - - if (FROM_ELEMS <= 1024) { - uint32_t buffer[FROM_ELEMS]; - generator.generate(buffer, buffer+FROM_ELEMS); - uneven_copy(buffer, dest, dest+size); - } else { - uint32_t* buffer = static_cast(malloc(GEN_SIZE * FROM_ELEMS)); - generator.generate(buffer, buffer+FROM_ELEMS); - uneven_copy(buffer, dest, dest+size); - free(static_cast(buffer)); - } -} - -template -inline void generate_to(SeedSeq&& generator, DestIter dest) -{ - typedef typename std::iterator_traits::value_type dest_t; - constexpr bool IS_32BIT = sizeof(dest_t) == sizeof(uint32_t); - - generate_to_impl(std::forward(generator), dest, - std::integral_constant{}); -} - -/* generate_one, produce a value of integral type using a SeedSeq - * (optionally, we can have it produce more than one and pick which one - * we want) - */ - -template -inline UInt generate_one(SeedSeq&& generator) -{ - UInt result[N]; - generate_to(std::forward(generator), result); - return result[i]; -} - template auto bounded_rand(RngType& rng, typename RngType::result_type upper_bound) -> typename RngType::result_type @@ -517,7 +454,7 @@ auto bounded_rand(RngType& rng, typename RngType::result_type upper_bound) typedef typename RngType::result_type rtype; rtype threshold = (RngType::max() - RngType::min() + rtype(1) - upper_bound) % upper_bound; - for (;;) { + for (;;) { //-V1044 rtype r = rng() - RngType::min(); if (r >= threshold) return r % upper_bound; diff --git a/base/pcg-random/pcg_random.hpp b/base/pcg-random/pcg_random.hpp index b7145e2309c..d9d3519a4cf 100644 --- a/base/pcg-random/pcg_random.hpp +++ b/base/pcg-random/pcg_random.hpp @@ -928,7 +928,7 @@ struct rxs_m_xs_mixin { constexpr bitcount_t shift = bits - xtypebits; constexpr bitcount_t mask = (1 << opbits) - 1; bitcount_t rshift = - opbits ? bitcount_t(internal >> (bits - opbits)) & mask : 0; + opbits ? bitcount_t(internal >> (bits - opbits)) & mask : 0; //-V547 internal ^= internal >> (opbits + rshift); internal *= mcg_multiplier::multiplier(); xtype result = internal >> shift; @@ -950,7 +950,7 @@ struct rxs_m_xs_mixin { internal *= mcg_unmultiplier::unmultiplier(); - bitcount_t rshift = opbits ? (internal >> (bits - opbits)) & mask : 0; + bitcount_t rshift = opbits ? (internal >> (bits - opbits)) & mask : 0; //-V547 internal = unxorshift(internal, bits, opbits + rshift); return internal; @@ -975,7 +975,7 @@ struct rxs_m_mixin { : 2; constexpr bitcount_t shift = bits - xtypebits; constexpr bitcount_t mask = (1 << opbits) - 1; - bitcount_t rshift = opbits ? (internal >> (bits - opbits)) & mask : 0; + bitcount_t rshift = opbits ? (internal >> (bits - opbits)) & mask : 0; //-V547 internal ^= internal >> (opbits + rshift); internal *= mcg_multiplier::multiplier(); xtype result = internal >> shift; @@ -1366,7 +1366,7 @@ void extended::selfinit() // - any strange correlations would only be apparent if we // were to backstep the generator so that the base generator // was generating the same values again - result_type xdiff = baseclass::operator()() - baseclass::operator()(); + result_type xdiff = baseclass::operator()() - baseclass::operator()(); //-V501 for (size_t i = 0; i < table_size; ++i) { data_[i] = baseclass::operator()() ^ xdiff; } From 3b767b4a64619476758f3484da9e7a536470b2e2 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sat, 8 May 2021 22:40:23 +0300 Subject: [PATCH 18/26] Fix strange code --- src/Common/HyperLogLogCounter.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Common/HyperLogLogCounter.h b/src/Common/HyperLogLogCounter.h index e32619b509e..5e74020ab3d 100644 --- a/src/Common/HyperLogLogCounter.h +++ b/src/Common/HyperLogLogCounter.h @@ -80,7 +80,7 @@ template struct MinCounterType /// Denominator of expression for HyperLogLog algorithm. template -class __attribute__ ((packed)) Denominator; +class Denominator; /// Returns true if rank storage is big. constexpr bool isBigRankStore(UInt8 precision) @@ -115,7 +115,7 @@ struct IntermediateDenominator -class __attribute__ ((packed)) Denominator> { @@ -159,7 +159,7 @@ private: /// Used when rank storage is big. template -class __attribute__ ((packed)) details::Denominator> { From 367f7fe6c93cb086977a74758c50a34cfc065e22 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sat, 8 May 2021 23:50:12 +0300 Subject: [PATCH 19/26] Fix warnings by PVS-Studio --- base/glibc-compatibility/musl/libm.h | 9 +++------ src/Access/DiskAccessStorage.cpp | 5 +++-- src/Common/HashTable/HashTable.h | 2 +- src/Coordination/Changelog.h | 2 +- src/Functions/hasColumnInTable.cpp | 2 +- src/Functions/isDecimalOverflow.cpp | 2 +- src/Storages/RabbitMQ/StorageRabbitMQ.cpp | 4 ++-- 7 files changed, 12 insertions(+), 14 deletions(-) diff --git a/base/glibc-compatibility/musl/libm.h b/base/glibc-compatibility/musl/libm.h index cdf766b49f3..e5029318693 100644 --- a/base/glibc-compatibility/musl/libm.h +++ b/base/glibc-compatibility/musl/libm.h @@ -1,9 +1,6 @@ #ifndef _LIBM_H #define _LIBM_H -// Disable warnings by PVS-Studio -//-V::GA - #include #include #include @@ -158,7 +155,7 @@ static inline long double fp_barrierl(long double x) static inline void fp_force_evalf(float x) { volatile float y; - y = x; + y = x; //-V1001 } #endif @@ -167,7 +164,7 @@ static inline void fp_force_evalf(float x) static inline void fp_force_eval(double x) { volatile double y; - y = x; + y = x; //-V1001 } #endif @@ -176,7 +173,7 @@ static inline void fp_force_eval(double x) static inline void fp_force_evall(long double x) { volatile long double y; - y = x; + y = x; //-V1001 } #endif diff --git a/src/Access/DiskAccessStorage.cpp b/src/Access/DiskAccessStorage.cpp index 80594f66dfc..8c38cd02f9c 100644 --- a/src/Access/DiskAccessStorage.cpp +++ b/src/Access/DiskAccessStorage.cpp @@ -355,8 +355,9 @@ String DiskAccessStorage::getStorageParamsJSON() const std::lock_guard lock{mutex}; Poco::JSON::Object json; json.set("path", directory_path); - if (readonly) - json.set("readonly", readonly.load()); + bool readonly_loaded = readonly; + if (readonly_loaded) + json.set("readonly", Poco::Dynamic::Var{true}); std::ostringstream oss; // STYLE_CHECK_ALLOW_STD_STRING_STREAM oss.exceptions(std::ios::failbit); Poco::JSON::Stringifier::stringify(json, oss); diff --git a/src/Common/HashTable/HashTable.h b/src/Common/HashTable/HashTable.h index 60271aabba7..128fed7822e 100644 --- a/src/Common/HashTable/HashTable.h +++ b/src/Common/HashTable/HashTable.h @@ -305,7 +305,7 @@ template struct ZeroValueStorage; template -struct ZeroValueStorage //-V308 +struct ZeroValueStorage //-V730 { private: bool has_zero = false; diff --git a/src/Coordination/Changelog.h b/src/Coordination/Changelog.h index 856e6035164..a50b734c5b4 100644 --- a/src/Coordination/Changelog.h +++ b/src/Coordination/Changelog.h @@ -32,7 +32,7 @@ struct ChangelogRecordHeader ChangelogVersion version = CURRENT_CHANGELOG_VERSION; uint64_t index = 0; /// entry log number uint64_t term = 0; - nuraft::log_val_type value_type; + nuraft::log_val_type value_type{}; uint64_t blob_size = 0; }; diff --git a/src/Functions/hasColumnInTable.cpp b/src/Functions/hasColumnInTable.cpp index 28f83a63386..6f34082a87d 100644 --- a/src/Functions/hasColumnInTable.cpp +++ b/src/Functions/hasColumnInTable.cpp @@ -134,7 +134,7 @@ ColumnPtr FunctionHasColumnInTable::executeImpl(const ColumnsWithTypeAndName & a has_column = remote_columns.hasPhysical(column_name); } - return DataTypeUInt8().createColumnConst(input_rows_count, Field(has_column)); + return DataTypeUInt8().createColumnConst(input_rows_count, Field{UInt64(has_column)}); } } diff --git a/src/Functions/isDecimalOverflow.cpp b/src/Functions/isDecimalOverflow.cpp index 0cac3675d12..ed69f28711f 100644 --- a/src/Functions/isDecimalOverflow.cpp +++ b/src/Functions/isDecimalOverflow.cpp @@ -85,7 +85,7 @@ public: auto result_column = ColumnUInt8::create(); - auto call = [&](const auto & types) -> bool //-V567 + auto call = [&](const auto & types) -> bool //-V657 { using Types = std::decay_t; using Type = typename Types::RightType; diff --git a/src/Storages/RabbitMQ/StorageRabbitMQ.cpp b/src/Storages/RabbitMQ/StorageRabbitMQ.cpp index 6f2d4813f05..95a9fe468b5 100644 --- a/src/Storages/RabbitMQ/StorageRabbitMQ.cpp +++ b/src/Storages/RabbitMQ/StorageRabbitMQ.cpp @@ -402,7 +402,7 @@ void StorageRabbitMQ::bindExchange() } } - while (!binding_created) //-V1044 + while (!binding_created) //-V776 { event_handler->iterateLoop(); } @@ -463,7 +463,7 @@ void StorageRabbitMQ::bindQueue(size_t queue_id) const String queue_name = !hash_exchange ? queue_base : std::to_string(queue_id) + "_" + queue_base; setup_channel->declareQueue(queue_name, AMQP::durable, queue_settings).onSuccess(success_callback).onError(error_callback); - while (!binding_created) //-V1044 + while (!binding_created) //-V776 { event_handler->iterateLoop(); } From e23a0d8f7e2af85306035aed8fb3c51db31ff74b Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sat, 8 May 2021 23:54:49 +0300 Subject: [PATCH 20/26] Fix warnings by PVS-Studio --- src/Storages/MergeTree/DataPartsExchange.cpp | 2 +- src/Storages/MergeTree/SimpleMergeSelector.cpp | 2 +- src/Storages/RabbitMQ/StorageRabbitMQ.cpp | 2 +- src/Storages/StorageJoin.cpp | 2 +- src/Storages/StorageMerge.cpp | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Storages/MergeTree/DataPartsExchange.cpp b/src/Storages/MergeTree/DataPartsExchange.cpp index 620dd2de278..78f52cdd0a0 100644 --- a/src/Storages/MergeTree/DataPartsExchange.cpp +++ b/src/Storages/MergeTree/DataPartsExchange.cpp @@ -546,7 +546,7 @@ MergeTreeData::MutableDataPartPtr Fetcher::downloadPartToDisk( String tmp_prefix = tmp_prefix_.empty() ? TMP_PREFIX : tmp_prefix_; /// We will remove directory if it's already exists. Make precautions. - if (tmp_prefix.empty() + if (tmp_prefix.empty() //-V560 || part_name.empty() || std::string::npos != tmp_prefix.find_first_of("/.") || std::string::npos != part_name.find_first_of("/.")) diff --git a/src/Storages/MergeTree/SimpleMergeSelector.cpp b/src/Storages/MergeTree/SimpleMergeSelector.cpp index 8ab01fb4b6c..0775e021c76 100644 --- a/src/Storages/MergeTree/SimpleMergeSelector.cpp +++ b/src/Storages/MergeTree/SimpleMergeSelector.cpp @@ -170,7 +170,7 @@ void selectWithinPartition( for (size_t end = begin + 2; end <= parts_count; ++end) { assert(end > begin); - if (settings.max_parts_to_merge_at_once && end - begin > settings.max_parts_to_merge_at_once) + if (settings.max_parts_to_merge_at_once && end - begin > settings.max_parts_to_merge_at_once) //-V658 break; if (!parts[end - 1].shall_participate_in_merges) diff --git a/src/Storages/RabbitMQ/StorageRabbitMQ.cpp b/src/Storages/RabbitMQ/StorageRabbitMQ.cpp index 95a9fe468b5..e49c8bf1729 100644 --- a/src/Storages/RabbitMQ/StorageRabbitMQ.cpp +++ b/src/Storages/RabbitMQ/StorageRabbitMQ.cpp @@ -192,7 +192,7 @@ String StorageRabbitMQ::getTableBasedName(String name, const StorageID & table_i std::shared_ptr StorageRabbitMQ::addSettings(ContextPtr local_context) const { auto modified_context = Context::createCopy(local_context); - modified_context->setSetting("input_format_skip_unknown_fields", true); + modified_context->setSetting("input_format_skip_unknown_fields", Field{true}); modified_context->setSetting("input_format_allow_errors_ratio", 0.); modified_context->setSetting("input_format_allow_errors_num", rabbitmq_settings->rabbitmq_skip_broken_messages.value); diff --git a/src/Storages/StorageJoin.cpp b/src/Storages/StorageJoin.cpp index 983b9213a35..d9970bab22c 100644 --- a/src/Storages/StorageJoin.cpp +++ b/src/Storages/StorageJoin.cpp @@ -408,7 +408,7 @@ private: if (!position) position = decltype(position)( - static_cast(new typename Map::const_iterator(map.begin())), + static_cast(new typename Map::const_iterator(map.begin())), //-V572 [](void * ptr) { delete reinterpret_cast(ptr); }); auto & it = *reinterpret_cast(position.get()); diff --git a/src/Storages/StorageMerge.cpp b/src/Storages/StorageMerge.cpp index bf7141576e8..1110b850ba9 100644 --- a/src/Storages/StorageMerge.cpp +++ b/src/Storages/StorageMerge.cpp @@ -241,7 +241,7 @@ Pipe StorageMerge::read( * since there is no certainty that it works when one of table is MergeTree and other is not. */ auto modified_context = Context::createCopy(local_context); - modified_context->setSetting("optimize_move_to_prewhere", false); + modified_context->setSetting("optimize_move_to_prewhere", Field{false}); /// What will be result structure depending on query processed stage in source tables? Block header = getHeaderForProcessingStage(*this, column_names, metadata_snapshot, query_info, local_context, processed_stage); From ce9eda6a87195a2996fab88da8047eff87d47e4f Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sat, 8 May 2021 23:56:17 +0300 Subject: [PATCH 21/26] Fix warnings by PVS-Studio --- src/Common/Dwarf.cpp | 4 ++-- src/Common/Dwarf.h | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Common/Dwarf.cpp b/src/Common/Dwarf.cpp index d0b3244dac2..18e9315d5c3 100644 --- a/src/Common/Dwarf.cpp +++ b/src/Common/Dwarf.cpp @@ -804,7 +804,7 @@ bool Dwarf::findLocation( findSubProgramDieForAddress(cu, die, address, base_addr_cu, subprogram); // Subprogram is the DIE of caller function. - if (check_inline && subprogram.abbr.has_children) + if (/*check_inline &&*/ subprogram.abbr.has_children) { // Use an extra location and get its call file and call line, so that // they can be used for the second last location when we don't have @@ -832,7 +832,7 @@ bool Dwarf::findLocation( // file+line of the non-inlined outer function making the call. // locationInfo.name is already set by the caller by looking up the // non-inlined function @address belongs to. - info.has_file_and_line = true; + info.has_file_and_line = true; //-V1048 info.file = call_locations[0].file; info.line = call_locations[0].line; diff --git a/src/Common/Dwarf.h b/src/Common/Dwarf.h index 9ea940c3380..4712daad28c 100644 --- a/src/Common/Dwarf.h +++ b/src/Common/Dwarf.h @@ -121,7 +121,7 @@ public: struct CallLocation { Path file = {}; - uint64_t line; + uint64_t line = 0; std::string_view name; }; @@ -202,8 +202,8 @@ private: // Abbreviation for a Debugging Information Entry. struct DIEAbbreviation { - uint64_t code; - uint64_t tag; + uint64_t code = 0; + uint64_t tag = 0; bool has_children = false; std::string_view attributes; From 903c306e048c7b9efcd0a0e5ad1394c1c1577d49 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sun, 9 May 2021 01:09:16 +0300 Subject: [PATCH 22/26] Fix some PVS-Studio warnings --- src/Common/Volnitsky.h | 2 +- src/Core/SettingsQuirks.cpp | 18 +++++------------- .../PushingToViewsBlockOutputStream.cpp | 2 +- .../SerializationLowCardinality.cpp | 2 +- src/Interpreters/ActionsVisitor.cpp | 2 +- src/Interpreters/ProcessList.h | 2 +- 6 files changed, 10 insertions(+), 18 deletions(-) diff --git a/src/Common/Volnitsky.h b/src/Common/Volnitsky.h index c674015fba9..13cef28ddcf 100644 --- a/src/Common/Volnitsky.h +++ b/src/Common/Volnitsky.h @@ -303,7 +303,7 @@ namespace VolnitskyTraits { /// ngram for Ul chars.c0 = c0u; - chars.c1 = c1l; + chars.c1 = c1l; //-V1048 putNGramBase(n, offset); } diff --git a/src/Core/SettingsQuirks.cpp b/src/Core/SettingsQuirks.cpp index 3bf9047af3a..6d584fe8906 100644 --- a/src/Core/SettingsQuirks.cpp +++ b/src/Core/SettingsQuirks.cpp @@ -4,9 +4,7 @@ #ifdef __linux__ #include -#endif -#ifdef __linux__ /// Detect does epoll_wait with nested epoll fds works correctly. /// Polling nested epoll fds from epoll_wait is required for async_socket_for_remote and use_hedged_requests. /// @@ -16,21 +14,15 @@ /// [2]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0c54a6a44bf3 bool nestedEpollWorks(Poco::Logger * log) { - bool nested_epoll_works = #if (LINUX_VERSION_CODE >= KERNEL_VERSION(5, 5, 0)) && (LINUX_VERSION_CODE < KERNEL_VERSION(5, 6, 13)) /// the check is correct since there will be no more 5.5.x releases. - false -#else - true -#endif - ; - - if (!nested_epoll_works) - { if (log) LOG_WARNING(log, "Nested epoll_wait has some issues on kernels [5.5.0, 5.6.13). You should upgrade it to avoid possible issues."); - } - return nested_epoll_works; + return false; +#else + (void)log; + return true; +#endif } #else bool nestedEpollWorks(Poco::Logger *) { return true; } diff --git a/src/DataStreams/PushingToViewsBlockOutputStream.cpp b/src/DataStreams/PushingToViewsBlockOutputStream.cpp index 16baf4377e0..c09420cab6f 100644 --- a/src/DataStreams/PushingToViewsBlockOutputStream.cpp +++ b/src/DataStreams/PushingToViewsBlockOutputStream.cpp @@ -63,7 +63,7 @@ PushingToViewsBlockOutputStream::PushingToViewsBlockOutputStream( // Do not deduplicate insertions into MV if the main insertion is Ok if (disable_deduplication_for_children) - insert_context->setSetting("insert_deduplicate", false); + insert_context->setSetting("insert_deduplicate", Field{false}); // Separate min_insert_block_size_rows/min_insert_block_size_bytes for children if (insert_settings.min_insert_block_size_rows_for_materialized_views) diff --git a/src/DataTypes/Serializations/SerializationLowCardinality.cpp b/src/DataTypes/Serializations/SerializationLowCardinality.cpp index 41d9a4100e0..69a41d83138 100644 --- a/src/DataTypes/Serializations/SerializationLowCardinality.cpp +++ b/src/DataTypes/Serializations/SerializationLowCardinality.cpp @@ -160,7 +160,7 @@ struct IndexesSerializationType return std::make_shared(); if (type == TUInt32) return std::make_shared(); - if (type == TUInt64) + if (type == TUInt64) //-V547 return std::make_shared(); throw Exception("Can't create DataType from IndexesSerializationType.", ErrorCodes::LOGICAL_ERROR); diff --git a/src/Interpreters/ActionsVisitor.cpp b/src/Interpreters/ActionsVisitor.cpp index b463ab0a586..b00c74601f1 100644 --- a/src/Interpreters/ActionsVisitor.cpp +++ b/src/Interpreters/ActionsVisitor.cpp @@ -207,7 +207,7 @@ static Block createBlockFromAST(const ASTPtr & node, const DataTypes & types, Co assert(tuple || func); - size_t tuple_size = tuple ? tuple->size() : func->arguments->children.size(); + size_t tuple_size = tuple ? tuple->size() : func->arguments->children.size(); //-V1004 if (tuple_size != num_columns) throw Exception("Incorrect size of tuple in set: " + toString(tuple_size) + " instead of " + toString(num_columns), ErrorCodes::INCORRECT_ELEMENT_OF_SET); diff --git a/src/Interpreters/ProcessList.h b/src/Interpreters/ProcessList.h index 3eeea9c8e5b..fca28b031db 100644 --- a/src/Interpreters/ProcessList.h +++ b/src/Interpreters/ProcessList.h @@ -276,7 +276,7 @@ protected: /// List of queries Container processes; - size_t max_size; /// 0 means no limit. Otherwise, when limit exceeded, an exception is thrown. + size_t max_size = 0; /// 0 means no limit. Otherwise, when limit exceeded, an exception is thrown. /// Stores per-user info: queries, statistics and limits UserToQueries user_to_queries; From 9ccf7f4864daa1d236a03b17201e6c6bfabc5fe0 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sun, 9 May 2021 01:54:46 +0300 Subject: [PATCH 23/26] Maybe fix error --- src/Common/HyperLogLogCounter.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Common/HyperLogLogCounter.h b/src/Common/HyperLogLogCounter.h index 5e74020ab3d..1cb014a050a 100644 --- a/src/Common/HyperLogLogCounter.h +++ b/src/Common/HyperLogLogCounter.h @@ -115,7 +115,7 @@ struct IntermediateDenominator -class Denominator> { @@ -159,7 +159,7 @@ private: /// Used when rank storage is big. template -class Denominator> { From f476a5c5d7bc668751c7b02c7d0c1da6cd038896 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sun, 9 May 2021 03:28:10 +0300 Subject: [PATCH 24/26] utils/generate-ya-make/generate-ya-make.sh --- src/Interpreters/ya.make | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Interpreters/ya.make b/src/Interpreters/ya.make index 105e1e11365..480c368c640 100644 --- a/src/Interpreters/ya.make +++ b/src/Interpreters/ya.make @@ -39,6 +39,7 @@ SRCS( CollectJoinOnKeysVisitor.cpp ColumnAliasesVisitor.cpp Context.cpp + ConvertStringsToEnumVisitor.cpp CrashLog.cpp CrossToInnerJoinVisitor.cpp DDLTask.cpp From 5618352512635fdc7bb51c90fb39eb99787a9064 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sun, 9 May 2021 17:48:11 +0300 Subject: [PATCH 25/26] Fix clang-tidy --- src/Interpreters/ConvertStringsToEnumVisitor.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Interpreters/ConvertStringsToEnumVisitor.cpp b/src/Interpreters/ConvertStringsToEnumVisitor.cpp index 85971457c36..62f7b825936 100644 --- a/src/Interpreters/ConvertStringsToEnumVisitor.cpp +++ b/src/Interpreters/ConvertStringsToEnumVisitor.cpp @@ -144,13 +144,13 @@ void ConvertStringsToEnumMatcher::visit(ASTFunction & function_node, Data & data if (function_node.arguments->children.size() != 2) return; - auto literal1 = function_node.arguments->children[1]->as(); - auto literal2 = function_node.arguments->children[2]->as(); + const ASTLiteral * literal1 = function_node.arguments->children[1]->as(); + const ASTLiteral * literal2 = function_node.arguments->children[2]->as(); if (!literal1 || !literal2) return; - if (String(literal1->value.getTypeName()) != "String" || - String(literal2->value.getTypeName()) != "String") + if (literal1->value.getTypeName() != std::string_view{"String"} + || literal2->value.getTypeName() != std::string_view{"String"}) return; changeIfArguments(function_node.arguments->children[1], @@ -161,13 +161,13 @@ void ConvertStringsToEnumMatcher::visit(ASTFunction & function_node, Data & data if (function_node.arguments->children.size() != 4) return; - auto literal_to = function_node.arguments->children[2]->as(); - auto literal_other = function_node.arguments->children[3]->as(); + const ASTLiteral * literal_to = function_node.arguments->children[2]->as(); + const ASTLiteral * literal_other = function_node.arguments->children[3]->as(); if (!literal_to || !literal_other) return; - if (String(literal_to->value.getTypeName()) != "Array" || - String(literal_other->value.getTypeName()) != "String") + if (literal_to->value.getTypeName() != std::string_view{"Array"} + || literal_other->value.getTypeName() != std::string_view{"String"}) return; Array array_to = literal_to->value.get(); From 02fd1d4bc3096d2b7e27e2edc94852098a847deb Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sun, 9 May 2021 20:02:22 +0300 Subject: [PATCH 26/26] Fix clang-tidy --- src/Interpreters/ConvertStringsToEnumVisitor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Interpreters/ConvertStringsToEnumVisitor.cpp b/src/Interpreters/ConvertStringsToEnumVisitor.cpp index 62f7b825936..fa2e0b6613a 100644 --- a/src/Interpreters/ConvertStringsToEnumVisitor.cpp +++ b/src/Interpreters/ConvertStringsToEnumVisitor.cpp @@ -171,7 +171,7 @@ void ConvertStringsToEnumMatcher::visit(ASTFunction & function_node, Data & data return; Array array_to = literal_to->value.get(); - if (array_to.size() == 0) + if (array_to.empty()) return; bool to_strings = checkSameType(array_to, "String");