diff --git a/src/Analyzer/Passes/QueryAnalysisPass.cpp b/src/Analyzer/Passes/QueryAnalysisPass.cpp index 3351d08bd8a..fee040d973d 100644 --- a/src/Analyzer/Passes/QueryAnalysisPass.cpp +++ b/src/Analyzer/Passes/QueryAnalysisPass.cpp @@ -1943,7 +1943,7 @@ void QueryAnalyzer::validateTableExpressionModifiers(const QueryTreeNodePtr & ta if (!table_node && !table_function_node && !query_node && !union_node) throw Exception(ErrorCodes::LOGICAL_ERROR, - "Unexpected table expression. Expected table, table function, query or union node. Actual {}", + "Unexpected table expression. Expected table, table function, query or union node. Table node: {}, scope node: {}", table_expression_node->formatASTForErrorMessage(), scope.scope_node->formatASTForErrorMessage()); @@ -5726,7 +5726,7 @@ void QueryAnalyzer::resolveQueryJoinTreeNode(QueryTreeNodePtr & join_tree_node, case QueryTreeNodeType::IDENTIFIER: { throw Exception(ErrorCodes::LOGICAL_ERROR, - "Identifiers in FROM section must be already resolved. In scope {}", + "Identifiers in FROM section must be already resolved. Node {}, scope {}", join_tree_node->formatASTForErrorMessage(), scope.scope_node->formatASTForErrorMessage()); } diff --git a/src/Common/Exception.h b/src/Common/Exception.h index 357841a7745..8177d92b59b 100644 --- a/src/Common/Exception.h +++ b/src/Common/Exception.h @@ -23,7 +23,10 @@ struct FormatStringHelperImpl std::string_view message_format_string; fmt::format_string fmt_str; template - consteval FormatStringHelperImpl(T && str) : message_format_string(tryGetStaticFormatString(str)), fmt_str(std::forward(str)) {} + consteval FormatStringHelperImpl(T && str) : message_format_string(tryGetStaticFormatString(str)), fmt_str(std::forward(str)) + { + formatStringCheckArgsNumImpl(message_format_string, sizeof...(Args)); + } template FormatStringHelperImpl(fmt::basic_runtime && str) : message_format_string(), fmt_str(std::forward>(str)) {} diff --git a/src/Common/LoggingFormatStringHelpers.h b/src/Common/LoggingFormatStringHelpers.h index 8d1a1a8ec67..18cab0c84fe 100644 --- a/src/Common/LoggingFormatStringHelpers.h +++ b/src/Common/LoggingFormatStringHelpers.h @@ -53,3 +53,64 @@ template constexpr auto firstArg(T && x, Ts &&...) /// For implicit conversion of fmt::basic_runtime<> to char* for std::string ctor template constexpr auto firstArg(fmt::basic_runtime && data, Ts &&...) { return data.str.data(); } +consteval ssize_t formatStringCountArgsNum(const char * const str, size_t len) +{ + /// It does not count named args, but we don't use them + size_t cnt = 0; + size_t i = 0; + while (i + 1 < len) + { + if (str[i] == '{' && str[i + 1] == '}') + { + i += 2; + cnt += 1; + } + else if (str[i] == '{') + { + /// Ignore checks for complex formatting like "{:.3f}" + return -1; + } + else + { + i += 1; + } + } + return cnt; +} + +[[noreturn]] void functionThatFailsCompilationOfConstevalFunctions(const char * error) +{ + throw std::runtime_error(error); +} + +/// fmt::format checks that there are enough arguments, but ignores extra arguments (e.g. fmt::format("{}", 1, 2) compiles) +/// This function will fail to compile if the number of "{}" substitutions does not exactly match +consteval void formatStringCheckArgsNumImpl(std::string_view str, size_t nargs) +{ + if (str.empty()) + return; + ssize_t cnt = formatStringCountArgsNum(str.data(), str.size()); + if (0 <= cnt && cnt != nargs) + functionThatFailsCompilationOfConstevalFunctions("unexpected number of arguments in a format string"); +} + +template +struct CheckArgsNumHelperImpl +{ + //std::enable_if_t, PreformattedMessage>> + template + consteval CheckArgsNumHelperImpl(T && str) + { + formatStringCheckArgsNumImpl(tryGetStaticFormatString(str), sizeof...(Args)); + } + + /// No checks for fmt::runtime and PreformattedMessage + template CheckArgsNumHelperImpl(fmt::basic_runtime &&) {} + template<> CheckArgsNumHelperImpl(PreformattedMessage &) {} + template<> CheckArgsNumHelperImpl(const PreformattedMessage &) {} + template<> CheckArgsNumHelperImpl(PreformattedMessage &&) {} + +}; + +template using CheckArgsNumHelper = CheckArgsNumHelperImpl...>; +template void formatStringCheckArgsNum(CheckArgsNumHelper, Args &&...) {} diff --git a/src/Common/NamedCollections/NamedCollectionUtils.cpp b/src/Common/NamedCollections/NamedCollectionUtils.cpp index d38a889554b..8beaa38210e 100644 --- a/src/Common/NamedCollections/NamedCollectionUtils.cpp +++ b/src/Common/NamedCollections/NamedCollectionUtils.cpp @@ -187,7 +187,7 @@ public: { throw Exception( ErrorCodes::BAD_ARGUMENTS, - "Value with key `{}` is used twice in the SET query", + "Value with key `{}` is used twice in the SET query (collection name: {})", name, query.collection_name); } } diff --git a/src/Common/TLDListsHolder.cpp b/src/Common/TLDListsHolder.cpp index 75e57d9b9d4..c493a88010b 100644 --- a/src/Common/TLDListsHolder.cpp +++ b/src/Common/TLDListsHolder.cpp @@ -100,7 +100,7 @@ size_t TLDListsHolder::parseAndAddTldList(const std::string & name, const std::s tld_list_tmp.emplace(line, TLDType::TLD_REGULAR); } if (!in.eof()) - throw Exception(ErrorCodes::LOGICAL_ERROR, "Not all list had been read", name); + throw Exception(ErrorCodes::LOGICAL_ERROR, "Not all list had been read: {}", name); TLDList tld_list(tld_list_tmp.size()); for (const auto & [host, type] : tld_list_tmp) diff --git a/src/Common/logger_useful.h b/src/Common/logger_useful.h index 52c32ff1715..170ca1c3b10 100644 --- a/src/Common/logger_useful.h +++ b/src/Common/logger_useful.h @@ -57,6 +57,7 @@ namespace if (_is_clients_log || _logger->is((PRIORITY))) \ { \ std::string formatted_message = numArgs(__VA_ARGS__) > 1 ? fmt::format(__VA_ARGS__) : firstArg(__VA_ARGS__); \ + formatStringCheckArgsNum(__VA_ARGS__); \ if (auto _channel = _logger->getChannel()) \ { \ std::string file_function; \ diff --git a/src/DataTypes/DataTypeObject.cpp b/src/DataTypes/DataTypeObject.cpp index 659f69b6c68..720436d0e0d 100644 --- a/src/DataTypes/DataTypeObject.cpp +++ b/src/DataTypes/DataTypeObject.cpp @@ -57,7 +57,7 @@ static DataTypePtr create(const ASTPtr & arguments) { if (func->name != "Nullable" || func->arguments->children.size() != 1) throw Exception(ErrorCodes::UNEXPECTED_AST_STRUCTURE, - "Expected 'Nullable()' as parameter for type Object", func->name); + "Expected 'Nullable()' as parameter for type Object (function: {})", func->name); schema_argument = func->arguments->children[0]; is_nullable = true; diff --git a/src/Databases/MySQL/DatabaseMySQL.cpp b/src/Databases/MySQL/DatabaseMySQL.cpp index 6c1c8b35f43..85944319999 100644 --- a/src/Databases/MySQL/DatabaseMySQL.cpp +++ b/src/Databases/MySQL/DatabaseMySQL.cpp @@ -136,7 +136,7 @@ ASTPtr DatabaseMySQL::getCreateTableQueryImpl(const String & table_name, Context if (local_tables_cache.find(table_name) == local_tables_cache.end()) { if (throw_on_error) - throw Exception(ErrorCodes::UNKNOWN_TABLE, "MySQL table {} doesn't exist.", database_name_in_mysql, table_name); + throw Exception(ErrorCodes::UNKNOWN_TABLE, "MySQL table {}.{} doesn't exist.", database_name_in_mysql, table_name); return nullptr; } @@ -180,7 +180,7 @@ time_t DatabaseMySQL::getObjectMetadataModificationTime(const String & table_nam fetchTablesIntoLocalCache(getContext()); if (local_tables_cache.find(table_name) == local_tables_cache.end()) - throw Exception(ErrorCodes::UNKNOWN_TABLE, "MySQL table {} doesn't exist.", database_name_in_mysql, table_name); + throw Exception(ErrorCodes::UNKNOWN_TABLE, "MySQL table {}.{} doesn't exist.", database_name_in_mysql, table_name); return time_t(local_tables_cache[table_name].first); } diff --git a/src/Databases/PostgreSQL/DatabasePostgreSQL.cpp b/src/Databases/PostgreSQL/DatabasePostgreSQL.cpp index 4465dbb3d9d..f2d57968baa 100644 --- a/src/Databases/PostgreSQL/DatabasePostgreSQL.cpp +++ b/src/Databases/PostgreSQL/DatabasePostgreSQL.cpp @@ -214,12 +214,12 @@ void DatabasePostgreSQL::attachTable(ContextPtr /* context_ */, const String & t if (!checkPostgresTable(table_name)) throw Exception(ErrorCodes::UNKNOWN_TABLE, - "Cannot attach PostgreSQL table {} because it does not exist in PostgreSQL", + "Cannot attach PostgreSQL table {} because it does not exist in PostgreSQL (database: {})", getTableNameForLogs(table_name), database_name); if (!detached_or_dropped.contains(table_name)) throw Exception(ErrorCodes::TABLE_ALREADY_EXISTS, - "Cannot attach PostgreSQL table {} because it already exists", + "Cannot attach PostgreSQL table {} because it already exists (database: {})", getTableNameForLogs(table_name), database_name); if (cache_tables) diff --git a/src/Functions/FunctionsAES.h b/src/Functions/FunctionsAES.h index f0edf6237eb..c748be5b9b8 100644 --- a/src/Functions/FunctionsAES.h +++ b/src/Functions/FunctionsAES.h @@ -597,7 +597,7 @@ private: if (input_value.size < tag_size) throw Exception(ErrorCodes::BAD_ARGUMENTS, "Encrypted data is too short: only {} bytes, " "should contain at least {} bytes of a tag.", - input_value.size, block_size, tag_size); + input_value.size, tag_size); input_value.size -= tag_size; } diff --git a/src/Functions/FunctionsExternalDictionaries.h b/src/Functions/FunctionsExternalDictionaries.h index 5c54fc5e27d..48ecc13da39 100644 --- a/src/Functions/FunctionsExternalDictionaries.h +++ b/src/Functions/FunctionsExternalDictionaries.h @@ -182,7 +182,7 @@ public: if (!isString(arguments[0])) throw Exception(ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT, - "Illegal type {} of first argument of function, expected a string", + "Illegal type {} of first argument of function {}, expected a string", arguments[0]->getName(), getName()); @@ -416,7 +416,7 @@ public: if (!(range_col_type->isValueRepresentedByInteger() && range_col_type->getSizeOfValueInMemory() <= sizeof(Int64))) throw Exception(ErrorCodes::ILLEGAL_COLUMN, - "Illegal type {} of fourth argument of function must be convertible to Int64.", + "Illegal type {} of fourth argument of function {} must be convertible to Int64.", range_col_type->getName(), getName()); diff --git a/src/IO/Lz4DeflatingWriteBuffer.cpp b/src/IO/Lz4DeflatingWriteBuffer.cpp index c2eb175d5a9..c3a1b8282c3 100644 --- a/src/IO/Lz4DeflatingWriteBuffer.cpp +++ b/src/IO/Lz4DeflatingWriteBuffer.cpp @@ -37,8 +37,7 @@ Lz4DeflatingWriteBuffer::Lz4DeflatingWriteBuffer( throw Exception( ErrorCodes::LZ4_ENCODER_FAILED, "creation of LZ4 compression context failed. LZ4F version: {}", - LZ4F_VERSION, - ErrorCodes::LZ4_ENCODER_FAILED); + LZ4F_VERSION); } Lz4DeflatingWriteBuffer::~Lz4DeflatingWriteBuffer() diff --git a/src/IO/S3/copyDataToS3.cpp b/src/IO/S3/copyDataToS3.cpp index f7018c51359..804e67803a9 100644 --- a/src/IO/S3/copyDataToS3.cpp +++ b/src/IO/S3/copyDataToS3.cpp @@ -131,7 +131,7 @@ namespace LOG_TRACE(log, "Completing multipart upload. Bucket: {}, Key: {}, Upload_id: {}, Parts: {}", dest_bucket, dest_key, multipart_upload_id, part_tags.size()); if (part_tags.empty()) - throw Exception("Failed to complete multipart upload. No parts have uploaded", ErrorCodes::S3_ERROR); + throw Exception(ErrorCodes::S3_ERROR, "Failed to complete multipart upload. No parts have uploaded"); Aws::S3::Model::CompleteMultipartUploadRequest request; request.SetBucket(dest_bucket); @@ -680,7 +680,7 @@ namespace if (!outcome.IsSuccess()) { abortMultipartUpload(); - throw Exception(outcome.GetError().GetMessage(), ErrorCodes::S3_ERROR); + throw Exception::createDeprecated(outcome.GetError().GetMessage(), ErrorCodes::S3_ERROR); } return outcome.GetResult().GetCopyPartResult().GetETag(); diff --git a/src/Interpreters/Context.cpp b/src/Interpreters/Context.cpp index 6872d9c0dce..08d3d368159 100644 --- a/src/Interpreters/Context.cpp +++ b/src/Interpreters/Context.cpp @@ -2046,7 +2046,7 @@ void Context::setQueryResultCache(size_t max_size_in_bytes, size_t max_entries, auto lock = getLock(); if (shared->query_result_cache) - throw Exception("Query result cache has been already created.", ErrorCodes::LOGICAL_ERROR); + throw Exception(ErrorCodes::LOGICAL_ERROR, "Query result cache has been already created."); shared->query_result_cache = std::make_shared(max_size_in_bytes, max_entries, max_entry_size_in_bytes, max_entry_size_in_records); } diff --git a/src/Interpreters/InterpreterSystemQuery.cpp b/src/Interpreters/InterpreterSystemQuery.cpp index c20aa66313f..5fa5e20be18 100644 --- a/src/Interpreters/InterpreterSystemQuery.cpp +++ b/src/Interpreters/InterpreterSystemQuery.cpp @@ -288,7 +288,7 @@ BlockIO InterpreterSystemQuery::execute() copyData(res->out, out); copyData(res->err, out); if (!out.str().empty()) - LOG_DEBUG(log, "The command returned output: {}", command, out.str()); + LOG_DEBUG(log, "The command {} returned output: {}", command, out.str()); res->wait(); break; } diff --git a/src/Interpreters/ProcessList.cpp b/src/Interpreters/ProcessList.cpp index aab8cff71ad..9ee6cf1afe8 100644 --- a/src/Interpreters/ProcessList.cpp +++ b/src/Interpreters/ProcessList.cpp @@ -637,7 +637,7 @@ void ProcessList::decreaseQueryKindAmount(const IAST::QueryKind & query_kind) if (found == query_kind_amounts.end()) throw Exception(ErrorCodes::LOGICAL_ERROR, "Wrong query kind amount: decrease before increase on '{}'", query_kind); else if (found->second == 0) - throw Exception(ErrorCodes::LOGICAL_ERROR, "Wrong query kind amount: decrease to negative on '{}'", query_kind, found->second); + throw Exception(ErrorCodes::LOGICAL_ERROR, "Wrong query kind amount: decrease to negative on '{}', {}", query_kind, found->second); else found->second -= 1; } diff --git a/src/Processors/Formats/Impl/BSONEachRowRowInputFormat.cpp b/src/Processors/Formats/Impl/BSONEachRowRowInputFormat.cpp index 349b063fabe..c4ffce2bc65 100644 --- a/src/Processors/Formats/Impl/BSONEachRowRowInputFormat.cpp +++ b/src/Processors/Formats/Impl/BSONEachRowRowInputFormat.cpp @@ -458,7 +458,7 @@ bool BSONEachRowRowInputFormat::readField(IColumn & column, const DataTypePtr & if (!format_settings.null_as_default) throw Exception(ErrorCodes::ILLEGAL_COLUMN, "Cannot insert BSON Null value into non-nullable column with type {}", - getBSONTypeName(bson_type), data_type->getName()); + data_type->getName()); column.insertDefault(); return false; diff --git a/src/Storages/MergeTree/MergeTreeData.cpp b/src/Storages/MergeTree/MergeTreeData.cpp index 8c67f0f1625..50e311efaa1 100644 --- a/src/Storages/MergeTree/MergeTreeData.cpp +++ b/src/Storages/MergeTree/MergeTreeData.cpp @@ -1776,7 +1776,7 @@ catch (...) { LOG_ERROR(log, "Loading of outdated parts failed. " "Will terminate to avoid undefined behaviour due to inconsistent set of parts. " - "Exception: ", getCurrentExceptionMessage(true)); + "Exception: {}", getCurrentExceptionMessage(true)); std::terminate(); } diff --git a/src/Storages/MergeTree/MergeTreeDataPartWide.cpp b/src/Storages/MergeTree/MergeTreeDataPartWide.cpp index a31b88365c2..a80f7d97326 100644 --- a/src/Storages/MergeTree/MergeTreeDataPartWide.cpp +++ b/src/Storages/MergeTree/MergeTreeDataPartWide.cpp @@ -207,7 +207,7 @@ void MergeTreeDataPartWide::checkConsistency(bool require_part_metadata) const if (!checksums.files.contains(bin_file_name)) throw Exception( ErrorCodes::NO_FILE_IN_DATA_PART, - "No {} file checksum for column {} in part ", + "No {} file checksum for column {} in part {}", bin_file_name, name_type.name, getDataPartStorage().getFullPath()); }); } diff --git a/src/Storages/MergeTree/PartMetadataManagerWithCache.cpp b/src/Storages/MergeTree/PartMetadataManagerWithCache.cpp index 90fd25bc4e7..b6260d5edb6 100644 --- a/src/Storages/MergeTree/PartMetadataManagerWithCache.cpp +++ b/src/Storages/MergeTree/PartMetadataManagerWithCache.cpp @@ -177,7 +177,7 @@ void PartMetadataManagerWithCache::assertAllDeleted(bool include_projection) con { throw Exception( ErrorCodes::LOGICAL_ERROR, - "Data part {} with type {} with meta file {} with projection name still in cache", + "Data part {} with type {} with meta file {} with projection name {} still in cache", part->name, part->getType().toString(), file_path, diff --git a/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp b/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp index 2f290c93591..f450604fded 100644 --- a/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp +++ b/src/Storages/PostgreSQL/PostgreSQLReplicationHandler.cpp @@ -619,7 +619,8 @@ void PostgreSQLReplicationHandler::removeTableFromPublication(pqxx::nontransacti catch (const pqxx::undefined_table &) { /// Removing table from replication must succeed even if table does not exist in PostgreSQL. - LOG_WARNING(log, "Did not remove table {} from publication, because table does not exist in PostgreSQL", doubleQuoteWithSchema(table_name), publication_name); + LOG_WARNING(log, "Did not remove table {} from publication, because table does not exist in PostgreSQL (publication: {})", + doubleQuoteWithSchema(table_name), publication_name); } } @@ -756,7 +757,7 @@ std::set PostgreSQLReplicationHandler::fetchRequiredTables() } LOG_ERROR(log, - "Publication {} already exists, but specified tables list differs from publication tables list in tables: {}. ", + "Publication {} already exists, but specified tables list differs from publication tables list in tables: {}. " "Will use tables list from setting. " "To avoid redundant work, you can try ALTER PUBLICATION query to remove redundant tables. " "Or you can you ALTER SETTING. " diff --git a/src/Storages/StorageDictionary.cpp b/src/Storages/StorageDictionary.cpp index 0eb3eb6b94e..5d1f08771be 100644 --- a/src/Storages/StorageDictionary.cpp +++ b/src/Storages/StorageDictionary.cpp @@ -153,7 +153,7 @@ void StorageDictionary::checkTableCanBeDropped() const dictionary_name); if (location == Location::DictionaryDatabase) throw Exception(ErrorCodes::CANNOT_DETACH_DICTIONARY_AS_TABLE, - "Cannot drop/detach table from a database with DICTIONARY engine, use DROP DICTIONARY or DETACH DICTIONARY query instead", + "Cannot drop/detach table '{}' from a database with DICTIONARY engine, use DROP DICTIONARY or DETACH DICTIONARY query instead", dictionary_name); } diff --git a/src/Storages/StorageS3Settings.cpp b/src/Storages/StorageS3Settings.cpp index fc505f67966..ee704b3e750 100644 --- a/src/Storages/StorageS3Settings.cpp +++ b/src/Storages/StorageS3Settings.cpp @@ -116,20 +116,17 @@ void S3Settings::RequestSettings::PartUploadSettings::validate() if (!upload_part_size_multiply_factor) throw Exception( ErrorCodes::INVALID_SETTING_VALUE, - "Setting upload_part_size_multiply_factor cannot be zero", - upload_part_size_multiply_factor); + "Setting upload_part_size_multiply_factor cannot be zero"); if (!upload_part_size_multiply_parts_count_threshold) throw Exception( ErrorCodes::INVALID_SETTING_VALUE, - "Setting upload_part_size_multiply_parts_count_threshold cannot be zero", - upload_part_size_multiply_parts_count_threshold); + "Setting upload_part_size_multiply_parts_count_threshold cannot be zero"); if (!max_part_number) throw Exception( ErrorCodes::INVALID_SETTING_VALUE, - "Setting max_part_number cannot be zero", - max_part_number); + "Setting max_part_number cannot be zero"); static constexpr size_t max_part_number_limit = 10000; if (max_part_number > max_part_number_limit)