From 06a131d75181a7b53c0553d944662a8bb55e8a2c Mon Sep 17 00:00:00 2001 From: avogar Date: Tue, 21 May 2024 18:46:21 +0000 Subject: [PATCH 01/40] Fix abort on uncaught exception in ~WriteBufferFromFileDescriptor in StatusFile --- src/Common/StatusFile.cpp | 15 ++++++++++++--- src/IO/WriteBufferFromFileDescriptor.cpp | 9 ++++++++- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/Common/StatusFile.cpp b/src/Common/StatusFile.cpp index ba7595ae6d7..80464f38082 100644 --- a/src/Common/StatusFile.cpp +++ b/src/Common/StatusFile.cpp @@ -85,9 +85,18 @@ StatusFile::StatusFile(std::string path_, FillFunction fill_) /// Write information about current server instance to the file. WriteBufferFromFileDescriptor out(fd, 1024); - fill(out); - /// Finalize here to avoid throwing exceptions in destructor. - out.finalize(); + try + { + fill(out); + /// Finalize here to avoid throwing exceptions in destructor. + out.finalize(); + } + catch (...) + { + /// Finalize in case of exception to avoid throwing exceptions in destructor + out.finalize(); + throw; + } } catch (...) { diff --git a/src/IO/WriteBufferFromFileDescriptor.cpp b/src/IO/WriteBufferFromFileDescriptor.cpp index 813ef0deab9..a758f99458d 100644 --- a/src/IO/WriteBufferFromFileDescriptor.cpp +++ b/src/IO/WriteBufferFromFileDescriptor.cpp @@ -105,7 +105,14 @@ WriteBufferFromFileDescriptor::WriteBufferFromFileDescriptor( WriteBufferFromFileDescriptor::~WriteBufferFromFileDescriptor() { - finalize(); + try + { + finalize(); + } + catch (...) + { + tryLogCurrentException(__PRETTY_FUNCTION__); + } } void WriteBufferFromFileDescriptor::finalizeImpl() From e2b7ca7d1137aed5c449eb9750b97e9be567681b Mon Sep 17 00:00:00 2001 From: pufit Date: Wed, 29 May 2024 11:54:31 -0400 Subject: [PATCH 02/40] Fix restore from backup for definers --- src/Interpreters/InterpreterCreateQuery.cpp | 8 +++---- src/Interpreters/InterpreterCreateQuery.h | 2 +- .../test_backup_restore_new/test.py | 24 +++++++++++++++++++ 3 files changed, 29 insertions(+), 5 deletions(-) diff --git a/src/Interpreters/InterpreterCreateQuery.cpp b/src/Interpreters/InterpreterCreateQuery.cpp index b30fc8bc092..bf6c4f68947 100644 --- a/src/Interpreters/InterpreterCreateQuery.cpp +++ b/src/Interpreters/InterpreterCreateQuery.cpp @@ -1086,7 +1086,7 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create) create.sql_security = std::make_shared(); if (create.sql_security) - processSQLSecurityOption(getContext(), create.sql_security->as(), create.attach, create.is_materialized_view); + processSQLSecurityOption(getContext(), create.sql_security->as(), create.attach, create.is_materialized_view, is_restore_from_backup); DDLGuardPtr ddl_guard; @@ -1880,7 +1880,7 @@ void InterpreterCreateQuery::addColumnsDescriptionToCreateQueryIfNecessary(ASTCr } } -void InterpreterCreateQuery::processSQLSecurityOption(ContextPtr context_, ASTSQLSecurity & sql_security, bool is_attach, bool is_materialized_view) +void InterpreterCreateQuery::processSQLSecurityOption(ContextPtr context_, ASTSQLSecurity & sql_security, bool is_attach, bool is_materialized_view, bool is_restore_from_backup_) { /// If no SQL security is specified, apply default from default_*_view_sql_security setting. if (!sql_security.type) @@ -1921,7 +1921,7 @@ void InterpreterCreateQuery::processSQLSecurityOption(ContextPtr context_, ASTSQ } /// Checks the permissions for the specified definer user. - if (sql_security.definer && !sql_security.is_definer_current_user && !is_attach) + if (sql_security.definer && !sql_security.is_definer_current_user && !is_attach && !is_restore_from_backup_) { const auto definer_name = sql_security.definer->toString(); @@ -1931,7 +1931,7 @@ void InterpreterCreateQuery::processSQLSecurityOption(ContextPtr context_, ASTSQ context_->checkAccess(AccessType::SET_DEFINER, definer_name); } - if (sql_security.type == SQLSecurityType::NONE && !is_attach) + if (sql_security.type == SQLSecurityType::NONE && !is_attach && !is_restore_from_backup_) context_->checkAccess(AccessType::ALLOW_SQL_SECURITY_NONE); } diff --git a/src/Interpreters/InterpreterCreateQuery.h b/src/Interpreters/InterpreterCreateQuery.h index be4a10eaf1d..c4bfa7b36d8 100644 --- a/src/Interpreters/InterpreterCreateQuery.h +++ b/src/Interpreters/InterpreterCreateQuery.h @@ -82,7 +82,7 @@ public: void extendQueryLogElemImpl(QueryLogElement & elem, const ASTPtr & ast, ContextPtr) const override; /// Check access right, validate definer statement and replace `CURRENT USER` with actual name. - static void processSQLSecurityOption(ContextPtr context_, ASTSQLSecurity & sql_security, bool is_attach = false, bool is_materialized_view = false); + static void processSQLSecurityOption(ContextPtr context_, ASTSQLSecurity & sql_security, bool is_attach = false, bool is_materialized_view = false, bool is_restore_from_backup_ = false); private: struct TableProperties diff --git a/tests/integration/test_backup_restore_new/test.py b/tests/integration/test_backup_restore_new/test.py index ef9e536976b..a6621dce231 100644 --- a/tests/integration/test_backup_restore_new/test.py +++ b/tests/integration/test_backup_restore_new/test.py @@ -168,6 +168,30 @@ def test_restore_table(engine): assert instance.query("SELECT count(), sum(x) FROM test.table") == "100\t4950\n" +def test_restore_materialized_view_with_definer(): + instance.query("CREATE DATABASE test") + instance.query("CREATE TABLE test.test_table (s String) ENGINE = MergeTree ORDER BY s") + instance.query("CREATE USER u1") + instance.query("GRANT SELECT ON *.* TO u1") + instance.query("GRANT INSERT ON *.* TO u1") + + instance.query( + """ + CREATE MATERIALIZED VIEW test.test_mv_1 (s String) + ENGINE = MergeTree ORDER BY s + DEFINER = u1 SQL SECURITY DEFINER + AS SELECT * FROM test.test_table + """ + ) + + backup_name = new_backup_name() + instance.query(f"BACKUP DATABASE test TO {backup_name}") + instance.query("DROP DATABASE test") + instance.query("DROP USER u1") + + instance.query(f"RESTORE DATABASE test FROM {backup_name}") + + @pytest.mark.parametrize( "engine", ["MergeTree", "Log", "TinyLog", "StripeLog", "Memory"] ) From b908421259dab2849c5da8c01d8909ba8966e6c1 Mon Sep 17 00:00:00 2001 From: robot-clickhouse Date: Wed, 29 May 2024 16:05:50 +0000 Subject: [PATCH 03/40] Automatic style fix --- tests/integration/test_backup_restore_new/test.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/integration/test_backup_restore_new/test.py b/tests/integration/test_backup_restore_new/test.py index a6621dce231..f5a63cd69a3 100644 --- a/tests/integration/test_backup_restore_new/test.py +++ b/tests/integration/test_backup_restore_new/test.py @@ -170,7 +170,9 @@ def test_restore_table(engine): def test_restore_materialized_view_with_definer(): instance.query("CREATE DATABASE test") - instance.query("CREATE TABLE test.test_table (s String) ENGINE = MergeTree ORDER BY s") + instance.query( + "CREATE TABLE test.test_table (s String) ENGINE = MergeTree ORDER BY s" + ) instance.query("CREATE USER u1") instance.query("GRANT SELECT ON *.* TO u1") instance.query("GRANT INSERT ON *.* TO u1") From f69918dba0f0dcd3bd90f58691635423d5c3c0b5 Mon Sep 17 00:00:00 2001 From: pufit Date: Wed, 29 May 2024 16:03:29 -0400 Subject: [PATCH 04/40] skip_check_permissions --- src/Interpreters/InterpreterCreateQuery.cpp | 8 ++++---- src/Interpreters/InterpreterCreateQuery.h | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Interpreters/InterpreterCreateQuery.cpp b/src/Interpreters/InterpreterCreateQuery.cpp index bf6c4f68947..a3eaced0d02 100644 --- a/src/Interpreters/InterpreterCreateQuery.cpp +++ b/src/Interpreters/InterpreterCreateQuery.cpp @@ -1086,7 +1086,7 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create) create.sql_security = std::make_shared(); if (create.sql_security) - processSQLSecurityOption(getContext(), create.sql_security->as(), create.attach, create.is_materialized_view, is_restore_from_backup); + processSQLSecurityOption(getContext(), create.sql_security->as(), create.is_materialized_view, /* skip_check_permissions= */ is_restore_from_backup || create.attach); DDLGuardPtr ddl_guard; @@ -1880,7 +1880,7 @@ void InterpreterCreateQuery::addColumnsDescriptionToCreateQueryIfNecessary(ASTCr } } -void InterpreterCreateQuery::processSQLSecurityOption(ContextPtr context_, ASTSQLSecurity & sql_security, bool is_attach, bool is_materialized_view, bool is_restore_from_backup_) +void InterpreterCreateQuery::processSQLSecurityOption(ContextPtr context_, ASTSQLSecurity & sql_security, bool is_materialized_view, bool skip_check_permissions) { /// If no SQL security is specified, apply default from default_*_view_sql_security setting. if (!sql_security.type) @@ -1921,7 +1921,7 @@ void InterpreterCreateQuery::processSQLSecurityOption(ContextPtr context_, ASTSQ } /// Checks the permissions for the specified definer user. - if (sql_security.definer && !sql_security.is_definer_current_user && !is_attach && !is_restore_from_backup_) + if (sql_security.definer && !sql_security.is_definer_current_user && !skip_check_permissions) { const auto definer_name = sql_security.definer->toString(); @@ -1931,7 +1931,7 @@ void InterpreterCreateQuery::processSQLSecurityOption(ContextPtr context_, ASTSQ context_->checkAccess(AccessType::SET_DEFINER, definer_name); } - if (sql_security.type == SQLSecurityType::NONE && !is_attach && !is_restore_from_backup_) + if (sql_security.type == SQLSecurityType::NONE && !skip_check_permissions) context_->checkAccess(AccessType::ALLOW_SQL_SECURITY_NONE); } diff --git a/src/Interpreters/InterpreterCreateQuery.h b/src/Interpreters/InterpreterCreateQuery.h index c4bfa7b36d8..70ef29e6b07 100644 --- a/src/Interpreters/InterpreterCreateQuery.h +++ b/src/Interpreters/InterpreterCreateQuery.h @@ -82,7 +82,7 @@ public: void extendQueryLogElemImpl(QueryLogElement & elem, const ASTPtr & ast, ContextPtr) const override; /// Check access right, validate definer statement and replace `CURRENT USER` with actual name. - static void processSQLSecurityOption(ContextPtr context_, ASTSQLSecurity & sql_security, bool is_attach = false, bool is_materialized_view = false, bool is_restore_from_backup_ = false); + static void processSQLSecurityOption(ContextPtr context_, ASTSQLSecurity & sql_security, bool is_materialized_view = false, bool skip_check_permissions = false); private: struct TableProperties From 63900550c9adba47034f0f136c45d4ef4d34076f Mon Sep 17 00:00:00 2001 From: pufit Date: Mon, 3 Jun 2024 19:16:42 -0400 Subject: [PATCH 05/40] Making use of LoadingStrictnessLevel --- src/Interpreters/InterpreterCreateQuery.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Interpreters/InterpreterCreateQuery.cpp b/src/Interpreters/InterpreterCreateQuery.cpp index a3eaced0d02..5e07f7a27f2 100644 --- a/src/Interpreters/InterpreterCreateQuery.cpp +++ b/src/Interpreters/InterpreterCreateQuery.cpp @@ -1082,11 +1082,14 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create) String current_database = getContext()->getCurrentDatabase(); auto database_name = create.database ? create.getDatabase() : current_database; + bool is_secondary_query = getContext()->getZooKeeperMetadataTransaction() && !getContext()->getZooKeeperMetadataTransaction()->isInitialQuery(); + auto mode = getLoadingStrictnessLevel(create.attach, /*force_attach*/ false, /*has_force_restore_data_flag*/ false, is_secondary_query || is_restore_from_backup); + if (!create.sql_security && create.supportSQLSecurity() && !getContext()->getServerSettings().ignore_empty_sql_security_in_create_view_query) create.sql_security = std::make_shared(); if (create.sql_security) - processSQLSecurityOption(getContext(), create.sql_security->as(), create.is_materialized_view, /* skip_check_permissions= */ is_restore_from_backup || create.attach); + processSQLSecurityOption(getContext(), create.sql_security->as(), create.is_materialized_view, /* skip_check_permissions= */ mode >= LoadingStrictnessLevel::SECONDARY_CREATE); DDLGuardPtr ddl_guard; @@ -1213,9 +1216,6 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create) if (!UserDefinedSQLFunctionFactory::instance().empty()) UserDefinedSQLFunctionVisitor::visit(query_ptr); - bool is_secondary_query = getContext()->getZooKeeperMetadataTransaction() && !getContext()->getZooKeeperMetadataTransaction()->isInitialQuery(); - auto mode = getLoadingStrictnessLevel(create.attach, /*force_attach*/ false, /*has_force_restore_data_flag*/ false, is_secondary_query || is_restore_from_backup); - /// Set and retrieve list of columns, indices and constraints. Set table engine if needed. Rewrite query in canonical way. TableProperties properties = getTablePropertiesAndNormalizeCreateQuery(create, mode); From 823a7d37f3bab8d4aa9cb8b2ff8b7b087e8a3037 Mon Sep 17 00:00:00 2001 From: Han Fei Date: Thu, 6 Jun 2024 20:18:42 +0200 Subject: [PATCH 06/40] support statistics on replicated merge tree --- src/Interpreters/InterpreterCreateQuery.cpp | 6 ++--- src/Parsers/ASTColumnDeclaration.cpp | 10 +++---- src/Parsers/ASTColumnDeclaration.h | 2 +- src/Parsers/ParserCreateQuery.h | 10 +++---- src/Storages/ColumnsDescription.cpp | 4 +++ src/Storages/StatisticsDescription.cpp | 2 +- .../test_manipulate_statistics/test.py | 27 ++++++++++++++++++- 7 files changed, 45 insertions(+), 16 deletions(-) diff --git a/src/Interpreters/InterpreterCreateQuery.cpp b/src/Interpreters/InterpreterCreateQuery.cpp index 66936dc25d7..a51d3e6dade 100644 --- a/src/Interpreters/InterpreterCreateQuery.cpp +++ b/src/Interpreters/InterpreterCreateQuery.cpp @@ -450,8 +450,8 @@ ASTPtr InterpreterCreateQuery::formatColumns(const ColumnsDescription & columns) if (!column.statistics.empty()) { - column_declaration->stat_type = column.statistics.getAST(); - column_declaration->children.push_back(column_declaration->stat_type); + column_declaration->statistics_desc = column.statistics.getAST(); + column_declaration->children.push_back(column_declaration->statistics_desc); } if (column.ttl) @@ -676,7 +676,7 @@ ColumnsDescription InterpreterCreateQuery::getColumnsDescription( } column.statistics.column_name = column.name; /// We assign column name here for better exception error message. - if (col_decl.stat_type) + if (col_decl.statistics_desc) { if (!skip_checks && !context_->getSettingsRef().allow_experimental_statistics) throw Exception(ErrorCodes::INCORRECT_QUERY, "Create table with statistics is now disabled. Turn on allow_experimental_statistics"); diff --git a/src/Parsers/ASTColumnDeclaration.cpp b/src/Parsers/ASTColumnDeclaration.cpp index 6c29e0bf9d5..4a8a3d2967d 100644 --- a/src/Parsers/ASTColumnDeclaration.cpp +++ b/src/Parsers/ASTColumnDeclaration.cpp @@ -39,10 +39,10 @@ ASTPtr ASTColumnDeclaration::clone() const res->children.push_back(res->codec); } - if (stat_type) + if (statistics_desc) { - res->stat_type = stat_type->clone(); - res->children.push_back(res->stat_type); + res->statistics_desc = statistics_desc->clone(); + res->children.push_back(res->statistics_desc); } if (ttl) @@ -111,10 +111,10 @@ void ASTColumnDeclaration::formatImpl(const FormatSettings & format_settings, Fo codec->formatImpl(format_settings, state, frame); } - if (stat_type) + if (statistics_desc) { format_settings.ostr << ' '; - stat_type->formatImpl(format_settings, state, frame); + statistics_desc->formatImpl(format_settings, state, frame); } if (ttl) diff --git a/src/Parsers/ASTColumnDeclaration.h b/src/Parsers/ASTColumnDeclaration.h index d775928d05c..914916d5074 100644 --- a/src/Parsers/ASTColumnDeclaration.h +++ b/src/Parsers/ASTColumnDeclaration.h @@ -19,7 +19,7 @@ public: bool ephemeral_default = false; ASTPtr comment; ASTPtr codec; - ASTPtr stat_type; + ASTPtr statistics_desc; ASTPtr ttl; ASTPtr collation; ASTPtr settings; diff --git a/src/Parsers/ParserCreateQuery.h b/src/Parsers/ParserCreateQuery.h index 27bb524970d..5f6df33176f 100644 --- a/src/Parsers/ParserCreateQuery.h +++ b/src/Parsers/ParserCreateQuery.h @@ -193,7 +193,7 @@ bool IParserColumnDeclaration::parseImpl(Pos & pos, ASTPtr & node, E ASTPtr default_expression; ASTPtr comment_expression; ASTPtr codec_expression; - ASTPtr stat_type_expression; + ASTPtr statistics_desc_expression; ASTPtr ttl_expression; ASTPtr collation_expression; ASTPtr settings; @@ -325,7 +325,7 @@ bool IParserColumnDeclaration::parseImpl(Pos & pos, ASTPtr & node, E if (s_stat.ignore(pos, expected)) { - if (!stat_type_parser.parse(pos, stat_type_expression, expected)) + if (!stat_type_parser.parse(pos, statistics_desc_expression, expected)) return false; } @@ -398,10 +398,10 @@ bool IParserColumnDeclaration::parseImpl(Pos & pos, ASTPtr & node, E column_declaration->children.push_back(std::move(settings)); } - if (stat_type_expression) + if (statistics_desc_expression) { - column_declaration->stat_type = stat_type_expression; - column_declaration->children.push_back(std::move(stat_type_expression)); + column_declaration->statistics_desc = statistics_desc_expression; + column_declaration->children.push_back(std::move(statistics_desc_expression)); } if (ttl_expression) diff --git a/src/Storages/ColumnsDescription.cpp b/src/Storages/ColumnsDescription.cpp index 69e39323219..d1babb817bf 100644 --- a/src/Storages/ColumnsDescription.cpp +++ b/src/Storages/ColumnsDescription.cpp @@ -25,6 +25,7 @@ #include #include #include "Parsers/ASTSetQuery.h" +#include "Storages/StatisticsDescription.h" #include #include #include @@ -207,6 +208,9 @@ void ColumnDescription::readText(ReadBuffer & buf) if (col_ast->settings) settings = col_ast->settings->as().changes; + + if (col_ast->statistics_desc) + statistics = ColumnStatisticsDescription::fromColumnDeclaration(*col_ast); } else throw Exception(ErrorCodes::CANNOT_PARSE_TEXT, "Cannot parse column description"); diff --git a/src/Storages/StatisticsDescription.cpp b/src/Storages/StatisticsDescription.cpp index dff1b7d3602..fc06c66b50e 100644 --- a/src/Storages/StatisticsDescription.cpp +++ b/src/Storages/StatisticsDescription.cpp @@ -171,7 +171,7 @@ std::vector ColumnStatisticsDescription::fromAST(co ColumnStatisticsDescription ColumnStatisticsDescription::fromColumnDeclaration(const ASTColumnDeclaration & column) { - const auto & stat_type_list_ast = column.stat_type->as().arguments; + const auto & stat_type_list_ast = column.statistics_desc->as().arguments; if (stat_type_list_ast->children.empty()) throw Exception(ErrorCodes::INCORRECT_QUERY, "We expect at least one statistics type for column {}", queryToString(column)); ColumnStatisticsDescription stats; diff --git a/tests/integration/test_manipulate_statistics/test.py b/tests/integration/test_manipulate_statistics/test.py index 2b26af940d1..bffee89ffc6 100644 --- a/tests/integration/test_manipulate_statistics/test.py +++ b/tests/integration/test_manipulate_statistics/test.py @@ -6,9 +6,12 @@ from helpers.cluster import ClickHouseCluster cluster = ClickHouseCluster(__file__) node1 = cluster.add_instance( - "node1", user_configs=["config/config.xml"], with_zookeeper=False + "node1", user_configs=["config/config.xml"], with_zookeeper=True ) +node2 = cluster.add_instance( + "node2", user_configs=["config/config.xml"], with_zookeeper=True +) @pytest.fixture(scope="module") def started_cluster(): @@ -122,3 +125,25 @@ def test_single_node_normal(started_cluster): """ ) run_test_single_node(started_cluster) + +def test_replicated_table_ddl(started_cluster): + node1.query("DROP TABLE IF EXISTS test_stat") + node2.query("DROP TABLE IF EXISTS test_stat") + + node1.query( + """ + CREATE TABLE test_stat(a Int64 STATISTICS(tdigest, uniq), b Int64 STATISTICS(tdigest, uniq), c Int64 STATISTICS(tdigest)) + ENGINE = ReplicatedMergeTree('/clickhouse/test/statistics', '1') ORDER BY a; + """ + ) + node2.query( + """ + CREATE TABLE test_stat(a Int64 STATISTICS(tdigest, uniq), b Int64 STATISTICS(tdigest, uniq), c Int64 STATISTICS(tdigest)) + ENGINE = ReplicatedMergeTree('/clickhouse/test/statistics', '2') ORDER BY a; + """ + ) + + node1.query("ALTER TABLE test_stat MODIFY STATISTICS c TYPE tdigest, uniq", settings={"alter_sync":"2"}); + node1.query("ALTER TABLE test_stat DROP STATISTICS b", settings={"alter_sync":"2"}); + + assert node2.query("SHOW CREATE TABLE test_stat") == "CREATE TABLE default.test_stat\\n(\\n `a` Int64 STATISTICS(tdigest, uniq),\\n `b` Int64,\\n `c` Int64 STATISTICS(tdigest, uniq)\\n)\\nENGINE = ReplicatedMergeTree(\\'/clickhouse/test/statistics\\', \\'2\\')\\nORDER BY a\\nSETTINGS index_granularity = 8192\n" From 0bd5164f83b1751999c667de4ec6327f9a59c5d6 Mon Sep 17 00:00:00 2001 From: Han Fei Date: Thu, 6 Jun 2024 21:30:16 +0200 Subject: [PATCH 07/40] fix style --- src/Storages/ColumnsDescription.cpp | 3 +-- .../integration/test_manipulate_statistics/test.py | 14 +++++++++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/Storages/ColumnsDescription.cpp b/src/Storages/ColumnsDescription.cpp index d1babb817bf..556f8a6e42d 100644 --- a/src/Storages/ColumnsDescription.cpp +++ b/src/Storages/ColumnsDescription.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -24,8 +25,6 @@ #include #include #include -#include "Parsers/ASTSetQuery.h" -#include "Storages/StatisticsDescription.h" #include #include #include diff --git a/tests/integration/test_manipulate_statistics/test.py b/tests/integration/test_manipulate_statistics/test.py index bffee89ffc6..53ab9682ad2 100644 --- a/tests/integration/test_manipulate_statistics/test.py +++ b/tests/integration/test_manipulate_statistics/test.py @@ -13,6 +13,7 @@ node2 = cluster.add_instance( "node2", user_configs=["config/config.xml"], with_zookeeper=True ) + @pytest.fixture(scope="module") def started_cluster(): try: @@ -126,6 +127,7 @@ def test_single_node_normal(started_cluster): ) run_test_single_node(started_cluster) + def test_replicated_table_ddl(started_cluster): node1.query("DROP TABLE IF EXISTS test_stat") node2.query("DROP TABLE IF EXISTS test_stat") @@ -143,7 +145,13 @@ def test_replicated_table_ddl(started_cluster): """ ) - node1.query("ALTER TABLE test_stat MODIFY STATISTICS c TYPE tdigest, uniq", settings={"alter_sync":"2"}); - node1.query("ALTER TABLE test_stat DROP STATISTICS b", settings={"alter_sync":"2"}); + node1.query( + "ALTER TABLE test_stat MODIFY STATISTICS c TYPE tdigest, uniq", + settings={"alter_sync": "2"}, + ) + node1.query("ALTER TABLE test_stat DROP STATISTICS b", settings={"alter_sync": "2"}) - assert node2.query("SHOW CREATE TABLE test_stat") == "CREATE TABLE default.test_stat\\n(\\n `a` Int64 STATISTICS(tdigest, uniq),\\n `b` Int64,\\n `c` Int64 STATISTICS(tdigest, uniq)\\n)\\nENGINE = ReplicatedMergeTree(\\'/clickhouse/test/statistics\\', \\'2\\')\\nORDER BY a\\nSETTINGS index_granularity = 8192\n" + assert ( + node2.query("SHOW CREATE TABLE test_stat") + == "CREATE TABLE default.test_stat\\n(\\n `a` Int64 STATISTICS(tdigest, uniq),\\n `b` Int64,\\n `c` Int64 STATISTICS(tdigest, uniq)\\n)\\nENGINE = ReplicatedMergeTree(\\'/clickhouse/test/statistics\\', \\'2\\')\\nORDER BY a\\nSETTINGS index_granularity = 8192\n" + ) From 5f7683f3914b0270ada6334c87410780074b71fd Mon Sep 17 00:00:00 2001 From: avogar Date: Mon, 10 Jun 2024 18:02:44 +0000 Subject: [PATCH 08/40] Avoid writing to finalized buffer in File-like storages --- src/Storages/ObjectStorage/StorageObjectStorageSink.cpp | 5 +++-- src/Storages/StorageFile.cpp | 3 ++- src/Storages/StorageURL.cpp | 3 ++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Storages/ObjectStorage/StorageObjectStorageSink.cpp b/src/Storages/ObjectStorage/StorageObjectStorageSink.cpp index 0a3cf19a590..154c2b07251 100644 --- a/src/Storages/ObjectStorage/StorageObjectStorageSink.cpp +++ b/src/Storages/ObjectStorage/StorageObjectStorageSink.cpp @@ -83,7 +83,6 @@ void StorageObjectStorageSink::finalize() { writer->finalize(); writer->flush(); - write_buf->finalize(); } catch (...) { @@ -91,12 +90,14 @@ void StorageObjectStorageSink::finalize() release(); throw; } + + write_buf->finalize(); } void StorageObjectStorageSink::release() { writer.reset(); - write_buf.reset(); + write_buf->finalize(); } PartitionedStorageObjectStorageSink::PartitionedStorageObjectStorageSink( diff --git a/src/Storages/StorageFile.cpp b/src/Storages/StorageFile.cpp index aaf84f6f82c..83bfcdaf415 100644 --- a/src/Storages/StorageFile.cpp +++ b/src/Storages/StorageFile.cpp @@ -1823,7 +1823,6 @@ private: { writer->finalize(); writer->flush(); - write_buf->finalize(); } catch (...) { @@ -1831,6 +1830,8 @@ private: release(); throw; } + + write_buf->finalize(); } void release() diff --git a/src/Storages/StorageURL.cpp b/src/Storages/StorageURL.cpp index 9302e7ef3e5..cd39ca42574 100644 --- a/src/Storages/StorageURL.cpp +++ b/src/Storages/StorageURL.cpp @@ -609,7 +609,6 @@ void StorageURLSink::finalize() { writer->finalize(); writer->flush(); - write_buf->finalize(); } catch (...) { @@ -617,6 +616,8 @@ void StorageURLSink::finalize() release(); throw; } + + write_buf->finalize(); } void StorageURLSink::release() From d9ff40851ee0e25413c959ad5cf31de244e2550d Mon Sep 17 00:00:00 2001 From: avogar Date: Tue, 11 Jun 2024 10:55:27 +0000 Subject: [PATCH 09/40] Don't use finalize in release() method --- src/IO/WriteBufferFromFile.cpp | 10 +++++++++- src/IO/WriteBufferFromFileDescriptor.cpp | 9 ++++++++- .../ObjectStorage/HDFS/WriteBufferFromHDFS.cpp | 10 ++-------- src/Storages/ObjectStorage/HDFS/WriteBufferFromHDFS.h | 2 -- .../ObjectStorage/StorageObjectStorageSink.cpp | 2 +- src/Storages/StorageFile.cpp | 2 +- src/Storages/StorageURL.cpp | 2 +- 7 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/IO/WriteBufferFromFile.cpp b/src/IO/WriteBufferFromFile.cpp index 0ca6c26f08c..d641e553671 100644 --- a/src/IO/WriteBufferFromFile.cpp +++ b/src/IO/WriteBufferFromFile.cpp @@ -77,7 +77,15 @@ WriteBufferFromFile::~WriteBufferFromFile() if (fd < 0) return; - finalize(); + try + { + finalize(); + } + catch (...) + { + tryLogCurrentException(__PRETTY_FUNCTION__); + } + int err = ::close(fd); /// Everything except for EBADF should be ignored in dtor, since all of /// others (EINTR/EIO/ENOSPC/EDQUOT) could be possible during writing to diff --git a/src/IO/WriteBufferFromFileDescriptor.cpp b/src/IO/WriteBufferFromFileDescriptor.cpp index 813ef0deab9..a758f99458d 100644 --- a/src/IO/WriteBufferFromFileDescriptor.cpp +++ b/src/IO/WriteBufferFromFileDescriptor.cpp @@ -105,7 +105,14 @@ WriteBufferFromFileDescriptor::WriteBufferFromFileDescriptor( WriteBufferFromFileDescriptor::~WriteBufferFromFileDescriptor() { - finalize(); + try + { + finalize(); + } + catch (...) + { + tryLogCurrentException(__PRETTY_FUNCTION__); + } } void WriteBufferFromFileDescriptor::finalizeImpl() diff --git a/src/Storages/ObjectStorage/HDFS/WriteBufferFromHDFS.cpp b/src/Storages/ObjectStorage/HDFS/WriteBufferFromHDFS.cpp index 2c14b38ce01..8277a769a11 100644 --- a/src/Storages/ObjectStorage/HDFS/WriteBufferFromHDFS.cpp +++ b/src/Storages/ObjectStorage/HDFS/WriteBufferFromHDFS.cpp @@ -132,11 +132,11 @@ void WriteBufferFromHDFS::sync() } -void WriteBufferFromHDFS::finalizeImpl() +WriteBufferFromHDFS::~WriteBufferFromHDFS() { try { - next(); + finalize(); } catch (...) { @@ -144,11 +144,5 @@ void WriteBufferFromHDFS::finalizeImpl() } } - -WriteBufferFromHDFS::~WriteBufferFromHDFS() -{ - finalize(); -} - } #endif diff --git a/src/Storages/ObjectStorage/HDFS/WriteBufferFromHDFS.h b/src/Storages/ObjectStorage/HDFS/WriteBufferFromHDFS.h index 71e6e55addc..e3f0ae96a8f 100644 --- a/src/Storages/ObjectStorage/HDFS/WriteBufferFromHDFS.h +++ b/src/Storages/ObjectStorage/HDFS/WriteBufferFromHDFS.h @@ -38,8 +38,6 @@ public: std::string getFileName() const override { return filename; } private: - void finalizeImpl() override; - struct WriteBufferFromHDFSImpl; std::unique_ptr impl; const std::string filename; diff --git a/src/Storages/ObjectStorage/StorageObjectStorageSink.cpp b/src/Storages/ObjectStorage/StorageObjectStorageSink.cpp index 154c2b07251..d13aec4a4f6 100644 --- a/src/Storages/ObjectStorage/StorageObjectStorageSink.cpp +++ b/src/Storages/ObjectStorage/StorageObjectStorageSink.cpp @@ -97,7 +97,7 @@ void StorageObjectStorageSink::finalize() void StorageObjectStorageSink::release() { writer.reset(); - write_buf->finalize(); + write_buf.reset(); } PartitionedStorageObjectStorageSink::PartitionedStorageObjectStorageSink( diff --git a/src/Storages/StorageFile.cpp b/src/Storages/StorageFile.cpp index 83bfcdaf415..16c248f1b7b 100644 --- a/src/Storages/StorageFile.cpp +++ b/src/Storages/StorageFile.cpp @@ -1837,7 +1837,7 @@ private: void release() { writer.reset(); - write_buf->finalize(); + write_buf.reset(); } StorageMetadataPtr metadata_snapshot; diff --git a/src/Storages/StorageURL.cpp b/src/Storages/StorageURL.cpp index cd39ca42574..f8424bc3d1b 100644 --- a/src/Storages/StorageURL.cpp +++ b/src/Storages/StorageURL.cpp @@ -623,7 +623,7 @@ void StorageURLSink::finalize() void StorageURLSink::release() { writer.reset(); - write_buf->finalize(); + write_buf.reset(); } class PartitionedStorageURLSink : public PartitionedSink From e0668ab8fa1fd90fc88c6d1088230279b6fb28b7 Mon Sep 17 00:00:00 2001 From: Han Fei Date: Tue, 11 Jun 2024 17:03:54 +0200 Subject: [PATCH 10/40] write more tests --- src/Interpreters/InterpreterCreateQuery.cpp | 3 +-- src/Storages/ColumnsDescription.cpp | 6 +++++- src/Storages/MergeTree/MergeTreeDataWriter.cpp | 6 ++++++ src/Storages/StatisticsDescription.cpp | 4 ++-- src/Storages/StatisticsDescription.h | 2 +- .../test_manipulate_statistics/test.py | 18 ++++++++++++++++++ 6 files changed, 33 insertions(+), 6 deletions(-) diff --git a/src/Interpreters/InterpreterCreateQuery.cpp b/src/Interpreters/InterpreterCreateQuery.cpp index a51d3e6dade..b2d65662eaa 100644 --- a/src/Interpreters/InterpreterCreateQuery.cpp +++ b/src/Interpreters/InterpreterCreateQuery.cpp @@ -680,8 +680,7 @@ ColumnsDescription InterpreterCreateQuery::getColumnsDescription( { if (!skip_checks && !context_->getSettingsRef().allow_experimental_statistics) throw Exception(ErrorCodes::INCORRECT_QUERY, "Create table with statistics is now disabled. Turn on allow_experimental_statistics"); - column.statistics = ColumnStatisticsDescription::fromColumnDeclaration(col_decl); - column.statistics.data_type = column.type; + column.statistics = ColumnStatisticsDescription::fromColumnDeclaration(col_decl, column.type); } if (col_decl.ttl) diff --git a/src/Storages/ColumnsDescription.cpp b/src/Storages/ColumnsDescription.cpp index 556f8a6e42d..c07583cd39d 100644 --- a/src/Storages/ColumnsDescription.cpp +++ b/src/Storages/ColumnsDescription.cpp @@ -209,7 +209,11 @@ void ColumnDescription::readText(ReadBuffer & buf) settings = col_ast->settings->as().changes; if (col_ast->statistics_desc) - statistics = ColumnStatisticsDescription::fromColumnDeclaration(*col_ast); + { + statistics = ColumnStatisticsDescription::fromColumnDeclaration(*col_ast, type); + /// every column has name `x` here, so we have to set the name manually. + statistics.column_name = name; + } } else throw Exception(ErrorCodes::CANNOT_PARSE_TEXT, "Cannot parse column description"); diff --git a/src/Storages/MergeTree/MergeTreeDataWriter.cpp b/src/Storages/MergeTree/MergeTreeDataWriter.cpp index 8e304936747..0dbddf05697 100644 --- a/src/Storages/MergeTree/MergeTreeDataWriter.cpp +++ b/src/Storages/MergeTree/MergeTreeDataWriter.cpp @@ -471,7 +471,13 @@ MergeTreeDataWriter::TemporaryPart MergeTreeDataWriter::writeTempPartImpl( ColumnsStatistics statistics; if (context->getSettingsRef().materialize_statistics_on_insert) + { + for (auto col : metadata_snapshot->getColumns()) + LOG_INFO(log, "column col {} stats {}", col.name, col.statistics.column_name); statistics = MergeTreeStatisticsFactory::instance().getMany(metadata_snapshot->getColumns()); + for (auto stats : statistics) + LOG_INFO(log, "writing stats {}", stats->columnName()); + } /// If we need to calculate some columns to sort. if (metadata_snapshot->hasSortingKey() || metadata_snapshot->hasSecondaryIndices()) diff --git a/src/Storages/StatisticsDescription.cpp b/src/Storages/StatisticsDescription.cpp index fc06c66b50e..f10fb78f933 100644 --- a/src/Storages/StatisticsDescription.cpp +++ b/src/Storages/StatisticsDescription.cpp @@ -169,7 +169,7 @@ std::vector ColumnStatisticsDescription::fromAST(co return result; } -ColumnStatisticsDescription ColumnStatisticsDescription::fromColumnDeclaration(const ASTColumnDeclaration & column) +ColumnStatisticsDescription ColumnStatisticsDescription::fromColumnDeclaration(const ASTColumnDeclaration & column, DataTypePtr data_type) { const auto & stat_type_list_ast = column.statistics_desc->as().arguments; if (stat_type_list_ast->children.empty()) @@ -185,7 +185,7 @@ ColumnStatisticsDescription ColumnStatisticsDescription::fromColumnDeclaration(c throw Exception(ErrorCodes::INCORRECT_QUERY, "Column {} already contains statistics type {}", stats.column_name, stat_type); stats.types_to_desc.emplace(stat.type, std::move(stat)); } - + stats.data_type = data_type; return stats; } diff --git a/src/Storages/StatisticsDescription.h b/src/Storages/StatisticsDescription.h index 59ad8944850..4862fb79d45 100644 --- a/src/Storages/StatisticsDescription.h +++ b/src/Storages/StatisticsDescription.h @@ -55,7 +55,7 @@ struct ColumnStatisticsDescription ASTPtr getAST() const; static std::vector fromAST(const ASTPtr & definition_ast, const ColumnsDescription & columns); - static ColumnStatisticsDescription fromColumnDeclaration(const ASTColumnDeclaration & column); + static ColumnStatisticsDescription fromColumnDeclaration(const ASTColumnDeclaration & column, DataTypePtr data_type); using StatisticsTypeDescMap = std::map; StatisticsTypeDescMap types_to_desc; diff --git a/tests/integration/test_manipulate_statistics/test.py b/tests/integration/test_manipulate_statistics/test.py index 53ab9682ad2..0ce90731e8d 100644 --- a/tests/integration/test_manipulate_statistics/test.py +++ b/tests/integration/test_manipulate_statistics/test.py @@ -155,3 +155,21 @@ def test_replicated_table_ddl(started_cluster): node2.query("SHOW CREATE TABLE test_stat") == "CREATE TABLE default.test_stat\\n(\\n `a` Int64 STATISTICS(tdigest, uniq),\\n `b` Int64,\\n `c` Int64 STATISTICS(tdigest, uniq)\\n)\\nENGINE = ReplicatedMergeTree(\\'/clickhouse/test/statistics\\', \\'2\\')\\nORDER BY a\\nSETTINGS index_granularity = 8192\n" ) + + node2.query("insert into test_stat values(1,2,3), (2,3,4)"); + # check_stat_file_on_disk(node2, "test_stat", "all_0_0_0", "a", True) + check_stat_file_on_disk(node2, "test_stat", "all_0_0_0", "a", True) + check_stat_file_on_disk(node2, "test_stat", "all_0_0_0", "c", True) + node1.query("ALTER TABLE test_stat RENAME COLUMN c TO d", settings={"alter_sync": "2"}) + assert (node2.query("select sum(a), sum(d) from test_stat") == "3\t7\n") + check_stat_file_on_disk(node2, "test_stat", "all_0_0_0_1", "a", True) + check_stat_file_on_disk(node2, "test_stat", "all_0_0_0_1", "c", False) + check_stat_file_on_disk(node2, "test_stat", "all_0_0_0_1", "d", True) + node1.query("ALTER TABLE test_stat CLEAR STATISTICS d", settings={"alter_sync": "2"}) + node1.query("ALTER TABLE test_stat ADD STATISTICS b type tdigest", settings={"alter_sync": "2"}) + check_stat_file_on_disk(node2, "test_stat", "all_0_0_0_2", "a", True) + check_stat_file_on_disk(node2, "test_stat", "all_0_0_0_2", "b", False) + check_stat_file_on_disk(node2, "test_stat", "all_0_0_0_2", "d", False) + node1.query("ALTER TABLE test_stat MATERIALIZE STATISTICS b", settings={"alter_sync": "2"}) + check_stat_file_on_disk(node2, "test_stat", "all_0_0_0_3", "a", True) + check_stat_file_on_disk(node2, "test_stat", "all_0_0_0_3", "b", True) From af4541755a43fcacf1a1a58edcf6b07bd5fcedec Mon Sep 17 00:00:00 2001 From: Han Fei Date: Wed, 12 Jun 2024 14:27:13 +0200 Subject: [PATCH 11/40] fix black --- .../test_manipulate_statistics/test.py | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/tests/integration/test_manipulate_statistics/test.py b/tests/integration/test_manipulate_statistics/test.py index 0ce90731e8d..9485b611c01 100644 --- a/tests/integration/test_manipulate_statistics/test.py +++ b/tests/integration/test_manipulate_statistics/test.py @@ -156,20 +156,29 @@ def test_replicated_table_ddl(started_cluster): == "CREATE TABLE default.test_stat\\n(\\n `a` Int64 STATISTICS(tdigest, uniq),\\n `b` Int64,\\n `c` Int64 STATISTICS(tdigest, uniq)\\n)\\nENGINE = ReplicatedMergeTree(\\'/clickhouse/test/statistics\\', \\'2\\')\\nORDER BY a\\nSETTINGS index_granularity = 8192\n" ) - node2.query("insert into test_stat values(1,2,3), (2,3,4)"); + node2.query("insert into test_stat values(1,2,3), (2,3,4)") # check_stat_file_on_disk(node2, "test_stat", "all_0_0_0", "a", True) check_stat_file_on_disk(node2, "test_stat", "all_0_0_0", "a", True) check_stat_file_on_disk(node2, "test_stat", "all_0_0_0", "c", True) - node1.query("ALTER TABLE test_stat RENAME COLUMN c TO d", settings={"alter_sync": "2"}) - assert (node2.query("select sum(a), sum(d) from test_stat") == "3\t7\n") + node1.query( + "ALTER TABLE test_stat RENAME COLUMN c TO d", settings={"alter_sync": "2"} + ) + assert node2.query("select sum(a), sum(d) from test_stat") == "3\t7\n" check_stat_file_on_disk(node2, "test_stat", "all_0_0_0_1", "a", True) check_stat_file_on_disk(node2, "test_stat", "all_0_0_0_1", "c", False) check_stat_file_on_disk(node2, "test_stat", "all_0_0_0_1", "d", True) - node1.query("ALTER TABLE test_stat CLEAR STATISTICS d", settings={"alter_sync": "2"}) - node1.query("ALTER TABLE test_stat ADD STATISTICS b type tdigest", settings={"alter_sync": "2"}) + node1.query( + "ALTER TABLE test_stat CLEAR STATISTICS d", settings={"alter_sync": "2"} + ) + node1.query( + "ALTER TABLE test_stat ADD STATISTICS b type tdigest", + settings={"alter_sync": "2"}, + ) check_stat_file_on_disk(node2, "test_stat", "all_0_0_0_2", "a", True) check_stat_file_on_disk(node2, "test_stat", "all_0_0_0_2", "b", False) check_stat_file_on_disk(node2, "test_stat", "all_0_0_0_2", "d", False) - node1.query("ALTER TABLE test_stat MATERIALIZE STATISTICS b", settings={"alter_sync": "2"}) + node1.query( + "ALTER TABLE test_stat MATERIALIZE STATISTICS b", settings={"alter_sync": "2"} + ) check_stat_file_on_disk(node2, "test_stat", "all_0_0_0_3", "a", True) check_stat_file_on_disk(node2, "test_stat", "all_0_0_0_3", "b", True) From d798fffb1d772266095f599173212874a3897ab1 Mon Sep 17 00:00:00 2001 From: Han Fei Date: Wed, 12 Jun 2024 16:46:14 +0200 Subject: [PATCH 12/40] fix --- src/Storages/MergeTree/MergeTreeDataWriter.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/Storages/MergeTree/MergeTreeDataWriter.cpp b/src/Storages/MergeTree/MergeTreeDataWriter.cpp index 0dbddf05697..8e304936747 100644 --- a/src/Storages/MergeTree/MergeTreeDataWriter.cpp +++ b/src/Storages/MergeTree/MergeTreeDataWriter.cpp @@ -471,13 +471,7 @@ MergeTreeDataWriter::TemporaryPart MergeTreeDataWriter::writeTempPartImpl( ColumnsStatistics statistics; if (context->getSettingsRef().materialize_statistics_on_insert) - { - for (auto col : metadata_snapshot->getColumns()) - LOG_INFO(log, "column col {} stats {}", col.name, col.statistics.column_name); statistics = MergeTreeStatisticsFactory::instance().getMany(metadata_snapshot->getColumns()); - for (auto stats : statistics) - LOG_INFO(log, "writing stats {}", stats->columnName()); - } /// If we need to calculate some columns to sort. if (metadata_snapshot->hasSortingKey() || metadata_snapshot->hasSecondaryIndices()) From 47686e0c4ae633f822ffc0926e5380130ecc1743 Mon Sep 17 00:00:00 2001 From: Grigorii Sokolik Date: Thu, 9 May 2024 14:27:43 +0300 Subject: [PATCH 13/40] [feature] A setting `http_response_headers` Implementing https://github.com/ClickHouse/ClickHouse/issues/59620 Deprecating `content_type` setting (still supported). --- src/Server/HTTPHandler.cpp | 530 +++++++++++++++++++------------------ src/Server/HTTPHandler.h | 60 +++-- 2 files changed, 308 insertions(+), 282 deletions(-) diff --git a/src/Server/HTTPHandler.cpp b/src/Server/HTTPHandler.cpp index 02d0959ff50..3c0bbf03986 100644 --- a/src/Server/HTTPHandler.cpp +++ b/src/Server/HTTPHandler.cpp @@ -1,8 +1,8 @@ #include +#include #include #include -#include #include #include #include @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -17,46 +18,47 @@ #include #include #include -#include -#include -#include #include +#include +#include +#include +#include +#include #include #include #include -#include #include #include #include #include #include -#include -#include -#include -#include +#include #include #include -#include #include "config.h" #include #include -#include -#include #include +#include +#include +#include +#include #include #include -#include #include #include #include +#include #include +#include +#include #if USE_SSL -#include +# include #endif @@ -65,68 +67,68 @@ namespace DB namespace ErrorCodes { - extern const int BAD_ARGUMENTS; - extern const int LOGICAL_ERROR; - extern const int CANNOT_COMPILE_REGEXP; - extern const int CANNOT_OPEN_FILE; - extern const int CANNOT_PARSE_TEXT; - extern const int CANNOT_PARSE_ESCAPE_SEQUENCE; - extern const int CANNOT_PARSE_QUOTED_STRING; - extern const int CANNOT_PARSE_DATE; - extern const int CANNOT_PARSE_DATETIME; - extern const int CANNOT_PARSE_NUMBER; - extern const int CANNOT_PARSE_DOMAIN_VALUE_FROM_STRING; - extern const int CANNOT_PARSE_IPV4; - extern const int CANNOT_PARSE_IPV6; - extern const int CANNOT_PARSE_UUID; - extern const int CANNOT_PARSE_INPUT_ASSERTION_FAILED; - extern const int CANNOT_SCHEDULE_TASK; - extern const int DUPLICATE_COLUMN; - extern const int ILLEGAL_COLUMN; - extern const int THERE_IS_NO_COLUMN; - extern const int UNKNOWN_ELEMENT_IN_AST; - extern const int UNKNOWN_TYPE_OF_AST_NODE; - extern const int TOO_DEEP_AST; - extern const int TOO_BIG_AST; - extern const int UNEXPECTED_AST_STRUCTURE; - extern const int VALUE_IS_OUT_OF_RANGE_OF_DATA_TYPE; +extern const int BAD_ARGUMENTS; +extern const int LOGICAL_ERROR; +extern const int CANNOT_SCHEDULE_TASK; +extern const int CANNOT_PARSE_TEXT; +extern const int CANNOT_PARSE_ESCAPE_SEQUENCE; +extern const int CANNOT_PARSE_QUOTED_STRING; +extern const int CANNOT_PARSE_DATE; +extern const int CANNOT_PARSE_DATETIME; +extern const int CANNOT_PARSE_NUMBER; +extern const int CANNOT_PARSE_DOMAIN_VALUE_FROM_STRING; +extern const int CANNOT_PARSE_IPV4; +extern const int CANNOT_PARSE_IPV6; +extern const int CANNOT_PARSE_UUID; +extern const int CANNOT_PARSE_INPUT_ASSERTION_FAILED; +extern const int CANNOT_OPEN_FILE; +extern const int CANNOT_COMPILE_REGEXP; +extern const int DUPLICATE_COLUMN; +extern const int ILLEGAL_COLUMN; +extern const int THERE_IS_NO_COLUMN; +extern const int UNKNOWN_ELEMENT_IN_AST; +extern const int UNKNOWN_TYPE_OF_AST_NODE; +extern const int TOO_DEEP_AST; +extern const int TOO_BIG_AST; +extern const int UNEXPECTED_AST_STRUCTURE; +extern const int VALUE_IS_OUT_OF_RANGE_OF_DATA_TYPE; - extern const int SYNTAX_ERROR; +extern const int SYNTAX_ERROR; - extern const int INCORRECT_DATA; - extern const int TYPE_MISMATCH; +extern const int INCORRECT_DATA; +extern const int TYPE_MISMATCH; - extern const int UNKNOWN_TABLE; - extern const int UNKNOWN_FUNCTION; - extern const int UNKNOWN_IDENTIFIER; - extern const int UNKNOWN_TYPE; - extern const int UNKNOWN_STORAGE; - extern const int UNKNOWN_DATABASE; - extern const int UNKNOWN_SETTING; - extern const int UNKNOWN_DIRECTION_OF_SORTING; - extern const int UNKNOWN_AGGREGATE_FUNCTION; - extern const int UNKNOWN_FORMAT; - extern const int UNKNOWN_DATABASE_ENGINE; - extern const int UNKNOWN_TYPE_OF_QUERY; - extern const int UNKNOWN_ROLE; - extern const int NO_ELEMENTS_IN_CONFIG; +extern const int UNKNOWN_TABLE; +extern const int UNKNOWN_FUNCTION; +extern const int UNKNOWN_IDENTIFIER; +extern const int UNKNOWN_TYPE; +extern const int UNKNOWN_STORAGE; +extern const int UNKNOWN_DATABASE; +extern const int UNKNOWN_SETTING; +extern const int UNKNOWN_DIRECTION_OF_SORTING; +extern const int UNKNOWN_AGGREGATE_FUNCTION; +extern const int UNKNOWN_FORMAT; +extern const int UNKNOWN_DATABASE_ENGINE; +extern const int UNKNOWN_TYPE_OF_QUERY; +extern const int UNKNOWN_ROLE; +extern const int NO_ELEMENTS_IN_CONFIG; - extern const int QUERY_IS_TOO_LARGE; +extern const int QUERY_IS_TOO_LARGE; - extern const int NOT_IMPLEMENTED; - extern const int SOCKET_TIMEOUT; +extern const int NOT_IMPLEMENTED; +extern const int SOCKET_TIMEOUT; - extern const int UNKNOWN_USER; - extern const int WRONG_PASSWORD; - extern const int REQUIRED_PASSWORD; - extern const int AUTHENTICATION_FAILED; - extern const int SET_NON_GRANTED_ROLE; +extern const int UNKNOWN_USER; +extern const int WRONG_PASSWORD; +extern const int REQUIRED_PASSWORD; +extern const int AUTHENTICATION_FAILED; +extern const int SET_NON_GRANTED_ROLE; - extern const int INVALID_SESSION_TIMEOUT; - extern const int HTTP_LENGTH_REQUIRED; - extern const int SUPPORT_IS_DISABLED; +extern const int INVALID_SESSION_TIMEOUT; +extern const int HTTP_LENGTH_REQUIRED; +extern const int SUPPORT_IS_DISABLED; - extern const int TIMEOUT_EXCEEDED; +extern const int TIMEOUT_EXCEEDED; } namespace @@ -145,9 +147,9 @@ bool tryAddHTTPOptionHeadersFromConfig(HTTPServerResponse & response, const Poco if (config.getString("http_options_response." + config_key + ".name", "").empty()) LOG_WARNING(getLogger("processOptionsRequest"), "Empty header was found in config. It will not be processed."); else - response.add(config.getString("http_options_response." + config_key + ".name", ""), - config.getString("http_options_response." + config_key + ".value", "")); - + response.add( + config.getString("http_options_response." + config_key + ".name", ""), + config.getString("http_options_response." + config_key + ".value", "")); } } return true; @@ -196,54 +198,37 @@ static Poco::Net::HTTPResponse::HTTPStatus exceptionCodeToHTTPStatus(int excepti { return HTTPResponse::HTTP_UNAUTHORIZED; } - else if (exception_code == ErrorCodes::UNKNOWN_USER || - exception_code == ErrorCodes::WRONG_PASSWORD || - exception_code == ErrorCodes::AUTHENTICATION_FAILED || - exception_code == ErrorCodes::SET_NON_GRANTED_ROLE) + else if ( + exception_code == ErrorCodes::UNKNOWN_USER || exception_code == ErrorCodes::WRONG_PASSWORD + || exception_code == ErrorCodes::AUTHENTICATION_FAILED || exception_code == ErrorCodes::SET_NON_GRANTED_ROLE) { return HTTPResponse::HTTP_FORBIDDEN; } - else if (exception_code == ErrorCodes::BAD_ARGUMENTS || - exception_code == ErrorCodes::CANNOT_COMPILE_REGEXP || - exception_code == ErrorCodes::CANNOT_PARSE_TEXT || - exception_code == ErrorCodes::CANNOT_PARSE_ESCAPE_SEQUENCE || - exception_code == ErrorCodes::CANNOT_PARSE_QUOTED_STRING || - exception_code == ErrorCodes::CANNOT_PARSE_DATE || - exception_code == ErrorCodes::CANNOT_PARSE_DATETIME || - exception_code == ErrorCodes::CANNOT_PARSE_NUMBER || - exception_code == ErrorCodes::CANNOT_PARSE_DOMAIN_VALUE_FROM_STRING || - exception_code == ErrorCodes::CANNOT_PARSE_IPV4 || - exception_code == ErrorCodes::CANNOT_PARSE_IPV6 || - exception_code == ErrorCodes::CANNOT_PARSE_INPUT_ASSERTION_FAILED || - exception_code == ErrorCodes::CANNOT_PARSE_UUID || - exception_code == ErrorCodes::DUPLICATE_COLUMN || - exception_code == ErrorCodes::ILLEGAL_COLUMN || - exception_code == ErrorCodes::UNKNOWN_ELEMENT_IN_AST || - exception_code == ErrorCodes::UNKNOWN_TYPE_OF_AST_NODE || - exception_code == ErrorCodes::THERE_IS_NO_COLUMN || - exception_code == ErrorCodes::TOO_DEEP_AST || - exception_code == ErrorCodes::TOO_BIG_AST || - exception_code == ErrorCodes::UNEXPECTED_AST_STRUCTURE || - exception_code == ErrorCodes::SYNTAX_ERROR || - exception_code == ErrorCodes::INCORRECT_DATA || - exception_code == ErrorCodes::TYPE_MISMATCH || - exception_code == ErrorCodes::VALUE_IS_OUT_OF_RANGE_OF_DATA_TYPE) + else if ( + exception_code == ErrorCodes::BAD_ARGUMENTS || exception_code == ErrorCodes::CANNOT_COMPILE_REGEXP + || exception_code == ErrorCodes::CANNOT_PARSE_TEXT || exception_code == ErrorCodes::CANNOT_PARSE_ESCAPE_SEQUENCE + || exception_code == ErrorCodes::CANNOT_PARSE_QUOTED_STRING || exception_code == ErrorCodes::CANNOT_PARSE_DATE + || exception_code == ErrorCodes::CANNOT_PARSE_DATETIME || exception_code == ErrorCodes::CANNOT_PARSE_NUMBER + || exception_code == ErrorCodes::CANNOT_PARSE_DOMAIN_VALUE_FROM_STRING || exception_code == ErrorCodes::CANNOT_PARSE_IPV4 + || exception_code == ErrorCodes::CANNOT_PARSE_IPV6 || exception_code == ErrorCodes::CANNOT_PARSE_INPUT_ASSERTION_FAILED + || exception_code == ErrorCodes::CANNOT_PARSE_UUID || exception_code == ErrorCodes::DUPLICATE_COLUMN + || exception_code == ErrorCodes::ILLEGAL_COLUMN || exception_code == ErrorCodes::UNKNOWN_ELEMENT_IN_AST + || exception_code == ErrorCodes::UNKNOWN_TYPE_OF_AST_NODE || exception_code == ErrorCodes::THERE_IS_NO_COLUMN + || exception_code == ErrorCodes::TOO_DEEP_AST || exception_code == ErrorCodes::TOO_BIG_AST + || exception_code == ErrorCodes::UNEXPECTED_AST_STRUCTURE || exception_code == ErrorCodes::SYNTAX_ERROR + || exception_code == ErrorCodes::INCORRECT_DATA || exception_code == ErrorCodes::TYPE_MISMATCH + || exception_code == ErrorCodes::VALUE_IS_OUT_OF_RANGE_OF_DATA_TYPE) { return HTTPResponse::HTTP_BAD_REQUEST; } - else if (exception_code == ErrorCodes::UNKNOWN_TABLE || - exception_code == ErrorCodes::UNKNOWN_FUNCTION || - exception_code == ErrorCodes::UNKNOWN_IDENTIFIER || - exception_code == ErrorCodes::UNKNOWN_TYPE || - exception_code == ErrorCodes::UNKNOWN_STORAGE || - exception_code == ErrorCodes::UNKNOWN_DATABASE || - exception_code == ErrorCodes::UNKNOWN_SETTING || - exception_code == ErrorCodes::UNKNOWN_DIRECTION_OF_SORTING || - exception_code == ErrorCodes::UNKNOWN_AGGREGATE_FUNCTION || - exception_code == ErrorCodes::UNKNOWN_FORMAT || - exception_code == ErrorCodes::UNKNOWN_DATABASE_ENGINE || - exception_code == ErrorCodes::UNKNOWN_TYPE_OF_QUERY || - exception_code == ErrorCodes::UNKNOWN_ROLE) + else if ( + exception_code == ErrorCodes::UNKNOWN_TABLE || exception_code == ErrorCodes::UNKNOWN_FUNCTION + || exception_code == ErrorCodes::UNKNOWN_IDENTIFIER || exception_code == ErrorCodes::UNKNOWN_TYPE + || exception_code == ErrorCodes::UNKNOWN_STORAGE || exception_code == ErrorCodes::UNKNOWN_DATABASE + || exception_code == ErrorCodes::UNKNOWN_SETTING || exception_code == ErrorCodes::UNKNOWN_DIRECTION_OF_SORTING + || exception_code == ErrorCodes::UNKNOWN_AGGREGATE_FUNCTION || exception_code == ErrorCodes::UNKNOWN_FORMAT + || exception_code == ErrorCodes::UNKNOWN_DATABASE_ENGINE || exception_code == ErrorCodes::UNKNOWN_TYPE_OF_QUERY + || exception_code == ErrorCodes::UNKNOWN_ROLE) { return HTTPResponse::HTTP_NOT_FOUND; } @@ -255,8 +240,7 @@ static Poco::Net::HTTPResponse::HTTPStatus exceptionCodeToHTTPStatus(int excepti { return HTTPResponse::HTTP_NOT_IMPLEMENTED; } - else if (exception_code == ErrorCodes::SOCKET_TIMEOUT || - exception_code == ErrorCodes::CANNOT_OPEN_FILE) + else if (exception_code == ErrorCodes::SOCKET_TIMEOUT || exception_code == ErrorCodes::CANNOT_OPEN_FILE) { return HTTPResponse::HTTP_SERVICE_UNAVAILABLE; } @@ -277,9 +261,7 @@ static Poco::Net::HTTPResponse::HTTPStatus exceptionCodeToHTTPStatus(int excepti } -static std::chrono::steady_clock::duration parseSessionTimeout( - const Poco::Util::AbstractConfiguration & config, - const HTMLForm & params) +static std::chrono::steady_clock::duration parseSessionTimeout(const Poco::Util::AbstractConfiguration & config, const HTMLForm & params) { unsigned session_timeout = config.getInt("default_session_timeout", 60); @@ -293,14 +275,19 @@ static std::chrono::steady_clock::duration parseSessionTimeout( throw Exception(ErrorCodes::INVALID_SESSION_TIMEOUT, "Invalid session timeout: '{}'", session_timeout_str); if (session_timeout > max_session_timeout) - throw Exception(ErrorCodes::INVALID_SESSION_TIMEOUT, "Session timeout '{}' is larger than max_session_timeout: {}. " + throw Exception( + ErrorCodes::INVALID_SESSION_TIMEOUT, + "Session timeout '{}' is larger than max_session_timeout: {}. " "Maximum session timeout could be modified in configuration file.", - session_timeout_str, max_session_timeout); + session_timeout_str, + max_session_timeout); } return std::chrono::seconds(session_timeout); } +std::optional> +parseHttpResponseHeaders(const Poco::Util::AbstractConfiguration & config, const std::string & config_prefix); void HTTPHandler::pushDelayedResults(Output & used_output) { @@ -338,11 +325,12 @@ void HTTPHandler::pushDelayedResults(Output & used_output) } -HTTPHandler::HTTPHandler(IServer & server_, const std::string & name, const std::optional & content_type_override_) +HTTPHandler::HTTPHandler( + IServer & server_, const std::string & name, const std::optional> & http_response_headers_override_) : server(server_) , log(getLogger(name)) , default_settings(server.context()->getSettingsRef()) - , content_type_override(content_type_override_) + , http_response_headers_override(http_response_headers_override_) { server_display_name = server.config().getString("display_name", getFQDNOrHostName()); } @@ -353,10 +341,7 @@ HTTPHandler::HTTPHandler(IServer & server_, const std::string & name, const std: HTTPHandler::~HTTPHandler() = default; -bool HTTPHandler::authenticateUser( - HTTPServerRequest & request, - HTMLForm & params, - HTTPServerResponse & response) +bool HTTPHandler::authenticateUser(HTTPServerRequest & request, HTMLForm & params, HTTPServerResponse & response) { using namespace Poco::Net; @@ -383,31 +368,36 @@ bool HTTPHandler::authenticateUser( { /// It is prohibited to mix different authorization schemes. if (has_http_credentials) - throw Exception(ErrorCodes::AUTHENTICATION_FAILED, - "Invalid authentication: it is not allowed " - "to use SSL certificate authentication and Authorization HTTP header simultaneously"); + throw Exception( + ErrorCodes::AUTHENTICATION_FAILED, + "Invalid authentication: it is not allowed " + "to use SSL certificate authentication and Authorization HTTP header simultaneously"); if (has_credentials_in_query_params) - throw Exception(ErrorCodes::AUTHENTICATION_FAILED, - "Invalid authentication: it is not allowed " - "to use SSL certificate authentication and authentication via parameters simultaneously simultaneously"); + throw Exception( + ErrorCodes::AUTHENTICATION_FAILED, + "Invalid authentication: it is not allowed " + "to use SSL certificate authentication and authentication via parameters simultaneously simultaneously"); if (has_ssl_certificate_auth) { #if USE_SSL if (!password.empty()) - throw Exception(ErrorCodes::AUTHENTICATION_FAILED, - "Invalid authentication: it is not allowed " - "to use SSL certificate authentication and authentication via password simultaneously"); + throw Exception( + ErrorCodes::AUTHENTICATION_FAILED, + "Invalid authentication: it is not allowed " + "to use SSL certificate authentication and authentication via password simultaneously"); if (request.havePeerCertificate()) certificate_common_name = request.peerCertificate().commonName(); if (certificate_common_name.empty()) - throw Exception(ErrorCodes::AUTHENTICATION_FAILED, - "Invalid authentication: SSL certificate authentication requires nonempty certificate's Common Name"); + throw Exception( + ErrorCodes::AUTHENTICATION_FAILED, + "Invalid authentication: SSL certificate authentication requires nonempty certificate's Common Name"); #else - throw Exception(ErrorCodes::SUPPORT_IS_DISABLED, - "SSL certificate authentication disabled because ClickHouse was built without SSL library"); + throw Exception( + ErrorCodes::SUPPORT_IS_DISABLED, + "SSL certificate authentication disabled because ClickHouse was built without SSL library"); #endif } } @@ -415,9 +405,10 @@ bool HTTPHandler::authenticateUser( { /// It is prohibited to mix different authorization schemes. if (has_credentials_in_query_params) - throw Exception(ErrorCodes::AUTHENTICATION_FAILED, - "Invalid authentication: it is not allowed " - "to use Authorization HTTP header and authentication via parameters simultaneously"); + throw Exception( + ErrorCodes::AUTHENTICATION_FAILED, + "Invalid authentication: it is not allowed " + "to use Authorization HTTP header and authentication via parameters simultaneously"); std::string scheme; std::string auth_info; @@ -438,7 +429,8 @@ bool HTTPHandler::authenticateUser( } else { - throw Exception(ErrorCodes::AUTHENTICATION_FAILED, "Invalid authentication: '{}' HTTP Authorization scheme is not supported", scheme); + throw Exception( + ErrorCodes::AUTHENTICATION_FAILED, "Invalid authentication: '{}' HTTP Authorization scheme is not supported", scheme); } } else @@ -464,7 +456,8 @@ bool HTTPHandler::authenticateUser( auto * gss_acceptor_context = dynamic_cast(request_credentials.get()); if (!gss_acceptor_context) - throw Exception(ErrorCodes::AUTHENTICATION_FAILED, "Invalid authentication: unexpected 'Negotiate' HTTP Authorization scheme expected"); + throw Exception( + ErrorCodes::AUTHENTICATION_FAILED, "Invalid authentication: unexpected 'Negotiate' HTTP Authorization scheme expected"); #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wunreachable-code" @@ -500,9 +493,10 @@ bool HTTPHandler::authenticateUser( if (params.has("quota_key")) { if (!quota_key.empty()) - throw Exception(ErrorCodes::BAD_ARGUMENTS, - "Invalid authentication: it is not allowed " - "to use quota key as HTTP header and as parameter simultaneously"); + throw Exception( + ErrorCodes::BAD_ARGUMENTS, + "Invalid authentication: it is not allowed " + "to use quota key as HTTP header and as parameter simultaneously"); quota_key = params.get("quota_key"); } @@ -627,26 +621,29 @@ void HTTPHandler::processQuery( size_t buffer_size_memory = (buffer_size_total > buffer_size_http) ? buffer_size_total : 0; bool enable_http_compression = params.getParsed("enable_http_compression", context->getSettingsRef().enable_http_compression); - Int64 http_zlib_compression_level = params.getParsed("http_zlib_compression_level", context->getSettingsRef().http_zlib_compression_level); + Int64 http_zlib_compression_level + = params.getParsed("http_zlib_compression_level", context->getSettingsRef().http_zlib_compression_level); - used_output.out_holder = - std::make_shared( - response, - request.getMethod() == HTTPRequest::HTTP_HEAD, - context->getServerSettings().keep_alive_timeout.totalSeconds(), - write_event); + used_output.out_holder = std::make_shared( + response, + request.getMethod() == HTTPRequest::HTTP_HEAD, + context->getServerSettings().keep_alive_timeout.totalSeconds(), + write_event); used_output.out = used_output.out_holder; used_output.out_maybe_compressed = used_output.out_holder; if (client_supports_http_compression && enable_http_compression) { used_output.out_holder->setCompressionMethodHeader(http_response_compression_method); - used_output.wrap_compressed_holder = - wrapWriteBufferWithCompressionMethod( - used_output.out_holder.get(), - http_response_compression_method, - static_cast(http_zlib_compression_level), - 0, DBMS_DEFAULT_BUFFER_SIZE, nullptr, 0, false); + used_output.wrap_compressed_holder = wrapWriteBufferWithCompressionMethod( + used_output.out_holder.get(), + http_response_compression_method, + static_cast(http_zlib_compression_level), + 0, + DBMS_DEFAULT_BUFFER_SIZE, + nullptr, + 0, + false); used_output.out = used_output.wrap_compressed_holder; } @@ -670,16 +667,13 @@ void HTTPHandler::processQuery( { auto tmp_data = std::make_shared(server.context()->getTempDataOnDisk()); - auto create_tmp_disk_buffer = [tmp_data] (const WriteBufferPtr &) -> WriteBufferPtr - { - return tmp_data->createRawStream(); - }; + auto create_tmp_disk_buffer = [tmp_data](const WriteBufferPtr &) -> WriteBufferPtr { return tmp_data->createRawStream(); }; cascade_buffer2.emplace_back(std::move(create_tmp_disk_buffer)); } else { - auto push_memory_buffer_and_continue = [next_buffer = used_output.out_maybe_compressed] (const WriteBufferPtr & prev_buf) + auto push_memory_buffer_and_continue = [next_buffer = used_output.out_maybe_compressed](const WriteBufferPtr & prev_buf) { auto * prev_memory_buffer = typeid_cast(prev_buf.get()); if (!prev_memory_buffer) @@ -694,7 +688,8 @@ void HTTPHandler::processQuery( cascade_buffer2.emplace_back(push_memory_buffer_and_continue); } - used_output.out_delayed_and_compressed_holder = std::make_unique(std::move(cascade_buffer1), std::move(cascade_buffer2)); + used_output.out_delayed_and_compressed_holder + = std::make_unique(std::move(cascade_buffer1), std::move(cascade_buffer2)); used_output.out_maybe_delayed_and_compressed = used_output.out_delayed_and_compressed_holder.get(); } else @@ -707,7 +702,8 @@ void HTTPHandler::processQuery( int zstd_window_log_max = static_cast(context->getSettingsRef().zstd_window_log_max); auto in_post = wrapReadBufferWithCompressionMethod( wrapReadBufferReference(request.getStream()), - chooseCompressionMethod({}, http_request_compression_method_str), zstd_window_log_max); + chooseCompressionMethod({}, http_request_compression_method_str), + zstd_window_log_max); /// The data can also be compressed using incompatible internal algorithm. This is indicated by /// 'decompress' query parameter. @@ -723,12 +719,26 @@ void HTTPHandler::processQuery( std::unique_ptr in; - static const NameSet reserved_param_names{"compress", "decompress", "user", "password", "quota_key", "query_id", "stacktrace", "role", - "buffer_size", "wait_end_of_query", "session_id", "session_timeout", "session_check", "client_protocol_version", "close_session"}; + static const NameSet reserved_param_names{ + "compress", + "decompress", + "user", + "password", + "quota_key", + "query_id", + "stacktrace", + "role", + "buffer_size", + "wait_end_of_query", + "session_id", + "session_timeout", + "session_check", + "client_protocol_version", + "close_session"}; Names reserved_param_suffixes; - auto param_could_be_skipped = [&] (const String & name) + auto param_could_be_skipped = [&](const String & name) { /// Empty parameter appears when URL like ?&a=b or a=b&&c=d. Just skip them for user's convenience. if (name.empty()) @@ -738,10 +748,8 @@ void HTTPHandler::processQuery( return true; for (const String & suffix : reserved_param_suffixes) - { if (endsWith(name, suffix)) return true; - } return false; }; @@ -859,47 +867,47 @@ void HTTPHandler::processQuery( if (settings.add_http_cors_header && !request.get("Origin", "").empty() && !config.has("http_options_response")) used_output.out_holder->addHeaderCORS(true); - auto append_callback = [my_context = context] (ProgressCallback callback) + auto append_callback = [my_context = context](ProgressCallback callback) { auto prev = my_context->getProgressCallback(); - my_context->setProgressCallback([prev, callback] (const Progress & progress) - { - if (prev) - prev(progress); + my_context->setProgressCallback( + [prev, callback](const Progress & progress) + { + if (prev) + prev(progress); - callback(progress); - }); + callback(progress); + }); }; /// While still no data has been sent, we will report about query execution progress by sending HTTP headers. /// Note that we add it unconditionally so the progress is available for `X-ClickHouse-Summary` - append_callback([&used_output](const Progress & progress) - { - used_output.out_holder->onProgress(progress); - }); + append_callback([&used_output](const Progress & progress) { used_output.out_holder->onProgress(progress); }); if (settings.readonly > 0 && settings.cancel_http_readonly_queries_on_client_close) { - append_callback([&context, &request](const Progress &) - { - /// Assume that at the point this method is called no one is reading data from the socket any more: - /// should be true for read-only queries. - if (!request.checkPeerConnected()) - context->killCurrentQuery(); - }); + append_callback( + [&context, &request](const Progress &) + { + /// Assume that at the point this method is called no one is reading data from the socket any more: + /// should be true for read-only queries. + if (!request.checkPeerConnected()) + context->killCurrentQuery(); + }); } customizeContext(request, context, *in_post_maybe_compressed); in = has_external_data ? std::move(in_param) : std::make_unique(*in_param, *in_post_maybe_compressed); - auto set_query_result = [&response, this] (const QueryResultDetails & details) + auto set_query_result = [&response, this](const QueryResultDetails & details) { response.add("X-ClickHouse-Query-Id", details.query_id); + if (http_response_headers_override) + for (auto [header_name, header_value] : *http_response_headers_override) + response.add(header_name, header_value); - if (content_type_override) - response.setContentType(*content_type_override); - else if (details.content_type) + if (response.getContentType() == Poco::Net::HTTPMessage::UNKNOWN_CONTENT_TYPE && details.content_type) response.setContentType(*details.content_type); if (details.format) @@ -909,7 +917,10 @@ void HTTPHandler::processQuery( response.add("X-ClickHouse-Timezone", *details.timezone); }; - auto handle_exception_in_output_format = [&](IOutputFormat & current_output_format, const String & format_name, const ContextPtr & context_, const std::optional & format_settings) + auto handle_exception_in_output_format = [&](IOutputFormat & current_output_format, + const String & format_name, + const ContextPtr & context_, + const std::optional & format_settings) { if (settings.http_write_exception_in_output_format && current_output_format.supportsWritingException()) { @@ -929,7 +940,8 @@ void HTTPHandler::processQuery( } else { - bool with_stacktrace = (params.getParsed("stacktrace", false) && server.config().getBool("enable_http_stacktrace", true)); + bool with_stacktrace + = (params.getParsed("stacktrace", false) && server.config().getBool("enable_http_stacktrace", true)); ExecutionStatus status = ExecutionStatus::fromCurrentException("", with_stacktrace); formatExceptionForClient(status.code, request, response, used_output); current_output_format.setException(status.message); @@ -970,7 +982,8 @@ try if (!used_output.out_holder && !used_output.exception_is_written) { /// If nothing was sent yet and we don't even know if we must compress the response. - WriteBufferFromHTTPServerResponse(response, request.getMethod() == HTTPRequest::HTTP_HEAD, DEFAULT_HTTP_KEEP_ALIVE_TIMEOUT).writeln(s); + WriteBufferFromHTTPServerResponse(response, request.getMethod() == HTTPRequest::HTTP_HEAD, DEFAULT_HTTP_KEEP_ALIVE_TIMEOUT) + .writeln(s); } else if (used_output.out_maybe_compressed) { @@ -1034,7 +1047,8 @@ catch (...) } } -void HTTPHandler::formatExceptionForClient(int exception_code, HTTPServerRequest & request, HTTPServerResponse & response, Output & used_output) +void HTTPHandler::formatExceptionForClient( + int exception_code, HTTPServerRequest & request, HTTPServerResponse & response, Output & used_output) { if (used_output.out_holder) used_output.out_holder->setExceptionCode(exception_code); @@ -1101,9 +1115,7 @@ void HTTPHandler::handleRequest(HTTPServerRequest & request, HTTPServerResponse std::string opentelemetry_traceparent = request.get("traceparent"); std::string error; if (!client_trace_context.parseTraceparentHeader(opentelemetry_traceparent, error)) - { LOG_DEBUG(log, "Failed to parse OpenTelemetry traceparent header '{}': {}", opentelemetry_traceparent, error); - } client_trace_context.tracestate = request.get("tracestate", ""); } @@ -1141,9 +1153,10 @@ void HTTPHandler::handleRequest(HTTPServerRequest & request, HTTPServerResponse /// Workaround. Poco does not detect 411 Length Required case. if (request.getMethod() == HTTPRequest::HTTP_POST && !request.getChunkedTransferEncoding() && !request.hasContentLength()) { - throw Exception(ErrorCodes::HTTP_LENGTH_REQUIRED, - "The Transfer-Encoding is not chunked and there " - "is no Content-Length header for POST request"); + throw Exception( + ErrorCodes::HTTP_LENGTH_REQUIRED, + "The Transfer-Encoding is not chunked and there " + "is no Content-Length header for POST request"); } processQuery(request, params, response, used_output, query_scope, write_event); @@ -1155,7 +1168,8 @@ void HTTPHandler::handleRequest(HTTPServerRequest & request, HTTPServerResponse catch (...) { SCOPE_EXIT({ - request_credentials.reset(); // ...so that the next requests on the connection have to always start afresh in case of exceptions. + request_credentials + .reset(); // ...so that the next requests on the connection have to always start afresh in case of exceptions. }); /// Check if exception was thrown in used_output.finalize(). @@ -1185,15 +1199,18 @@ void HTTPHandler::handleRequest(HTTPServerRequest & request, HTTPServerResponse used_output.finalize(); } -DynamicQueryHandler::DynamicQueryHandler(IServer & server_, const std::string & param_name_, const std::optional& content_type_override_) - : HTTPHandler(server_, "DynamicQueryHandler", content_type_override_), param_name(param_name_) +DynamicQueryHandler::DynamicQueryHandler( + IServer & server_, + const std::string & param_name_, + const std::optional> & http_response_headers_override_) + : HTTPHandler(server_, "DynamicQueryHandler", http_response_headers_override_), param_name(param_name_) { } bool DynamicQueryHandler::customizeQueryParam(ContextMutablePtr context, const std::string & key, const std::string & value) { if (key == param_name) - return true; /// do nothing + return true; /// do nothing if (startsWith(key, QUERY_PARAMETER_NAME_PREFIX)) { @@ -1227,16 +1244,10 @@ std::string DynamicQueryHandler::getQuery(HTTPServerRequest & request, HTMLForm std::string full_query; /// Params are of both form params POST and uri (GET params) for (const auto & it : params) - { if (it.first == param_name) - { full_query += it.second; - } else - { customizeQueryParam(context, it.first, it.second); - } - } return full_query; } @@ -1247,8 +1258,8 @@ PredefinedQueryHandler::PredefinedQueryHandler( const std::string & predefined_query_, const CompiledRegexPtr & url_regex_, const std::unordered_map & header_name_with_regex_, - const std::optional & content_type_override_) - : HTTPHandler(server_, "PredefinedQueryHandler", content_type_override_) + const std::optional> & http_response_headers_override_) + : HTTPHandler(server_, "PredefinedQueryHandler", http_response_headers_override_) , receive_params(receive_params_) , predefined_query(predefined_query_) , url_regex(url_regex_) @@ -1334,20 +1345,37 @@ std::string PredefinedQueryHandler::getQuery(HTTPServerRequest & request, HTMLFo return predefined_query; } -HTTPRequestHandlerFactoryPtr createDynamicHandlerFactory(IServer & server, - const Poco::Util::AbstractConfiguration & config, - const std::string & config_prefix) +std::optional> +parseHttpResponseHeaders(const Poco::Util::AbstractConfiguration & config, const std::string & config_prefix) +{ + std::unordered_map http_response_headers_override; + String http_response_headers_key = config_prefix + ".handler.http_response_headers"; + String http_response_headers_key_prefix = http_response_headers_key + "."; + if (config.has(http_response_headers_key)) + { + Poco::Util::AbstractConfiguration::Keys keys; + config.keys(config_prefix + ".handler.http_response_headers", keys); + for (const auto & key : keys) + http_response_headers_override[key] = config.getString(http_response_headers_key_prefix + key); + } + if (config.has(config_prefix + ".handler.content_type")) + http_response_headers_override[Poco::Net::HTTPMessage::CONTENT_TYPE] = config.getString(config_prefix + ".handler.content_type"); + + if (http_response_headers_override.empty()) + return std::nullopt; + + return std::optional(std::move(http_response_headers_override)); +} + +HTTPRequestHandlerFactoryPtr +createDynamicHandlerFactory(IServer & server, const Poco::Util::AbstractConfiguration & config, const std::string & config_prefix) { auto query_param_name = config.getString(config_prefix + ".handler.query_param_name", "query"); - std::optional content_type_override; - if (config.has(config_prefix + ".handler.content_type")) - content_type_override = config.getString(config_prefix + ".handler.content_type"); + std::optional> http_response_headers_override = parseHttpResponseHeaders(config, config_prefix); - auto creator = [&server, query_param_name, content_type_override] () -> std::unique_ptr - { - return std::make_unique(server, query_param_name, content_type_override); - }; + auto creator = [&server, query_param_name, http_response_headers_override]() -> std::unique_ptr + { return std::make_unique(server, query_param_name, http_response_headers_override); }; auto factory = std::make_shared>(std::move(creator)); factory->addFiltersFromConfig(config, config_prefix); @@ -1357,11 +1385,14 @@ HTTPRequestHandlerFactoryPtr createDynamicHandlerFactory(IServer & server, static inline bool capturingNamedQueryParam(NameSet receive_params, const CompiledRegexPtr & compiled_regex) { const auto & capturing_names = compiled_regex->NamedCapturingGroups(); - return std::count_if(capturing_names.begin(), capturing_names.end(), [&](const auto & iterator) - { - return std::count_if(receive_params.begin(), receive_params.end(), - [&](const auto & param_name) { return param_name == iterator.first; }); - }); + return std::count_if( + capturing_names.begin(), + capturing_names.end(), + [&](const auto & iterator) + { + return std::count_if( + receive_params.begin(), receive_params.end(), [&](const auto & param_name) { return param_name == iterator.first; }); + }); } static inline CompiledRegexPtr getCompiledRegex(const std::string & expression) @@ -1369,15 +1400,18 @@ static inline CompiledRegexPtr getCompiledRegex(const std::string & expression) auto compiled_regex = std::make_shared(expression); if (!compiled_regex->ok()) - throw Exception(ErrorCodes::CANNOT_COMPILE_REGEXP, "Cannot compile re2: {} for http handling rule, error: {}. " - "Look at https://github.com/google/re2/wiki/Syntax for reference.", expression, compiled_regex->error()); + throw Exception( + ErrorCodes::CANNOT_COMPILE_REGEXP, + "Cannot compile re2: {} for http handling rule, error: {}. " + "Look at https://github.com/google/re2/wiki/Syntax for reference.", + expression, + compiled_regex->error()); return compiled_regex; } -HTTPRequestHandlerFactoryPtr createPredefinedHandlerFactory(IServer & server, - const Poco::Util::AbstractConfiguration & config, - const std::string & config_prefix) +HTTPRequestHandlerFactoryPtr +createPredefinedHandlerFactory(IServer & server, const Poco::Util::AbstractConfiguration & config, const std::string & config_prefix) { if (!config.has(config_prefix + ".handler.query")) throw Exception(ErrorCodes::NO_ELEMENTS_IN_CONFIG, "There is no path '{}.handler.query' in configuration file.", config_prefix); @@ -1402,9 +1436,7 @@ HTTPRequestHandlerFactoryPtr createPredefinedHandlerFactory(IServer & server, headers_name_with_regex.emplace(std::make_pair(header_name, regex)); } - std::optional content_type_override; - if (config.has(config_prefix + ".handler.content_type")) - content_type_override = config.getString(config_prefix + ".handler.content_type"); + std::optional> http_response_headers_override = parseHttpResponseHeaders(config, config_prefix); std::shared_ptr> factory; @@ -1418,18 +1450,12 @@ HTTPRequestHandlerFactoryPtr createPredefinedHandlerFactory(IServer & server, auto regex = getCompiledRegex(url_expression); if (capturingNamedQueryParam(analyze_receive_params, regex)) { - auto creator = [ - &server, - analyze_receive_params, - predefined_query, - regex, - headers_name_with_regex, - content_type_override] + auto creator + = [&server, analyze_receive_params, predefined_query, regex, headers_name_with_regex, http_response_headers_override] -> std::unique_ptr { return std::make_unique( - server, analyze_receive_params, predefined_query, regex, - headers_name_with_regex, content_type_override); + server, analyze_receive_params, predefined_query, regex, headers_name_with_regex, http_response_headers_override); }; factory = std::make_shared>(std::move(creator)); factory->addFiltersFromConfig(config, config_prefix); @@ -1437,17 +1463,11 @@ HTTPRequestHandlerFactoryPtr createPredefinedHandlerFactory(IServer & server, } } - auto creator = [ - &server, - analyze_receive_params, - predefined_query, - headers_name_with_regex, - content_type_override] + auto creator = [&server, analyze_receive_params, predefined_query, headers_name_with_regex, http_response_headers_override] -> std::unique_ptr { return std::make_unique( - server, analyze_receive_params, predefined_query, CompiledRegexPtr{}, - headers_name_with_regex, content_type_override); + server, analyze_receive_params, predefined_query, CompiledRegexPtr{}, headers_name_with_regex, http_response_headers_override); }; factory = std::make_shared>(std::move(creator)); diff --git a/src/Server/HTTPHandler.h b/src/Server/HTTPHandler.h index a96402247a2..5eba8b9d2a6 100644 --- a/src/Server/HTTPHandler.h +++ b/src/Server/HTTPHandler.h @@ -1,21 +1,27 @@ #pragma once +#include +#include +#include +#include #include +#include #include #include #include #include #include -#include -#include #include namespace CurrentMetrics { - extern const Metric HTTPConnection; +extern const Metric HTTPConnection; } -namespace Poco { class Logger; } +namespace Poco +{ +class Logger; +} namespace DB { @@ -31,13 +37,16 @@ using CompiledRegexPtr = std::shared_ptr; class HTTPHandler : public HTTPRequestHandler { public: - HTTPHandler(IServer & server_, const std::string & name, const std::optional & content_type_override_); + HTTPHandler( + IServer & server_, + const std::string & name, + const std::optional> & http_response_headers_override_); ~HTTPHandler() override; void handleRequest(HTTPServerRequest & request, HTTPServerResponse & response, const ProfileEvents::Event & write_event) override; /// This method is called right before the query execution. - virtual void customizeContext(HTTPServerRequest & /* request */, ContextMutablePtr /* context */, ReadBuffer & /* body */) {} + virtual void customizeContext(HTTPServerRequest & /* request */, ContextMutablePtr /* context */, ReadBuffer & /* body */) { } virtual bool customizeQueryParam(ContextMutablePtr context, const std::string & key, const std::string & value) = 0; @@ -113,8 +122,8 @@ private: /// See settings http_max_fields, http_max_field_name_size, http_max_field_value_size in HTMLForm. const Settings & default_settings; - /// Overrides Content-Type provided by the format of the response. - std::optional content_type_override; + /// Overrides for response headers. + std::optional> http_response_headers_override; // session is reset at the end of each request/response. std::unique_ptr session; @@ -128,10 +137,7 @@ private: // Returns false when the user is not authenticated yet, and the 'Negotiate' response is sent, // the session and request_credentials instances are preserved. // Throws an exception if authentication failed. - bool authenticateUser( - HTTPServerRequest & request, - HTMLForm & params, - HTTPServerResponse & response); + bool authenticateUser(HTTPServerRequest & request, HTMLForm & params, HTTPServerResponse & response); /// Also initializes 'used_output'. void processQuery( @@ -143,17 +149,9 @@ private: const ProfileEvents::Event & write_event); void trySendExceptionToClient( - const std::string & s, - int exception_code, - HTTPServerRequest & request, - HTTPServerResponse & response, - Output & used_output); + const std::string & s, int exception_code, HTTPServerRequest & request, HTTPServerResponse & response, Output & used_output); - void formatExceptionForClient( - int exception_code, - HTTPServerRequest & request, - HTTPServerResponse & response, - Output & used_output); + void formatExceptionForClient(int exception_code, HTTPServerRequest & request, HTTPServerResponse & response, Output & used_output); static void pushDelayedResults(Output & used_output); }; @@ -162,12 +160,16 @@ class DynamicQueryHandler : public HTTPHandler { private: std::string param_name; + public: - explicit DynamicQueryHandler(IServer & server_, const std::string & param_name_ = "query", const std::optional& content_type_override_ = std::nullopt); + explicit DynamicQueryHandler( + IServer & server_, + const std::string & param_name_ = "query", + const std::optional> & http_response_headers_override_ = std::nullopt); std::string getQuery(HTTPServerRequest & request, HTMLForm & params, ContextMutablePtr context) override; - bool customizeQueryParam(ContextMutablePtr context, const std::string &key, const std::string &value) override; + bool customizeQueryParam(ContextMutablePtr context, const std::string & key, const std::string & value) override; }; class PredefinedQueryHandler : public HTTPHandler @@ -177,11 +179,15 @@ private: std::string predefined_query; CompiledRegexPtr url_regex; std::unordered_map header_name_with_capture_regex; + public: PredefinedQueryHandler( - IServer & server_, const NameSet & receive_params_, const std::string & predefined_query_ - , const CompiledRegexPtr & url_regex_, const std::unordered_map & header_name_with_regex_ - , const std::optional & content_type_override_); + IServer & server_, + const NameSet & receive_params_, + const std::string & predefined_query_, + const CompiledRegexPtr & url_regex_, + const std::unordered_map & header_name_with_regex_, + const std::optional> & http_response_headers_override_ = std::nullopt); void customizeContext(HTTPServerRequest & request, ContextMutablePtr context, ReadBuffer & body) override; From 1245be5d978e0d0b2e60cc29f571dd7e19684630 Mon Sep 17 00:00:00 2001 From: Grigorii Sokolik Date: Mon, 13 May 2024 10:59:32 +0300 Subject: [PATCH 14/40] feat-59620 tests --- .../test_http_handlers_config/test.py | 16 ++++++++++++++++ .../test_dynamic_handler/config.xml | 4 ++++ .../test_predefined_handler/config.xml | 4 ++++ .../test_static_handler/config.xml | 4 ++++ 4 files changed, 28 insertions(+) diff --git a/tests/integration/test_http_handlers_config/test.py b/tests/integration/test_http_handlers_config/test.py index f6ac42a2db2..d8b58976642 100644 --- a/tests/integration/test_http_handlers_config/test.py +++ b/tests/integration/test_http_handlers_config/test.py @@ -88,6 +88,8 @@ def test_dynamic_query_handler(): "application/whatever; charset=cp1337" == res_custom_ct.headers["content-type"] ) + assert "it works" == res_custom_ct.headers["X-Test-Http-Response-Headers-Works"] + assert "also works" == res_custom_ct.headers["X-Test-Http-Response-Headers-Even-Multiple"] def test_predefined_query_handler(): @@ -146,6 +148,8 @@ def test_predefined_query_handler(): ) assert b"max_final_threads\t1\nmax_threads\t1\n" == res2.content assert "application/generic+one" == res2.headers["content-type"] + assert "it works" == res2.headers["X-Test-Http-Response-Headers-Works"] + assert "also works" == res2.headers["X-Test-Http-Response-Headers-Even-Multiple"] cluster.instance.query( "CREATE TABLE test_table (id UInt32, data String) Engine=TinyLog" @@ -212,6 +216,18 @@ def test_fixed_static_handler(): "test_get_fixed_static_handler", method="GET", headers={"XXX": "xxx"} ).content ) + assert ( + "it works" + == cluster.instance.http_request( + "test_get_fixed_static_handler", method="GET", headers={"XXX": "xxx"} + ).headers["X-Test-Http-Response-Headers-Works"] + ) + assert ( + "also works" + == cluster.instance.http_request( + "test_get_fixed_static_handler", method="GET", headers={"XXX": "xxx"} + ).headers["X-Test-Http-Response-Headers-Even-Multiple"] + ) def test_config_static_handler(): diff --git a/tests/integration/test_http_handlers_config/test_dynamic_handler/config.xml b/tests/integration/test_http_handlers_config/test_dynamic_handler/config.xml index c9b61c21507..58fedbd9078 100644 --- a/tests/integration/test_http_handlers_config/test_dynamic_handler/config.xml +++ b/tests/integration/test_http_handlers_config/test_dynamic_handler/config.xml @@ -18,6 +18,10 @@ dynamic_query_handler get_dynamic_handler_query application/whatever; charset=cp1337 + + it works + also works + diff --git a/tests/integration/test_http_handlers_config/test_predefined_handler/config.xml b/tests/integration/test_http_handlers_config/test_predefined_handler/config.xml index 1b8ddfab323..a7804721f12 100644 --- a/tests/integration/test_http_handlers_config/test_predefined_handler/config.xml +++ b/tests/integration/test_http_handlers_config/test_predefined_handler/config.xml @@ -19,6 +19,10 @@ predefined_query_handler SELECT name, value FROM system.settings WHERE name = {setting_name_1:String} OR name = {setting_name_2:String} application/generic+one + + it works + also works + diff --git a/tests/integration/test_http_handlers_config/test_static_handler/config.xml b/tests/integration/test_http_handlers_config/test_static_handler/config.xml index ff24e6dec96..76c1f588853 100644 --- a/tests/integration/test_http_handlers_config/test_static_handler/config.xml +++ b/tests/integration/test_http_handlers_config/test_static_handler/config.xml @@ -12,6 +12,10 @@ 402 text/html; charset=UTF-8 Test get static handler and fix content + + it works + also works + From a417a1c676f88dd7f793bed486f686a9187c956e Mon Sep 17 00:00:00 2001 From: Grigorii Sokolik Date: Tue, 28 May 2024 16:13:47 +0300 Subject: [PATCH 15/40] feat-59620 Style fix --- src/Server/HTTPHandler.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Server/HTTPHandler.cpp b/src/Server/HTTPHandler.cpp index 3c0bbf03986..81a873c8c49 100644 --- a/src/Server/HTTPHandler.cpp +++ b/src/Server/HTTPHandler.cpp @@ -287,7 +287,7 @@ static std::chrono::steady_clock::duration parseSessionTimeout(const Poco::Util: } std::optional> -parseHttpResponseHeaders(const Poco::Util::AbstractConfiguration & config, const std::string & config_prefix); +parseHTTPResponseHeaders(const Poco::Util::AbstractConfiguration & config, const std::string & config_prefix); void HTTPHandler::pushDelayedResults(Output & used_output) { @@ -1346,7 +1346,7 @@ std::string PredefinedQueryHandler::getQuery(HTTPServerRequest & request, HTMLFo } std::optional> -parseHttpResponseHeaders(const Poco::Util::AbstractConfiguration & config, const std::string & config_prefix) +parseHTTPResponseHeaders(const Poco::Util::AbstractConfiguration & config, const std::string & config_prefix) { std::unordered_map http_response_headers_override; String http_response_headers_key = config_prefix + ".handler.http_response_headers"; @@ -1354,7 +1354,7 @@ parseHttpResponseHeaders(const Poco::Util::AbstractConfiguration & config, const if (config.has(http_response_headers_key)) { Poco::Util::AbstractConfiguration::Keys keys; - config.keys(config_prefix + ".handler.http_response_headers", keys); + config.keys(http_response_headers_key, keys); for (const auto & key : keys) http_response_headers_override[key] = config.getString(http_response_headers_key_prefix + key); } @@ -1372,7 +1372,7 @@ createDynamicHandlerFactory(IServer & server, const Poco::Util::AbstractConfigur { auto query_param_name = config.getString(config_prefix + ".handler.query_param_name", "query"); - std::optional> http_response_headers_override = parseHttpResponseHeaders(config, config_prefix); + std::optional> http_response_headers_override = parseHTTPResponseHeaders(config, config_prefix); auto creator = [&server, query_param_name, http_response_headers_override]() -> std::unique_ptr { return std::make_unique(server, query_param_name, http_response_headers_override); }; @@ -1436,7 +1436,7 @@ createPredefinedHandlerFactory(IServer & server, const Poco::Util::AbstractConfi headers_name_with_regex.emplace(std::make_pair(header_name, regex)); } - std::optional> http_response_headers_override = parseHttpResponseHeaders(config, config_prefix); + std::optional> http_response_headers_override = parseHTTPResponseHeaders(config, config_prefix); std::shared_ptr> factory; From 1c9652c06bace095da1911848f7c1b4680cde164 Mon Sep 17 00:00:00 2001 From: Grigorii Sokolik Date: Tue, 28 May 2024 17:45:20 +0300 Subject: [PATCH 16/40] feat-59620 Style fix --- tests/integration/test_http_handlers_config/test.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/integration/test_http_handlers_config/test.py b/tests/integration/test_http_handlers_config/test.py index d8b58976642..166ff720922 100644 --- a/tests/integration/test_http_handlers_config/test.py +++ b/tests/integration/test_http_handlers_config/test.py @@ -84,10 +84,7 @@ def test_dynamic_query_handler(): headers={"XXX": "xxx"}, ) assert 200 == res_custom_ct.status_code - assert ( - "application/whatever; charset=cp1337" - == res_custom_ct.headers["content-type"] - ) + assert "application/whatever; charset=cp1337" == res_custom_ct.headers["content-type"] assert "it works" == res_custom_ct.headers["X-Test-Http-Response-Headers-Works"] assert "also works" == res_custom_ct.headers["X-Test-Http-Response-Headers-Even-Multiple"] From 1618ce43971cf9432eec8e1e102bd4319c97c161 Mon Sep 17 00:00:00 2001 From: Grigorii Sokolik Date: Tue, 28 May 2024 17:58:03 +0300 Subject: [PATCH 17/40] feat-59620 Style fix --- .../integration/test_http_handlers_config/test.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/tests/integration/test_http_handlers_config/test.py b/tests/integration/test_http_handlers_config/test.py index 166ff720922..b2efbf4bb65 100644 --- a/tests/integration/test_http_handlers_config/test.py +++ b/tests/integration/test_http_handlers_config/test.py @@ -84,9 +84,15 @@ def test_dynamic_query_handler(): headers={"XXX": "xxx"}, ) assert 200 == res_custom_ct.status_code - assert "application/whatever; charset=cp1337" == res_custom_ct.headers["content-type"] + assert ( + "application/whatever; charset=cp1337" + == res_custom_ct.headers["content-type"] + ) assert "it works" == res_custom_ct.headers["X-Test-Http-Response-Headers-Works"] - assert "also works" == res_custom_ct.headers["X-Test-Http-Response-Headers-Even-Multiple"] + assert ( + "also works" + == res_custom_ct.headers["X-Test-Http-Response-Headers-Even-Multiple"] + ) def test_predefined_query_handler(): @@ -146,7 +152,9 @@ def test_predefined_query_handler(): assert b"max_final_threads\t1\nmax_threads\t1\n" == res2.content assert "application/generic+one" == res2.headers["content-type"] assert "it works" == res2.headers["X-Test-Http-Response-Headers-Works"] - assert "also works" == res2.headers["X-Test-Http-Response-Headers-Even-Multiple"] + assert ( + "also works" == res2.headers["X-Test-Http-Response-Headers-Even-Multiple"] + ) cluster.instance.query( "CREATE TABLE test_table (id UInt32, data String) Engine=TinyLog" From ffa66225d07c8e1fd0c2ba2aaab486fc2e81b1d1 Mon Sep 17 00:00:00 2001 From: Grigorii Sokolik Date: Tue, 4 Jun 2024 23:59:08 +0300 Subject: [PATCH 18/40] Fix tests --- src/Server/HTTPHandler.cpp | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/Server/HTTPHandler.cpp b/src/Server/HTTPHandler.cpp index 81a873c8c49..00757142882 100644 --- a/src/Server/HTTPHandler.cpp +++ b/src/Server/HTTPHandler.cpp @@ -900,14 +900,19 @@ void HTTPHandler::processQuery( customizeContext(request, context, *in_post_maybe_compressed); in = has_external_data ? std::move(in_param) : std::make_unique(*in_param, *in_post_maybe_compressed); - auto set_query_result = [&response, this](const QueryResultDetails & details) + if (http_response_headers_override) { + for (auto [header_name, header_value] : *http_response_headers_override) + response.set(header_name, header_value); + } + + auto set_query_result = [this, &response](const QueryResultDetails & details) { response.add("X-ClickHouse-Query-Id", details.query_id); - if (http_response_headers_override) - for (auto [header_name, header_value] : *http_response_headers_override) - response.add(header_name, header_value); - if (response.getContentType() == Poco::Net::HTTPMessage::UNKNOWN_CONTENT_TYPE && details.content_type) + if (!( + http_response_headers_override.has_value() + && http_response_headers_override->contains(Poco::Net::HTTPMessage::CONTENT_TYPE) + ) && details.content_type) response.setContentType(*details.content_type); if (details.format) @@ -1362,9 +1367,9 @@ parseHTTPResponseHeaders(const Poco::Util::AbstractConfiguration & config, const http_response_headers_override[Poco::Net::HTTPMessage::CONTENT_TYPE] = config.getString(config_prefix + ".handler.content_type"); if (http_response_headers_override.empty()) - return std::nullopt; + return {}; - return std::optional(std::move(http_response_headers_override)); + return std::move(http_response_headers_override); } HTTPRequestHandlerFactoryPtr From 979b447451532715167edee437e42d03e6734c9e Mon Sep 17 00:00:00 2001 From: Grigorii Sokolik Date: Wed, 5 Jun 2024 07:51:26 +0300 Subject: [PATCH 19/40] Fix styles --- src/Server/HTTPHandler.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Server/HTTPHandler.cpp b/src/Server/HTTPHandler.cpp index 00757142882..da635f99014 100644 --- a/src/Server/HTTPHandler.cpp +++ b/src/Server/HTTPHandler.cpp @@ -900,10 +900,9 @@ void HTTPHandler::processQuery( customizeContext(request, context, *in_post_maybe_compressed); in = has_external_data ? std::move(in_param) : std::make_unique(*in_param, *in_post_maybe_compressed); - if (http_response_headers_override) { + if (http_response_headers_override) for (auto [header_name, header_value] : *http_response_headers_override) response.set(header_name, header_value); - } auto set_query_result = [this, &response](const QueryResultDetails & details) { From 06383d7a7a1c1a1a111090fc52e863e120232376 Mon Sep 17 00:00:00 2001 From: Grigorii Sokolik Date: Thu, 6 Jun 2024 10:50:31 +0300 Subject: [PATCH 20/40] Fix Static handler --- src/Server/HTTPHandler.cpp | 58 ++++----------- src/Server/HTTPHandler.h | 23 +++--- src/Server/HTTPHandlerFactory.cpp | 96 +++++++++++++------------ src/Server/HTTPResponseHeaderWriter.cpp | 69 ++++++++++++++++++ src/Server/HTTPResponseHeaderWriter.h | 23 ++++++ src/Server/StaticRequestHandler.cpp | 66 +++++++++-------- src/Server/StaticRequestHandler.h | 9 +-- 7 files changed, 204 insertions(+), 140 deletions(-) create mode 100644 src/Server/HTTPResponseHeaderWriter.cpp create mode 100644 src/Server/HTTPResponseHeaderWriter.h diff --git a/src/Server/HTTPHandler.cpp b/src/Server/HTTPHandler.cpp index da635f99014..a2af9905c72 100644 --- a/src/Server/HTTPHandler.cpp +++ b/src/Server/HTTPHandler.cpp @@ -27,6 +27,7 @@ #include #include #include +#include "Common/logger_useful.h" #include #include #include @@ -286,9 +287,6 @@ static std::chrono::steady_clock::duration parseSessionTimeout(const Poco::Util: return std::chrono::seconds(session_timeout); } -std::optional> -parseHTTPResponseHeaders(const Poco::Util::AbstractConfiguration & config, const std::string & config_prefix); - void HTTPHandler::pushDelayedResults(Output & used_output) { std::vector write_buffers; @@ -325,8 +323,7 @@ void HTTPHandler::pushDelayedResults(Output & used_output) } -HTTPHandler::HTTPHandler( - IServer & server_, const std::string & name, const std::optional> & http_response_headers_override_) +HTTPHandler::HTTPHandler(IServer & server_, const std::string & name, const HTTPResponseHeaderSetup & http_response_headers_override_) : server(server_) , log(getLogger(name)) , default_settings(server.context()->getSettingsRef()) @@ -711,7 +708,8 @@ void HTTPHandler::processQuery( bool is_in_post_compressed = false; if (params.getParsed("decompress", false)) { - in_post_maybe_compressed = std::make_unique(*in_post, /* allow_different_codecs_ = */ false, /* external_data_ = */ true); + in_post_maybe_compressed + = std::make_unique(*in_post, /* allow_different_codecs_ = */ false, /* external_data_ = */ true); is_in_post_compressed = true; } else @@ -900,18 +898,14 @@ void HTTPHandler::processQuery( customizeContext(request, context, *in_post_maybe_compressed); in = has_external_data ? std::move(in_param) : std::make_unique(*in_param, *in_post_maybe_compressed); - if (http_response_headers_override) - for (auto [header_name, header_value] : *http_response_headers_override) - response.set(header_name, header_value); + applyHTTPResponseHeaders(response, http_response_headers_override); auto set_query_result = [this, &response](const QueryResultDetails & details) { response.add("X-ClickHouse-Query-Id", details.query_id); - if (!( - http_response_headers_override.has_value() - && http_response_headers_override->contains(Poco::Net::HTTPMessage::CONTENT_TYPE) - ) && details.content_type) + if (!(http_response_headers_override && http_response_headers_override->contains(Poco::Net::HTTPMessage::CONTENT_TYPE)) + && details.content_type) response.setContentType(*details.content_type); if (details.format) @@ -1125,10 +1119,8 @@ void HTTPHandler::handleRequest(HTTPServerRequest & request, HTTPServerResponse // Setup tracing context for this thread auto context = session->sessionOrGlobalContext(); - thread_trace_context = std::make_unique("HTTPHandler", - client_trace_context, - context->getSettingsRef(), - context->getOpenTelemetrySpanLog()); + thread_trace_context = std::make_unique( + "HTTPHandler", client_trace_context, context->getSettingsRef(), context->getOpenTelemetrySpanLog()); thread_trace_context->root_span.kind = OpenTelemetry::SpanKind::SERVER; thread_trace_context->root_span.addAttribute("clickhouse.uri", request.getURI()); @@ -1204,9 +1196,7 @@ void HTTPHandler::handleRequest(HTTPServerRequest & request, HTTPServerResponse } DynamicQueryHandler::DynamicQueryHandler( - IServer & server_, - const std::string & param_name_, - const std::optional> & http_response_headers_override_) + IServer & server_, const std::string & param_name_, const HTTPResponseHeaderSetup & http_response_headers_override_) : HTTPHandler(server_, "DynamicQueryHandler", http_response_headers_override_), param_name(param_name_) { } @@ -1262,7 +1252,7 @@ PredefinedQueryHandler::PredefinedQueryHandler( const std::string & predefined_query_, const CompiledRegexPtr & url_regex_, const std::unordered_map & header_name_with_regex_, - const std::optional> & http_response_headers_override_) + const HTTPResponseHeaderSetup & http_response_headers_override_) : HTTPHandler(server_, "PredefinedQueryHandler", http_response_headers_override_) , receive_params(receive_params_) , predefined_query(predefined_query_) @@ -1349,34 +1339,12 @@ std::string PredefinedQueryHandler::getQuery(HTTPServerRequest & request, HTMLFo return predefined_query; } -std::optional> -parseHTTPResponseHeaders(const Poco::Util::AbstractConfiguration & config, const std::string & config_prefix) -{ - std::unordered_map http_response_headers_override; - String http_response_headers_key = config_prefix + ".handler.http_response_headers"; - String http_response_headers_key_prefix = http_response_headers_key + "."; - if (config.has(http_response_headers_key)) - { - Poco::Util::AbstractConfiguration::Keys keys; - config.keys(http_response_headers_key, keys); - for (const auto & key : keys) - http_response_headers_override[key] = config.getString(http_response_headers_key_prefix + key); - } - if (config.has(config_prefix + ".handler.content_type")) - http_response_headers_override[Poco::Net::HTTPMessage::CONTENT_TYPE] = config.getString(config_prefix + ".handler.content_type"); - - if (http_response_headers_override.empty()) - return {}; - - return std::move(http_response_headers_override); -} - HTTPRequestHandlerFactoryPtr createDynamicHandlerFactory(IServer & server, const Poco::Util::AbstractConfiguration & config, const std::string & config_prefix) { auto query_param_name = config.getString(config_prefix + ".handler.query_param_name", "query"); - std::optional> http_response_headers_override = parseHTTPResponseHeaders(config, config_prefix); + HTTPResponseHeaderSetup http_response_headers_override = parseHTTPResponseHeaders(config, config_prefix); auto creator = [&server, query_param_name, http_response_headers_override]() -> std::unique_ptr { return std::make_unique(server, query_param_name, http_response_headers_override); }; @@ -1440,7 +1408,7 @@ createPredefinedHandlerFactory(IServer & server, const Poco::Util::AbstractConfi headers_name_with_regex.emplace(std::make_pair(header_name, regex)); } - std::optional> http_response_headers_override = parseHTTPResponseHeaders(config, config_prefix); + HTTPResponseHeaderSetup http_response_headers_override = parseHTTPResponseHeaders(config, config_prefix); std::shared_ptr> factory; diff --git a/src/Server/HTTPHandler.h b/src/Server/HTTPHandler.h index 5eba8b9d2a6..1bf1dbdebf8 100644 --- a/src/Server/HTTPHandler.h +++ b/src/Server/HTTPHandler.h @@ -13,6 +13,8 @@ #include #include +#include "HTTPResponseHeaderWriter.h" + namespace CurrentMetrics { extern const Metric HTTPConnection; @@ -37,10 +39,7 @@ using CompiledRegexPtr = std::shared_ptr; class HTTPHandler : public HTTPRequestHandler { public: - HTTPHandler( - IServer & server_, - const std::string & name, - const std::optional> & http_response_headers_override_); + HTTPHandler(IServer & server_, const std::string & name, const HTTPResponseHeaderSetup & http_response_headers_override_); ~HTTPHandler() override; void handleRequest(HTTPServerRequest & request, HTTPServerResponse & response, const ProfileEvents::Event & write_event) override; @@ -86,10 +85,7 @@ private: bool exception_is_written = false; std::function exception_writer; - bool hasDelayed() const - { - return out_maybe_delayed_and_compressed != out_maybe_compressed.get(); - } + bool hasDelayed() const { return out_maybe_delayed_and_compressed != out_maybe_compressed.get(); } void finalize() { @@ -103,10 +99,7 @@ private: out->finalize(); } - bool isFinalized() const - { - return finalized; - } + bool isFinalized() const { return finalized; } }; IServer & server; @@ -123,7 +116,7 @@ private: const Settings & default_settings; /// Overrides for response headers. - std::optional> http_response_headers_override; + HTTPResponseHeaderSetup http_response_headers_override; // session is reset at the end of each request/response. std::unique_ptr session; @@ -165,7 +158,7 @@ public: explicit DynamicQueryHandler( IServer & server_, const std::string & param_name_ = "query", - const std::optional> & http_response_headers_override_ = std::nullopt); + const HTTPResponseHeaderSetup & http_response_headers_override_ = std::nullopt); std::string getQuery(HTTPServerRequest & request, HTMLForm & params, ContextMutablePtr context) override; @@ -187,7 +180,7 @@ public: const std::string & predefined_query_, const CompiledRegexPtr & url_regex_, const std::unordered_map & header_name_with_regex_, - const std::optional> & http_response_headers_override_ = std::nullopt); + const HTTPResponseHeaderSetup & http_response_headers_override_ = std::nullopt); void customizeContext(HTTPServerRequest & request, ContextMutablePtr context, ReadBuffer & body) override; diff --git a/src/Server/HTTPHandlerFactory.cpp b/src/Server/HTTPHandlerFactory.cpp index 9a67e576345..d125e08c704 100644 --- a/src/Server/HTTPHandlerFactory.cpp +++ b/src/Server/HTTPHandlerFactory.cpp @@ -1,18 +1,18 @@ #include #include +#include #include #include -#include #include #include "HTTPHandler.h" -#include "Server/PrometheusMetricsWriter.h" -#include "StaticRequestHandler.h" -#include "ReplicasStatusHandler.h" #include "InterserverIOHTTPHandler.h" #include "PrometheusRequestHandler.h" +#include "ReplicasStatusHandler.h" +#include "Server/PrometheusMetricsWriter.h" +#include "StaticRequestHandler.h" #include "WebUIRequestHandler.h" @@ -21,9 +21,9 @@ namespace DB namespace ErrorCodes { - extern const int LOGICAL_ERROR; - extern const int UNKNOWN_ELEMENT_IN_CONFIG; - extern const int INVALID_CONFIG_PARAMETER; +extern const int LOGICAL_ERROR; +extern const int UNKNOWN_ELEMENT_IN_CONFIG; +extern const int INVALID_CONFIG_PARAMETER; } namespace @@ -35,10 +35,7 @@ private: std::string url; public: - explicit RedirectRequestHandler(std::string url_) - : url(std::move(url_)) - { - } + explicit RedirectRequestHandler(std::string url_) : url(std::move(url_)) { } void handleRequest(HTTPServerRequest &, HTTPServerResponse & response, const ProfileEvents::Event &) override { @@ -46,9 +43,8 @@ public: } }; -HTTPRequestHandlerFactoryPtr createRedirectHandlerFactory( - const Poco::Util::AbstractConfiguration & config, - const std::string & config_prefix) +HTTPRequestHandlerFactoryPtr +createRedirectHandlerFactory(const Poco::Util::AbstractConfiguration & config, const std::string & config_prefix) { std::string url = config.getString(config_prefix + ".handler.location"); @@ -74,7 +70,8 @@ static auto createPingHandlerFactory(IServer & server) auto creator = [&server]() -> std::unique_ptr { constexpr auto ping_response_expression = "Ok.\n"; - return std::make_unique(server, ping_response_expression); + return std::make_unique( + server, ping_response_expression, parseHTTPResponseHeaders("text/html; charset=UTF-8")); }; return std::make_shared>(std::move(creator)); } @@ -102,8 +99,12 @@ static inline auto createHandlersFactoryFromConfig( const auto & handler_type = config.getString(prefix + "." + key + ".handler.type", ""); if (handler_type.empty()) - throw Exception(ErrorCodes::INVALID_CONFIG_PARAMETER, "Handler type in config is not specified here: " - "{}.{}.handler.type", prefix, key); + throw Exception( + ErrorCodes::INVALID_CONFIG_PARAMETER, + "Handler type in config is not specified here: " + "{}.{}.handler.type", + prefix, + key); if (handler_type == "static") { @@ -154,19 +155,27 @@ static inline auto createHandlersFactoryFromConfig( main_handler_factory->addHandler(std::move(handler)); } else - throw Exception(ErrorCodes::INVALID_CONFIG_PARAMETER, "Unknown handler type '{}' in config here: {}.{}.handler.type", - handler_type, prefix, key); + throw Exception( + ErrorCodes::INVALID_CONFIG_PARAMETER, + "Unknown handler type '{}' in config here: {}.{}.handler.type", + handler_type, + prefix, + key); } else - throw Exception(ErrorCodes::UNKNOWN_ELEMENT_IN_CONFIG, "Unknown element in config: " - "{}.{}, must be 'rule' or 'defaults'", prefix, key); + throw Exception( + ErrorCodes::UNKNOWN_ELEMENT_IN_CONFIG, + "Unknown element in config: " + "{}.{}, must be 'rule' or 'defaults'", + prefix, + key); } return main_handler_factory; } -static inline HTTPRequestHandlerFactoryPtr -createHTTPHandlerFactory(IServer & server, const Poco::Util::AbstractConfiguration & config, const std::string & name, AsynchronousMetrics & async_metrics) +static inline HTTPRequestHandlerFactoryPtr createHTTPHandlerFactory( + IServer & server, const Poco::Util::AbstractConfiguration & config, const std::string & name, AsynchronousMetrics & async_metrics) { if (config.has("http_handlers")) { @@ -193,7 +202,8 @@ static inline HTTPRequestHandlerFactoryPtr createInterserverHTTPHandlerFactory(I } -HTTPRequestHandlerFactoryPtr createHandlerFactory(IServer & server, const Poco::Util::AbstractConfiguration & config, AsynchronousMetrics & async_metrics, const std::string & name) +HTTPRequestHandlerFactoryPtr createHandlerFactory( + IServer & server, const Poco::Util::AbstractConfiguration & config, AsynchronousMetrics & async_metrics, const std::string & name) { if (name == "HTTPHandler-factory" || name == "HTTPSHandler-factory") return createHTTPHandlerFactory(server, config, name, async_metrics); @@ -214,7 +224,8 @@ void addCommonDefaultHandlersFactory(HTTPRequestHandlerFactoryMain & factory, IS auto root_creator = [&server]() -> std::unique_ptr { constexpr auto root_response_expression = "config://http_server_default_response"; - return std::make_unique(server, root_response_expression); + return std::make_unique( + server, root_response_expression, parseHTTPResponseHeaders("text/html; charset=UTF-8")); }; auto root_handler = std::make_shared>(std::move(root_creator)); root_handler->attachStrictPath("/"); @@ -265,28 +276,23 @@ void addDefaultHandlersFactory( { addCommonDefaultHandlersFactory(factory, server); - auto dynamic_creator = [&server] () -> std::unique_ptr - { - return std::make_unique(server, "query"); - }; + auto dynamic_creator + = [&server]() -> std::unique_ptr { return std::make_unique(server, "query"); }; auto query_handler = std::make_shared>(std::move(dynamic_creator)); - query_handler->addFilter([](const auto & request) + query_handler->addFilter( + [](const auto & request) { - bool path_matches_get_or_head = startsWith(request.getURI(), "?") - || startsWith(request.getURI(), "/?") - || startsWith(request.getURI(), "/query?"); - bool is_get_or_head_request = request.getMethod() == Poco::Net::HTTPRequest::HTTP_GET - || request.getMethod() == Poco::Net::HTTPRequest::HTTP_HEAD; + bool path_matches_get_or_head + = startsWith(request.getURI(), "?") || startsWith(request.getURI(), "/?") || startsWith(request.getURI(), "/query?"); + bool is_get_or_head_request + = request.getMethod() == Poco::Net::HTTPRequest::HTTP_GET || request.getMethod() == Poco::Net::HTTPRequest::HTTP_HEAD; - bool path_matches_post_or_options = path_matches_get_or_head - || request.getURI() == "/" - || request.getURI().empty(); - bool is_post_or_options_request = request.getMethod() == Poco::Net::HTTPRequest::HTTP_POST - || request.getMethod() == Poco::Net::HTTPRequest::HTTP_OPTIONS; + bool path_matches_post_or_options = path_matches_get_or_head || request.getURI() == "/" || request.getURI().empty(); + bool is_post_or_options_request + = request.getMethod() == Poco::Net::HTTPRequest::HTTP_POST || request.getMethod() == Poco::Net::HTTPRequest::HTTP_OPTIONS; return (path_matches_get_or_head && is_get_or_head_request) || (path_matches_post_or_options && is_post_or_options_request); - } - ); + }); factory.addHandler(query_handler); /// We check that prometheus handler will be served on current (default) port. @@ -294,10 +300,8 @@ void addDefaultHandlersFactory( if (config.has("prometheus") && config.getInt("prometheus.port", 0) == 0) { auto writer = std::make_shared(config, "prometheus", async_metrics); - auto creator = [&server, writer] () -> std::unique_ptr - { - return std::make_unique(server, writer); - }; + auto creator = [&server, writer]() -> std::unique_ptr + { return std::make_unique(server, writer); }; auto prometheus_handler = std::make_shared>(std::move(creator)); prometheus_handler->attachStrictPath(config.getString("prometheus.endpoint", "/metrics")); prometheus_handler->allowGetAndHeadRequest(); diff --git a/src/Server/HTTPResponseHeaderWriter.cpp b/src/Server/HTTPResponseHeaderWriter.cpp new file mode 100644 index 00000000000..f5ab196c4b0 --- /dev/null +++ b/src/Server/HTTPResponseHeaderWriter.cpp @@ -0,0 +1,69 @@ +#include "HTTPResponseHeaderWriter.h" +#include +#include +#include + +namespace DB +{ + +std::unordered_map +baseParseHTTPResponseHeaders(const Poco::Util::AbstractConfiguration & config, const std::string & config_prefix) +{ + std::unordered_map http_response_headers_override; + String http_response_headers_key = config_prefix + ".handler.http_response_headers"; + String http_response_headers_key_prefix = http_response_headers_key + "."; + if (config.has(http_response_headers_key)) + { + Poco::Util::AbstractConfiguration::Keys keys; + config.keys(http_response_headers_key, keys); + for (const auto & key : keys) + { + http_response_headers_override[key] = config.getString(http_response_headers_key_prefix + key); + } + } + if (config.has(config_prefix + ".handler.content_type")) + http_response_headers_override[Poco::Net::HTTPMessage::CONTENT_TYPE] = config.getString(config_prefix + ".handler.content_type"); + + return http_response_headers_override; +} + +HTTPResponseHeaderSetup parseHTTPResponseHeaders(const Poco::Util::AbstractConfiguration & config, const std::string & config_prefix) +{ + std::unordered_map http_response_headers_override = baseParseHTTPResponseHeaders(config, config_prefix); + + if (http_response_headers_override.empty()) + return {}; + + return std::move(http_response_headers_override); +} + +std::unordered_map parseHTTPResponseHeaders( + const Poco::Util::AbstractConfiguration & config, const std::string & config_prefix, const std::string & default_content_type) +{ + std::unordered_map http_response_headers_override = baseParseHTTPResponseHeaders(config, config_prefix); + + if (!http_response_headers_override.contains(Poco::Net::HTTPMessage::CONTENT_TYPE)) + http_response_headers_override[Poco::Net::HTTPMessage::CONTENT_TYPE] = default_content_type; + + return http_response_headers_override; +} + +std::unordered_map parseHTTPResponseHeaders(const std::string & default_content_type) +{ + return {{{Poco::Net::HTTPMessage::CONTENT_TYPE, default_content_type}}}; +} + +void applyHTTPResponseHeaders(Poco::Net::HTTPResponse & response, const HTTPResponseHeaderSetup & setup) +{ + if (setup) + for (auto [header_name, header_value] : *setup) + response.set(header_name, header_value); +} + +void applyHTTPResponseHeaders(Poco::Net::HTTPResponse & response, const std::unordered_map & setup) +{ + for (auto [header_name, header_value] : setup) + response.set(header_name, header_value); +} + +} diff --git a/src/Server/HTTPResponseHeaderWriter.h b/src/Server/HTTPResponseHeaderWriter.h new file mode 100644 index 00000000000..066cf84eca7 --- /dev/null +++ b/src/Server/HTTPResponseHeaderWriter.h @@ -0,0 +1,23 @@ +#include +#include +#include +#include +#include +#include + +namespace DB +{ + +using HTTPResponseHeaderSetup = std::optional>; + +HTTPResponseHeaderSetup parseHTTPResponseHeaders(const Poco::Util::AbstractConfiguration & config, const std::string & config_prefix); + +std::unordered_map parseHTTPResponseHeaders( + const Poco::Util::AbstractConfiguration & config, const std::string & config_prefix, const std::string & default_content_type); + +std::unordered_map parseHTTPResponseHeaders(const std::string & default_content_type); + +void applyHTTPResponseHeaders(Poco::Net::HTTPResponse & response, const HTTPResponseHeaderSetup & setup); + +void applyHTTPResponseHeaders(Poco::Net::HTTPResponse & response, const std::unordered_map & setup); +} diff --git a/src/Server/StaticRequestHandler.cpp b/src/Server/StaticRequestHandler.cpp index 67bf3875de4..e320507fc66 100644 --- a/src/Server/StaticRequestHandler.cpp +++ b/src/Server/StaticRequestHandler.cpp @@ -2,23 +2,24 @@ #include "IServer.h" #include "HTTPHandlerFactory.h" -#include "HTTPHandlerRequestFilter.h" +#include "HTTPResponseHeaderWriter.h" #include #include #include -#include #include -#include +#include #include +#include #include +#include +#include +#include #include #include -#include #include -#include namespace fs = std::filesystem; @@ -28,15 +29,16 @@ namespace DB namespace ErrorCodes { - extern const int INCORRECT_FILE_NAME; - extern const int HTTP_LENGTH_REQUIRED; - extern const int INVALID_CONFIG_PARAMETER; +extern const int INCORRECT_FILE_NAME; +extern const int HTTP_LENGTH_REQUIRED; +extern const int INVALID_CONFIG_PARAMETER; } static inline std::unique_ptr responseWriteBuffer(HTTPServerRequest & request, HTTPServerResponse & response, UInt64 keep_alive_timeout) { - auto buf = std::unique_ptr(new WriteBufferFromHTTPServerResponse(response, request.getMethod() == HTTPRequest::HTTP_HEAD, keep_alive_timeout)); + auto buf = std::unique_ptr( + new WriteBufferFromHTTPServerResponse(response, request.getMethod() == HTTPRequest::HTTP_HEAD, keep_alive_timeout)); /// The client can pass a HTTP header indicating supported compression method (gzip or deflate). String http_response_compression_methods = request.get("Accept-Encoding", ""); @@ -61,8 +63,8 @@ static inline void trySendExceptionToClient( /// If HTTP method is POST and Keep-Alive is turned on, we should read the whole request body /// to avoid reading part of the current request body in the next request. - if (request.getMethod() == Poco::Net::HTTPRequest::HTTP_POST - && response.getKeepAlive() && !request.getStream().eof() && exception_code != ErrorCodes::HTTP_LENGTH_REQUIRED) + if (request.getMethod() == Poco::Net::HTTPRequest::HTTP_POST && response.getKeepAlive() && !request.getStream().eof() + && exception_code != ErrorCodes::HTTP_LENGTH_REQUIRED) request.getStream().ignore(std::numeric_limits::max()); response.setStatusAndReason(Poco::Net::HTTPResponse::HTTP_INTERNAL_SERVER_ERROR); @@ -87,23 +89,26 @@ static inline void trySendExceptionToClient( } } -void StaticRequestHandler::handleRequest(HTTPServerRequest & request, HTTPServerResponse & response, const ProfileEvents::Event & /*write_event*/) +void StaticRequestHandler::handleRequest( + HTTPServerRequest & request, HTTPServerResponse & response, const ProfileEvents::Event & /*write_event*/) { auto keep_alive_timeout = server.context()->getServerSettings().keep_alive_timeout.totalSeconds(); auto out = responseWriteBuffer(request, response, keep_alive_timeout); try { - response.setContentType(content_type); + applyHTTPResponseHeaders(response, http_response_headers_override); if (request.getVersion() == Poco::Net::HTTPServerRequest::HTTP_1_1) response.setChunkedTransferEncoding(true); /// Workaround. Poco does not detect 411 Length Required case. - if (request.getMethod() == Poco::Net::HTTPRequest::HTTP_POST && !request.getChunkedTransferEncoding() && !request.hasContentLength()) - throw Exception(ErrorCodes::HTTP_LENGTH_REQUIRED, - "The Transfer-Encoding is not chunked and there " - "is no Content-Length header for POST request"); + if (request.getMethod() == Poco::Net::HTTPRequest::HTTP_POST && !request.getChunkedTransferEncoding() + && !request.hasContentLength()) + throw Exception( + ErrorCodes::HTTP_LENGTH_REQUIRED, + "The Transfer-Encoding is not chunked and there " + "is no Content-Length header for POST request"); setResponseDefaultHeaders(response, keep_alive_timeout); response.setStatusAndReason(Poco::Net::HTTPResponse::HTTPStatus(status)); @@ -144,9 +149,10 @@ void StaticRequestHandler::writeResponse(WriteBuffer & out) else if (startsWith(response_expression, config_prefix)) { if (response_expression.size() <= config_prefix.size()) - throw Exception(ErrorCodes::INVALID_CONFIG_PARAMETER, - "Static handling rule handler must contain a complete configuration path, for example: " - "config://config_key"); + throw Exception( + ErrorCodes::INVALID_CONFIG_PARAMETER, + "Static handling rule handler must contain a complete configuration path, for example: " + "config://config_key"); const auto & config_path = response_expression.substr(config_prefix.size(), response_expression.size() - config_prefix.size()); writeString(server.config().getRawString(config_path, "Ok.\n"), out); @@ -155,23 +161,23 @@ void StaticRequestHandler::writeResponse(WriteBuffer & out) writeString(response_expression, out); } -StaticRequestHandler::StaticRequestHandler(IServer & server_, const String & expression, int status_, const String & content_type_) - : server(server_), status(status_), content_type(content_type_), response_expression(expression) +StaticRequestHandler::StaticRequestHandler( + IServer & server_, const String & expression, const std::unordered_map & http_response_headers_override_, int status_) + : server(server_), status(status_), http_response_headers_override(http_response_headers_override_), response_expression(expression) { } -HTTPRequestHandlerFactoryPtr createStaticHandlerFactory(IServer & server, - const Poco::Util::AbstractConfiguration & config, - const std::string & config_prefix) +HTTPRequestHandlerFactoryPtr +createStaticHandlerFactory(IServer & server, const Poco::Util::AbstractConfiguration & config, const std::string & config_prefix) { int status = config.getInt(config_prefix + ".handler.status", 200); std::string response_content = config.getRawString(config_prefix + ".handler.response_content", "Ok.\n"); - std::string response_content_type = config.getString(config_prefix + ".handler.content_type", "text/plain; charset=UTF-8"); - auto creator = [&server, response_content, status, response_content_type]() -> std::unique_ptr - { - return std::make_unique(server, response_content, status, response_content_type); - }; + std::unordered_map http_response_headers_override + = parseHTTPResponseHeaders(config, config_prefix, "text/plain; charset=UTF-8"); + + auto creator = [&server, http_response_headers_override, response_content, status]() -> std::unique_ptr + { return std::make_unique(server, response_content, http_response_headers_override, status); }; auto factory = std::make_shared>(std::move(creator)); diff --git a/src/Server/StaticRequestHandler.h b/src/Server/StaticRequestHandler.h index 38d774bb0aa..41fb395d969 100644 --- a/src/Server/StaticRequestHandler.h +++ b/src/Server/StaticRequestHandler.h @@ -1,9 +1,9 @@ #pragma once +#include #include #include - namespace DB { @@ -17,15 +17,16 @@ private: IServer & server; int status; - String content_type; + /// Overrides for response headers. + std::unordered_map http_response_headers_override; String response_expression; public: StaticRequestHandler( IServer & server, const String & expression, - int status_ = 200, - const String & content_type_ = "text/html; charset=UTF-8"); + const std::unordered_map & http_response_headers_override_, + int status_ = 200); void writeResponse(WriteBuffer & out); From 8119054cea7840c421bfeb0b90927ef7e5de9b97 Mon Sep 17 00:00:00 2001 From: Grigorii Sokolik Date: Thu, 6 Jun 2024 11:17:10 +0300 Subject: [PATCH 21/40] Add documentation --- docs/en/interfaces/http.md | 37 ++++++++++++++++++++++++++++++++++++- docs/ru/interfaces/http.md | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/docs/en/interfaces/http.md b/docs/en/interfaces/http.md index eb1a3ba1dbc..f5b6326fa96 100644 --- a/docs/en/interfaces/http.md +++ b/docs/en/interfaces/http.md @@ -508,7 +508,7 @@ Now `rule` can configure `method`, `headers`, `url`, `handler`: - `headers` are responsible for matching the header part of the HTTP request. It is compatible with RE2’s regular expressions. It is an optional configuration. If it is not defined in the configuration file, it does not match the header portion of the HTTP request. -- `handler` contains the main processing part. Now `handler` can configure `type`, `status`, `content_type`, `response_content`, `query`, `query_param_name`. +- `handler` contains the main processing part. Now `handler` can configure `type`, `status`, `content_type`, `http_response_headers`, `response_content`, `query`, `query_param_name`. `type` currently supports three types: [predefined_query_handler](#predefined_query_handler), [dynamic_query_handler](#dynamic_query_handler), [static](#static). - `query` — use with `predefined_query_handler` type, executes query when the handler is called. @@ -519,6 +519,8 @@ Now `rule` can configure `method`, `headers`, `url`, `handler`: - `content_type` — use with any type, response [content-type](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Type). + - `http_response_headers` — use with any type, response headers map. Could be used to set content type as well. + - `response_content` — use with `static` type, response content sent to client, when using the prefix ‘file://’ or ‘config://’, find the content from the file or configuration sends to client. Next are the configuration methods for different `type`. @@ -616,6 +618,33 @@ Return a message. static 402 text/html; charset=UTF-8 + + en + 43 + + Say Hi! + + + + +``` + +`http_response_headers` could be used to set content type instead of `content_type`. + +``` xml + + + GET + xxx + /hi + + static + 402 + + text/html; charset=UTF-8 + en + 43 + Say Hi! @@ -696,6 +725,9 @@ Find the content from the file send to client. static text/html; charset=UTF-8 + + 737060cd8c284d8af7ad3082f209582d + file:///absolute_path_file.html @@ -706,6 +738,9 @@ Find the content from the file send to client. static text/html; charset=UTF-8 + + 737060cd8c284d8af7ad3082f209582d + file://./relative_path_file.html diff --git a/docs/ru/interfaces/http.md b/docs/ru/interfaces/http.md index 5f11f1b430b..d9da51892f9 100644 --- a/docs/ru/interfaces/http.md +++ b/docs/ru/interfaces/http.md @@ -414,6 +414,8 @@ $ curl -v 'http://localhost:8123/predefined_query' - `content_type` — используется со всеми типами, возвращает [content-type](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Type). + - `http_response_headers` — используется со всеми типами чтобы добавить кастомные хедеры в ответ. Может использоваться в том числе для задания хедера `Content-Type` вместо `content_type`. + - `response_content` — используется с типом`static`, содержимое ответа, отправленное клиенту, при использовании префикса ‘file://’ or ‘config://’, находит содержимое из файла или конфигурации, отправленного клиенту. Далее приведены методы настройки для различных типов. @@ -509,6 +511,33 @@ max_final_threads 2 static 402 text/html; charset=UTF-8 + + en + 43 + + Say Hi! + + + + +``` + +`http_response_headers` так же может использоваться для определения `Content-Type` вместо `content_type`. + +``` xml + + + GET + xxx + /hi + + static + 402 + + text/html; charset=UTF-8 + en + 43 + Say Hi! @@ -589,6 +618,9 @@ $ curl -v -H 'XXX:xxx' 'http://localhost:8123/get_config_static_handler' static text/html; charset=UTF-8 + + 737060cd8c284d8af7ad3082f209582d + file:///absolute_path_file.html @@ -599,6 +631,9 @@ $ curl -v -H 'XXX:xxx' 'http://localhost:8123/get_config_static_handler' static text/html; charset=UTF-8 + + 737060cd8c284d8af7ad3082f209582d + file://./relative_path_file.html From 7bff47701b530d1e463ad21e5a895f1e8bc31df9 Mon Sep 17 00:00:00 2001 From: Grigorii Sokolik Date: Thu, 6 Jun 2024 11:51:16 +0300 Subject: [PATCH 22/40] Fix styles --- src/Server/HTTPResponseHeaderWriter.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Server/HTTPResponseHeaderWriter.h b/src/Server/HTTPResponseHeaderWriter.h index 066cf84eca7..06281abb42d 100644 --- a/src/Server/HTTPResponseHeaderWriter.h +++ b/src/Server/HTTPResponseHeaderWriter.h @@ -1,3 +1,5 @@ +#pragma once + #include #include #include From 2aa2f2f7da202cb1ddcd9a8e5a395b3bd3821403 Mon Sep 17 00:00:00 2001 From: Grigorii Sokolik Date: Wed, 12 Jun 2024 22:55:32 +0300 Subject: [PATCH 23/40] Rollback unnecessary style fixes --- src/Server/HTTPHandler.cpp | 470 ++++++++++++++-------------- src/Server/HTTPHandler.h | 42 ++- src/Server/HTTPHandlerFactory.cpp | 90 +++--- src/Server/StaticRequestHandler.cpp | 46 ++- 4 files changed, 331 insertions(+), 317 deletions(-) diff --git a/src/Server/HTTPHandler.cpp b/src/Server/HTTPHandler.cpp index a2af9905c72..f6ca69813ae 100644 --- a/src/Server/HTTPHandler.cpp +++ b/src/Server/HTTPHandler.cpp @@ -1,8 +1,8 @@ #include -#include #include #include +#include #include #include #include @@ -10,7 +10,6 @@ #include #include #include -#include #include #include #include @@ -18,37 +17,38 @@ #include #include #include -#include #include -#include -#include #include -#include +#include +#include #include #include #include -#include "Common/logger_useful.h" +#include #include #include #include #include #include +#include +#include +#include -#include #include #include +#include #include "config.h" #include #include -#include #include #include #include -#include +#include #include #include +#include #include #include @@ -59,7 +59,7 @@ #include #if USE_SSL -# include +#include #endif @@ -68,68 +68,68 @@ namespace DB namespace ErrorCodes { -extern const int BAD_ARGUMENTS; -extern const int LOGICAL_ERROR; -extern const int CANNOT_SCHEDULE_TASK; -extern const int CANNOT_PARSE_TEXT; -extern const int CANNOT_PARSE_ESCAPE_SEQUENCE; -extern const int CANNOT_PARSE_QUOTED_STRING; -extern const int CANNOT_PARSE_DATE; -extern const int CANNOT_PARSE_DATETIME; -extern const int CANNOT_PARSE_NUMBER; -extern const int CANNOT_PARSE_DOMAIN_VALUE_FROM_STRING; -extern const int CANNOT_PARSE_IPV4; -extern const int CANNOT_PARSE_IPV6; -extern const int CANNOT_PARSE_UUID; -extern const int CANNOT_PARSE_INPUT_ASSERTION_FAILED; -extern const int CANNOT_OPEN_FILE; -extern const int CANNOT_COMPILE_REGEXP; -extern const int DUPLICATE_COLUMN; -extern const int ILLEGAL_COLUMN; -extern const int THERE_IS_NO_COLUMN; -extern const int UNKNOWN_ELEMENT_IN_AST; -extern const int UNKNOWN_TYPE_OF_AST_NODE; -extern const int TOO_DEEP_AST; -extern const int TOO_BIG_AST; -extern const int UNEXPECTED_AST_STRUCTURE; -extern const int VALUE_IS_OUT_OF_RANGE_OF_DATA_TYPE; + extern const int BAD_ARGUMENTS; + extern const int LOGICAL_ERROR; + extern const int CANNOT_COMPILE_REGEXP; + extern const int CANNOT_OPEN_FILE; + extern const int CANNOT_PARSE_TEXT; + extern const int CANNOT_PARSE_ESCAPE_SEQUENCE; + extern const int CANNOT_PARSE_QUOTED_STRING; + extern const int CANNOT_PARSE_DATE; + extern const int CANNOT_PARSE_DATETIME; + extern const int CANNOT_PARSE_NUMBER; + extern const int CANNOT_PARSE_DOMAIN_VALUE_FROM_STRING; + extern const int CANNOT_PARSE_IPV4; + extern const int CANNOT_PARSE_IPV6; + extern const int CANNOT_PARSE_UUID; + extern const int CANNOT_PARSE_INPUT_ASSERTION_FAILED; + extern const int CANNOT_SCHEDULE_TASK; + extern const int DUPLICATE_COLUMN; + extern const int ILLEGAL_COLUMN; + extern const int THERE_IS_NO_COLUMN; + extern const int UNKNOWN_ELEMENT_IN_AST; + extern const int UNKNOWN_TYPE_OF_AST_NODE; + extern const int TOO_DEEP_AST; + extern const int TOO_BIG_AST; + extern const int UNEXPECTED_AST_STRUCTURE; + extern const int VALUE_IS_OUT_OF_RANGE_OF_DATA_TYPE; -extern const int SYNTAX_ERROR; + extern const int SYNTAX_ERROR; -extern const int INCORRECT_DATA; -extern const int TYPE_MISMATCH; + extern const int INCORRECT_DATA; + extern const int TYPE_MISMATCH; -extern const int UNKNOWN_TABLE; -extern const int UNKNOWN_FUNCTION; -extern const int UNKNOWN_IDENTIFIER; -extern const int UNKNOWN_TYPE; -extern const int UNKNOWN_STORAGE; -extern const int UNKNOWN_DATABASE; -extern const int UNKNOWN_SETTING; -extern const int UNKNOWN_DIRECTION_OF_SORTING; -extern const int UNKNOWN_AGGREGATE_FUNCTION; -extern const int UNKNOWN_FORMAT; -extern const int UNKNOWN_DATABASE_ENGINE; -extern const int UNKNOWN_TYPE_OF_QUERY; -extern const int UNKNOWN_ROLE; -extern const int NO_ELEMENTS_IN_CONFIG; + extern const int UNKNOWN_TABLE; + extern const int UNKNOWN_FUNCTION; + extern const int UNKNOWN_IDENTIFIER; + extern const int UNKNOWN_TYPE; + extern const int UNKNOWN_STORAGE; + extern const int UNKNOWN_DATABASE; + extern const int UNKNOWN_SETTING; + extern const int UNKNOWN_DIRECTION_OF_SORTING; + extern const int UNKNOWN_AGGREGATE_FUNCTION; + extern const int UNKNOWN_FORMAT; + extern const int UNKNOWN_DATABASE_ENGINE; + extern const int UNKNOWN_TYPE_OF_QUERY; + extern const int UNKNOWN_ROLE; + extern const int NO_ELEMENTS_IN_CONFIG; -extern const int QUERY_IS_TOO_LARGE; + extern const int QUERY_IS_TOO_LARGE; -extern const int NOT_IMPLEMENTED; -extern const int SOCKET_TIMEOUT; + extern const int NOT_IMPLEMENTED; + extern const int SOCKET_TIMEOUT; -extern const int UNKNOWN_USER; -extern const int WRONG_PASSWORD; -extern const int REQUIRED_PASSWORD; -extern const int AUTHENTICATION_FAILED; -extern const int SET_NON_GRANTED_ROLE; + extern const int UNKNOWN_USER; + extern const int WRONG_PASSWORD; + extern const int REQUIRED_PASSWORD; + extern const int AUTHENTICATION_FAILED; + extern const int SET_NON_GRANTED_ROLE; -extern const int INVALID_SESSION_TIMEOUT; -extern const int HTTP_LENGTH_REQUIRED; -extern const int SUPPORT_IS_DISABLED; + extern const int INVALID_SESSION_TIMEOUT; + extern const int HTTP_LENGTH_REQUIRED; + extern const int SUPPORT_IS_DISABLED; -extern const int TIMEOUT_EXCEEDED; + extern const int TIMEOUT_EXCEEDED; } namespace @@ -148,9 +148,9 @@ bool tryAddHTTPOptionHeadersFromConfig(HTTPServerResponse & response, const Poco if (config.getString("http_options_response." + config_key + ".name", "").empty()) LOG_WARNING(getLogger("processOptionsRequest"), "Empty header was found in config. It will not be processed."); else - response.add( - config.getString("http_options_response." + config_key + ".name", ""), - config.getString("http_options_response." + config_key + ".value", "")); + response.add(config.getString("http_options_response." + config_key + ".name", ""), + config.getString("http_options_response." + config_key + ".value", "")); + } } return true; @@ -199,37 +199,54 @@ static Poco::Net::HTTPResponse::HTTPStatus exceptionCodeToHTTPStatus(int excepti { return HTTPResponse::HTTP_UNAUTHORIZED; } - else if ( - exception_code == ErrorCodes::UNKNOWN_USER || exception_code == ErrorCodes::WRONG_PASSWORD - || exception_code == ErrorCodes::AUTHENTICATION_FAILED || exception_code == ErrorCodes::SET_NON_GRANTED_ROLE) + else if (exception_code == ErrorCodes::UNKNOWN_USER || + exception_code == ErrorCodes::WRONG_PASSWORD || + exception_code == ErrorCodes::AUTHENTICATION_FAILED || + exception_code == ErrorCodes::SET_NON_GRANTED_ROLE) { return HTTPResponse::HTTP_FORBIDDEN; } - else if ( - exception_code == ErrorCodes::BAD_ARGUMENTS || exception_code == ErrorCodes::CANNOT_COMPILE_REGEXP - || exception_code == ErrorCodes::CANNOT_PARSE_TEXT || exception_code == ErrorCodes::CANNOT_PARSE_ESCAPE_SEQUENCE - || exception_code == ErrorCodes::CANNOT_PARSE_QUOTED_STRING || exception_code == ErrorCodes::CANNOT_PARSE_DATE - || exception_code == ErrorCodes::CANNOT_PARSE_DATETIME || exception_code == ErrorCodes::CANNOT_PARSE_NUMBER - || exception_code == ErrorCodes::CANNOT_PARSE_DOMAIN_VALUE_FROM_STRING || exception_code == ErrorCodes::CANNOT_PARSE_IPV4 - || exception_code == ErrorCodes::CANNOT_PARSE_IPV6 || exception_code == ErrorCodes::CANNOT_PARSE_INPUT_ASSERTION_FAILED - || exception_code == ErrorCodes::CANNOT_PARSE_UUID || exception_code == ErrorCodes::DUPLICATE_COLUMN - || exception_code == ErrorCodes::ILLEGAL_COLUMN || exception_code == ErrorCodes::UNKNOWN_ELEMENT_IN_AST - || exception_code == ErrorCodes::UNKNOWN_TYPE_OF_AST_NODE || exception_code == ErrorCodes::THERE_IS_NO_COLUMN - || exception_code == ErrorCodes::TOO_DEEP_AST || exception_code == ErrorCodes::TOO_BIG_AST - || exception_code == ErrorCodes::UNEXPECTED_AST_STRUCTURE || exception_code == ErrorCodes::SYNTAX_ERROR - || exception_code == ErrorCodes::INCORRECT_DATA || exception_code == ErrorCodes::TYPE_MISMATCH - || exception_code == ErrorCodes::VALUE_IS_OUT_OF_RANGE_OF_DATA_TYPE) + else if (exception_code == ErrorCodes::BAD_ARGUMENTS || + exception_code == ErrorCodes::CANNOT_COMPILE_REGEXP || + exception_code == ErrorCodes::CANNOT_PARSE_TEXT || + exception_code == ErrorCodes::CANNOT_PARSE_ESCAPE_SEQUENCE || + exception_code == ErrorCodes::CANNOT_PARSE_QUOTED_STRING || + exception_code == ErrorCodes::CANNOT_PARSE_DATE || + exception_code == ErrorCodes::CANNOT_PARSE_DATETIME || + exception_code == ErrorCodes::CANNOT_PARSE_NUMBER || + exception_code == ErrorCodes::CANNOT_PARSE_DOMAIN_VALUE_FROM_STRING || + exception_code == ErrorCodes::CANNOT_PARSE_IPV4 || + exception_code == ErrorCodes::CANNOT_PARSE_IPV6 || + exception_code == ErrorCodes::CANNOT_PARSE_INPUT_ASSERTION_FAILED || + exception_code == ErrorCodes::CANNOT_PARSE_UUID || + exception_code == ErrorCodes::DUPLICATE_COLUMN || + exception_code == ErrorCodes::ILLEGAL_COLUMN || + exception_code == ErrorCodes::UNKNOWN_ELEMENT_IN_AST || + exception_code == ErrorCodes::UNKNOWN_TYPE_OF_AST_NODE || + exception_code == ErrorCodes::THERE_IS_NO_COLUMN || + exception_code == ErrorCodes::TOO_DEEP_AST || + exception_code == ErrorCodes::TOO_BIG_AST || + exception_code == ErrorCodes::UNEXPECTED_AST_STRUCTURE || + exception_code == ErrorCodes::SYNTAX_ERROR || + exception_code == ErrorCodes::INCORRECT_DATA || + exception_code == ErrorCodes::TYPE_MISMATCH || + exception_code == ErrorCodes::VALUE_IS_OUT_OF_RANGE_OF_DATA_TYPE) { return HTTPResponse::HTTP_BAD_REQUEST; } - else if ( - exception_code == ErrorCodes::UNKNOWN_TABLE || exception_code == ErrorCodes::UNKNOWN_FUNCTION - || exception_code == ErrorCodes::UNKNOWN_IDENTIFIER || exception_code == ErrorCodes::UNKNOWN_TYPE - || exception_code == ErrorCodes::UNKNOWN_STORAGE || exception_code == ErrorCodes::UNKNOWN_DATABASE - || exception_code == ErrorCodes::UNKNOWN_SETTING || exception_code == ErrorCodes::UNKNOWN_DIRECTION_OF_SORTING - || exception_code == ErrorCodes::UNKNOWN_AGGREGATE_FUNCTION || exception_code == ErrorCodes::UNKNOWN_FORMAT - || exception_code == ErrorCodes::UNKNOWN_DATABASE_ENGINE || exception_code == ErrorCodes::UNKNOWN_TYPE_OF_QUERY - || exception_code == ErrorCodes::UNKNOWN_ROLE) + else if (exception_code == ErrorCodes::UNKNOWN_TABLE || + exception_code == ErrorCodes::UNKNOWN_FUNCTION || + exception_code == ErrorCodes::UNKNOWN_IDENTIFIER || + exception_code == ErrorCodes::UNKNOWN_TYPE || + exception_code == ErrorCodes::UNKNOWN_STORAGE || + exception_code == ErrorCodes::UNKNOWN_DATABASE || + exception_code == ErrorCodes::UNKNOWN_SETTING || + exception_code == ErrorCodes::UNKNOWN_DIRECTION_OF_SORTING || + exception_code == ErrorCodes::UNKNOWN_AGGREGATE_FUNCTION || + exception_code == ErrorCodes::UNKNOWN_FORMAT || + exception_code == ErrorCodes::UNKNOWN_DATABASE_ENGINE || + exception_code == ErrorCodes::UNKNOWN_TYPE_OF_QUERY || + exception_code == ErrorCodes::UNKNOWN_ROLE) { return HTTPResponse::HTTP_NOT_FOUND; } @@ -241,7 +258,8 @@ static Poco::Net::HTTPResponse::HTTPStatus exceptionCodeToHTTPStatus(int excepti { return HTTPResponse::HTTP_NOT_IMPLEMENTED; } - else if (exception_code == ErrorCodes::SOCKET_TIMEOUT || exception_code == ErrorCodes::CANNOT_OPEN_FILE) + else if (exception_code == ErrorCodes::SOCKET_TIMEOUT || + exception_code == ErrorCodes::CANNOT_OPEN_FILE) { return HTTPResponse::HTTP_SERVICE_UNAVAILABLE; } @@ -262,7 +280,9 @@ static Poco::Net::HTTPResponse::HTTPStatus exceptionCodeToHTTPStatus(int excepti } -static std::chrono::steady_clock::duration parseSessionTimeout(const Poco::Util::AbstractConfiguration & config, const HTMLForm & params) +static std::chrono::steady_clock::duration parseSessionTimeout( + const Poco::Util::AbstractConfiguration & config, + const HTMLForm & params) { unsigned session_timeout = config.getInt("default_session_timeout", 60); @@ -276,17 +296,15 @@ static std::chrono::steady_clock::duration parseSessionTimeout(const Poco::Util: throw Exception(ErrorCodes::INVALID_SESSION_TIMEOUT, "Invalid session timeout: '{}'", session_timeout_str); if (session_timeout > max_session_timeout) - throw Exception( - ErrorCodes::INVALID_SESSION_TIMEOUT, - "Session timeout '{}' is larger than max_session_timeout: {}. " + throw Exception(ErrorCodes::INVALID_SESSION_TIMEOUT, "Session timeout '{}' is larger than max_session_timeout: {}. " "Maximum session timeout could be modified in configuration file.", - session_timeout_str, - max_session_timeout); + session_timeout_str, max_session_timeout); } return std::chrono::seconds(session_timeout); } + void HTTPHandler::pushDelayedResults(Output & used_output) { std::vector write_buffers; @@ -338,7 +356,10 @@ HTTPHandler::HTTPHandler(IServer & server_, const std::string & name, const HTTP HTTPHandler::~HTTPHandler() = default; -bool HTTPHandler::authenticateUser(HTTPServerRequest & request, HTMLForm & params, HTTPServerResponse & response) +bool HTTPHandler::authenticateUser( + HTTPServerRequest & request, + HTMLForm & params, + HTTPServerResponse & response) { using namespace Poco::Net; @@ -365,36 +386,31 @@ bool HTTPHandler::authenticateUser(HTTPServerRequest & request, HTMLForm & param { /// It is prohibited to mix different authorization schemes. if (has_http_credentials) - throw Exception( - ErrorCodes::AUTHENTICATION_FAILED, - "Invalid authentication: it is not allowed " - "to use SSL certificate authentication and Authorization HTTP header simultaneously"); + throw Exception(ErrorCodes::AUTHENTICATION_FAILED, + "Invalid authentication: it is not allowed " + "to use SSL certificate authentication and Authorization HTTP header simultaneously"); if (has_credentials_in_query_params) - throw Exception( - ErrorCodes::AUTHENTICATION_FAILED, - "Invalid authentication: it is not allowed " - "to use SSL certificate authentication and authentication via parameters simultaneously simultaneously"); + throw Exception(ErrorCodes::AUTHENTICATION_FAILED, + "Invalid authentication: it is not allowed " + "to use SSL certificate authentication and authentication via parameters simultaneously simultaneously"); if (has_ssl_certificate_auth) { #if USE_SSL if (!password.empty()) - throw Exception( - ErrorCodes::AUTHENTICATION_FAILED, - "Invalid authentication: it is not allowed " - "to use SSL certificate authentication and authentication via password simultaneously"); + throw Exception(ErrorCodes::AUTHENTICATION_FAILED, + "Invalid authentication: it is not allowed " + "to use SSL certificate authentication and authentication via password simultaneously"); if (request.havePeerCertificate()) certificate_common_name = request.peerCertificate().commonName(); if (certificate_common_name.empty()) - throw Exception( - ErrorCodes::AUTHENTICATION_FAILED, - "Invalid authentication: SSL certificate authentication requires nonempty certificate's Common Name"); + throw Exception(ErrorCodes::AUTHENTICATION_FAILED, + "Invalid authentication: SSL certificate authentication requires nonempty certificate's Common Name"); #else - throw Exception( - ErrorCodes::SUPPORT_IS_DISABLED, - "SSL certificate authentication disabled because ClickHouse was built without SSL library"); + throw Exception(ErrorCodes::SUPPORT_IS_DISABLED, + "SSL certificate authentication disabled because ClickHouse was built without SSL library"); #endif } } @@ -402,10 +418,9 @@ bool HTTPHandler::authenticateUser(HTTPServerRequest & request, HTMLForm & param { /// It is prohibited to mix different authorization schemes. if (has_credentials_in_query_params) - throw Exception( - ErrorCodes::AUTHENTICATION_FAILED, - "Invalid authentication: it is not allowed " - "to use Authorization HTTP header and authentication via parameters simultaneously"); + throw Exception(ErrorCodes::AUTHENTICATION_FAILED, + "Invalid authentication: it is not allowed " + "to use Authorization HTTP header and authentication via parameters simultaneously"); std::string scheme; std::string auth_info; @@ -426,8 +441,7 @@ bool HTTPHandler::authenticateUser(HTTPServerRequest & request, HTMLForm & param } else { - throw Exception( - ErrorCodes::AUTHENTICATION_FAILED, "Invalid authentication: '{}' HTTP Authorization scheme is not supported", scheme); + throw Exception(ErrorCodes::AUTHENTICATION_FAILED, "Invalid authentication: '{}' HTTP Authorization scheme is not supported", scheme); } } else @@ -453,8 +467,7 @@ bool HTTPHandler::authenticateUser(HTTPServerRequest & request, HTMLForm & param auto * gss_acceptor_context = dynamic_cast(request_credentials.get()); if (!gss_acceptor_context) - throw Exception( - ErrorCodes::AUTHENTICATION_FAILED, "Invalid authentication: unexpected 'Negotiate' HTTP Authorization scheme expected"); + throw Exception(ErrorCodes::AUTHENTICATION_FAILED, "Invalid authentication: unexpected 'Negotiate' HTTP Authorization scheme expected"); #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wunreachable-code" @@ -490,10 +503,9 @@ bool HTTPHandler::authenticateUser(HTTPServerRequest & request, HTMLForm & param if (params.has("quota_key")) { if (!quota_key.empty()) - throw Exception( - ErrorCodes::BAD_ARGUMENTS, - "Invalid authentication: it is not allowed " - "to use quota key as HTTP header and as parameter simultaneously"); + throw Exception(ErrorCodes::BAD_ARGUMENTS, + "Invalid authentication: it is not allowed " + "to use quota key as HTTP header and as parameter simultaneously"); quota_key = params.get("quota_key"); } @@ -618,29 +630,26 @@ void HTTPHandler::processQuery( size_t buffer_size_memory = (buffer_size_total > buffer_size_http) ? buffer_size_total : 0; bool enable_http_compression = params.getParsed("enable_http_compression", context->getSettingsRef().enable_http_compression); - Int64 http_zlib_compression_level - = params.getParsed("http_zlib_compression_level", context->getSettingsRef().http_zlib_compression_level); + Int64 http_zlib_compression_level = params.getParsed("http_zlib_compression_level", context->getSettingsRef().http_zlib_compression_level); - used_output.out_holder = std::make_shared( - response, - request.getMethod() == HTTPRequest::HTTP_HEAD, - context->getServerSettings().keep_alive_timeout.totalSeconds(), - write_event); + used_output.out_holder = + std::make_shared( + response, + request.getMethod() == HTTPRequest::HTTP_HEAD, + context->getServerSettings().keep_alive_timeout.totalSeconds(), + write_event); used_output.out = used_output.out_holder; used_output.out_maybe_compressed = used_output.out_holder; if (client_supports_http_compression && enable_http_compression) { used_output.out_holder->setCompressionMethodHeader(http_response_compression_method); - used_output.wrap_compressed_holder = wrapWriteBufferWithCompressionMethod( - used_output.out_holder.get(), - http_response_compression_method, - static_cast(http_zlib_compression_level), - 0, - DBMS_DEFAULT_BUFFER_SIZE, - nullptr, - 0, - false); + used_output.wrap_compressed_holder = + wrapWriteBufferWithCompressionMethod( + used_output.out_holder.get(), + http_response_compression_method, + static_cast(http_zlib_compression_level), + 0, DBMS_DEFAULT_BUFFER_SIZE, nullptr, 0, false); used_output.out = used_output.wrap_compressed_holder; } @@ -664,13 +673,15 @@ void HTTPHandler::processQuery( { auto tmp_data = std::make_shared(server.context()->getTempDataOnDisk()); - auto create_tmp_disk_buffer = [tmp_data](const WriteBufferPtr &) -> WriteBufferPtr { return tmp_data->createRawStream(); }; + auto create_tmp_disk_buffer = [tmp_data] (const WriteBufferPtr &) -> WriteBufferPtr { + return tmp_data->createRawStream(); + }; cascade_buffer2.emplace_back(std::move(create_tmp_disk_buffer)); } else { - auto push_memory_buffer_and_continue = [next_buffer = used_output.out_maybe_compressed](const WriteBufferPtr & prev_buf) + auto push_memory_buffer_and_continue = [next_buffer = used_output.out_maybe_compressed] (const WriteBufferPtr & prev_buf) { auto * prev_memory_buffer = typeid_cast(prev_buf.get()); if (!prev_memory_buffer) @@ -685,8 +696,7 @@ void HTTPHandler::processQuery( cascade_buffer2.emplace_back(push_memory_buffer_and_continue); } - used_output.out_delayed_and_compressed_holder - = std::make_unique(std::move(cascade_buffer1), std::move(cascade_buffer2)); + used_output.out_delayed_and_compressed_holder = std::make_unique(std::move(cascade_buffer1), std::move(cascade_buffer2)); used_output.out_maybe_delayed_and_compressed = used_output.out_delayed_and_compressed_holder.get(); } else @@ -699,8 +709,7 @@ void HTTPHandler::processQuery( int zstd_window_log_max = static_cast(context->getSettingsRef().zstd_window_log_max); auto in_post = wrapReadBufferWithCompressionMethod( wrapReadBufferReference(request.getStream()), - chooseCompressionMethod({}, http_request_compression_method_str), - zstd_window_log_max); + chooseCompressionMethod({}, http_request_compression_method_str), zstd_window_log_max); /// The data can also be compressed using incompatible internal algorithm. This is indicated by /// 'decompress' query parameter. @@ -708,8 +717,7 @@ void HTTPHandler::processQuery( bool is_in_post_compressed = false; if (params.getParsed("decompress", false)) { - in_post_maybe_compressed - = std::make_unique(*in_post, /* allow_different_codecs_ = */ false, /* external_data_ = */ true); + in_post_maybe_compressed = std::make_unique(*in_post, /* allow_different_codecs_ = */ false, /* external_data_ = */ true); is_in_post_compressed = true; } else @@ -717,26 +725,12 @@ void HTTPHandler::processQuery( std::unique_ptr in; - static const NameSet reserved_param_names{ - "compress", - "decompress", - "user", - "password", - "quota_key", - "query_id", - "stacktrace", - "role", - "buffer_size", - "wait_end_of_query", - "session_id", - "session_timeout", - "session_check", - "client_protocol_version", - "close_session"}; + static const NameSet reserved_param_names{"compress", "decompress", "user", "password", "quota_key", "query_id", "stacktrace", "role", + "buffer_size", "wait_end_of_query", "session_id", "session_timeout", "session_check", "client_protocol_version", "close_session"}; Names reserved_param_suffixes; - auto param_could_be_skipped = [&](const String & name) + auto param_could_be_skipped = [&] (const String & name) { /// Empty parameter appears when URL like ?&a=b or a=b&&c=d. Just skip them for user's convenience. if (name.empty()) @@ -746,8 +740,10 @@ void HTTPHandler::processQuery( return true; for (const String & suffix : reserved_param_suffixes) + { if (endsWith(name, suffix)) return true; + } return false; }; @@ -865,34 +861,35 @@ void HTTPHandler::processQuery( if (settings.add_http_cors_header && !request.get("Origin", "").empty() && !config.has("http_options_response")) used_output.out_holder->addHeaderCORS(true); - auto append_callback = [my_context = context](ProgressCallback callback) + auto append_callback = [my_context = context] (ProgressCallback callback) { auto prev = my_context->getProgressCallback(); - my_context->setProgressCallback( - [prev, callback](const Progress & progress) - { - if (prev) - prev(progress); + my_context->setProgressCallback([prev, callback] (const Progress & progress) + { + if (prev) + prev(progress); - callback(progress); - }); + callback(progress); + }); }; /// While still no data has been sent, we will report about query execution progress by sending HTTP headers. /// Note that we add it unconditionally so the progress is available for `X-ClickHouse-Summary` - append_callback([&used_output](const Progress & progress) { used_output.out_holder->onProgress(progress); }); + append_callback([&used_output](const Progress & progress) + { + used_output.out_holder->onProgress(progress); + }); if (settings.readonly > 0 && settings.cancel_http_readonly_queries_on_client_close) { - append_callback( - [&context, &request](const Progress &) - { - /// Assume that at the point this method is called no one is reading data from the socket any more: - /// should be true for read-only queries. - if (!request.checkPeerConnected()) - context->killCurrentQuery(); - }); + append_callback([&context, &request](const Progress &) + { + /// Assume that at the point this method is called no one is reading data from the socket any more: + /// should be true for read-only queries. + if (!request.checkPeerConnected()) + context->killCurrentQuery(); + }); } customizeContext(request, context, *in_post_maybe_compressed); @@ -900,7 +897,7 @@ void HTTPHandler::processQuery( applyHTTPResponseHeaders(response, http_response_headers_override); - auto set_query_result = [this, &response](const QueryResultDetails & details) + auto set_query_result = [&response, this] (const QueryResultDetails & details) { response.add("X-ClickHouse-Query-Id", details.query_id); @@ -915,10 +912,7 @@ void HTTPHandler::processQuery( response.add("X-ClickHouse-Timezone", *details.timezone); }; - auto handle_exception_in_output_format = [&](IOutputFormat & current_output_format, - const String & format_name, - const ContextPtr & context_, - const std::optional & format_settings) + auto handle_exception_in_output_format = [&](IOutputFormat & current_output_format, const String & format_name, const ContextPtr & context_, const std::optional & format_settings) { if (settings.http_write_exception_in_output_format && current_output_format.supportsWritingException()) { @@ -938,8 +932,7 @@ void HTTPHandler::processQuery( } else { - bool with_stacktrace - = (params.getParsed("stacktrace", false) && server.config().getBool("enable_http_stacktrace", true)); + bool with_stacktrace = (params.getParsed("stacktrace", false) && server.config().getBool("enable_http_stacktrace", true)); ExecutionStatus status = ExecutionStatus::fromCurrentException("", with_stacktrace); formatExceptionForClient(status.code, request, response, used_output); current_output_format.setException(status.message); @@ -980,8 +973,7 @@ try if (!used_output.out_holder && !used_output.exception_is_written) { /// If nothing was sent yet and we don't even know if we must compress the response. - WriteBufferFromHTTPServerResponse(response, request.getMethod() == HTTPRequest::HTTP_HEAD, DEFAULT_HTTP_KEEP_ALIVE_TIMEOUT) - .writeln(s); + WriteBufferFromHTTPServerResponse(response, request.getMethod() == HTTPRequest::HTTP_HEAD, DEFAULT_HTTP_KEEP_ALIVE_TIMEOUT).writeln(s); } else if (used_output.out_maybe_compressed) { @@ -1045,8 +1037,7 @@ catch (...) } } -void HTTPHandler::formatExceptionForClient( - int exception_code, HTTPServerRequest & request, HTTPServerResponse & response, Output & used_output) +void HTTPHandler::formatExceptionForClient(int exception_code, HTTPServerRequest & request, HTTPServerResponse & response, Output & used_output) { if (used_output.out_holder) used_output.out_holder->setExceptionCode(exception_code); @@ -1113,14 +1104,18 @@ void HTTPHandler::handleRequest(HTTPServerRequest & request, HTTPServerResponse std::string opentelemetry_traceparent = request.get("traceparent"); std::string error; if (!client_trace_context.parseTraceparentHeader(opentelemetry_traceparent, error)) + { LOG_DEBUG(log, "Failed to parse OpenTelemetry traceparent header '{}': {}", opentelemetry_traceparent, error); + } client_trace_context.tracestate = request.get("tracestate", ""); } // Setup tracing context for this thread auto context = session->sessionOrGlobalContext(); - thread_trace_context = std::make_unique( - "HTTPHandler", client_trace_context, context->getSettingsRef(), context->getOpenTelemetrySpanLog()); + thread_trace_context = std::make_unique("HTTPHandler", + client_trace_context, + context->getSettingsRef(), + context->getOpenTelemetrySpanLog()); thread_trace_context->root_span.kind = OpenTelemetry::SpanKind::SERVER; thread_trace_context->root_span.addAttribute("clickhouse.uri", request.getURI()); @@ -1149,10 +1144,9 @@ void HTTPHandler::handleRequest(HTTPServerRequest & request, HTTPServerResponse /// Workaround. Poco does not detect 411 Length Required case. if (request.getMethod() == HTTPRequest::HTTP_POST && !request.getChunkedTransferEncoding() && !request.hasContentLength()) { - throw Exception( - ErrorCodes::HTTP_LENGTH_REQUIRED, - "The Transfer-Encoding is not chunked and there " - "is no Content-Length header for POST request"); + throw Exception(ErrorCodes::HTTP_LENGTH_REQUIRED, + "The Transfer-Encoding is not chunked and there " + "is no Content-Length header for POST request"); } processQuery(request, params, response, used_output, query_scope, write_event); @@ -1164,8 +1158,7 @@ void HTTPHandler::handleRequest(HTTPServerRequest & request, HTTPServerResponse catch (...) { SCOPE_EXIT({ - request_credentials - .reset(); // ...so that the next requests on the connection have to always start afresh in case of exceptions. + request_credentials.reset(); // ...so that the next requests on the connection have to always start afresh in case of exceptions. }); /// Check if exception was thrown in used_output.finalize(). @@ -1204,7 +1197,7 @@ DynamicQueryHandler::DynamicQueryHandler( bool DynamicQueryHandler::customizeQueryParam(ContextMutablePtr context, const std::string & key, const std::string & value) { if (key == param_name) - return true; /// do nothing + return true; /// do nothing if (startsWith(key, QUERY_PARAMETER_NAME_PREFIX)) { @@ -1238,10 +1231,16 @@ std::string DynamicQueryHandler::getQuery(HTTPServerRequest & request, HTMLForm std::string full_query; /// Params are of both form params POST and uri (GET params) for (const auto & it : params) + { if (it.first == param_name) + { full_query += it.second; + } else + { customizeQueryParam(context, it.first, it.second); + } + } return full_query; } @@ -1339,8 +1338,9 @@ std::string PredefinedQueryHandler::getQuery(HTTPServerRequest & request, HTMLFo return predefined_query; } -HTTPRequestHandlerFactoryPtr -createDynamicHandlerFactory(IServer & server, const Poco::Util::AbstractConfiguration & config, const std::string & config_prefix) +HTTPRequestHandlerFactoryPtr createDynamicHandlerFactory(IServer & server, + const Poco::Util::AbstractConfiguration & config, + const std::string & config_prefix) { auto query_param_name = config.getString(config_prefix + ".handler.query_param_name", "query"); @@ -1357,14 +1357,11 @@ createDynamicHandlerFactory(IServer & server, const Poco::Util::AbstractConfigur static inline bool capturingNamedQueryParam(NameSet receive_params, const CompiledRegexPtr & compiled_regex) { const auto & capturing_names = compiled_regex->NamedCapturingGroups(); - return std::count_if( - capturing_names.begin(), - capturing_names.end(), - [&](const auto & iterator) - { - return std::count_if( - receive_params.begin(), receive_params.end(), [&](const auto & param_name) { return param_name == iterator.first; }); - }); + return std::count_if(capturing_names.begin(), capturing_names.end(), [&](const auto & iterator) + { + return std::count_if(receive_params.begin(), receive_params.end(), + [&](const auto & param_name) { return param_name == iterator.first; }); + }); } static inline CompiledRegexPtr getCompiledRegex(const std::string & expression) @@ -1372,18 +1369,15 @@ static inline CompiledRegexPtr getCompiledRegex(const std::string & expression) auto compiled_regex = std::make_shared(expression); if (!compiled_regex->ok()) - throw Exception( - ErrorCodes::CANNOT_COMPILE_REGEXP, - "Cannot compile re2: {} for http handling rule, error: {}. " - "Look at https://github.com/google/re2/wiki/Syntax for reference.", - expression, - compiled_regex->error()); + throw Exception(ErrorCodes::CANNOT_COMPILE_REGEXP, "Cannot compile re2: {} for http handling rule, error: {}. " + "Look at https://github.com/google/re2/wiki/Syntax for reference.", expression, compiled_regex->error()); return compiled_regex; } -HTTPRequestHandlerFactoryPtr -createPredefinedHandlerFactory(IServer & server, const Poco::Util::AbstractConfiguration & config, const std::string & config_prefix) +HTTPRequestHandlerFactoryPtr createPredefinedHandlerFactory(IServer & server, + const Poco::Util::AbstractConfiguration & config, + const std::string & config_prefix) { if (!config.has(config_prefix + ".handler.query")) throw Exception(ErrorCodes::NO_ELEMENTS_IN_CONFIG, "There is no path '{}.handler.query' in configuration file.", config_prefix); @@ -1422,12 +1416,18 @@ createPredefinedHandlerFactory(IServer & server, const Poco::Util::AbstractConfi auto regex = getCompiledRegex(url_expression); if (capturingNamedQueryParam(analyze_receive_params, regex)) { - auto creator - = [&server, analyze_receive_params, predefined_query, regex, headers_name_with_regex, http_response_headers_override] + auto creator = [ + &server, + analyze_receive_params, + predefined_query, + regex, + headers_name_with_regex, + http_response_headers_override] -> std::unique_ptr { return std::make_unique( - server, analyze_receive_params, predefined_query, regex, headers_name_with_regex, http_response_headers_override); + server, analyze_receive_params, predefined_query, regex, + headers_name_with_regex, http_response_headers_override); }; factory = std::make_shared>(std::move(creator)); factory->addFiltersFromConfig(config, config_prefix); @@ -1435,11 +1435,17 @@ createPredefinedHandlerFactory(IServer & server, const Poco::Util::AbstractConfi } } - auto creator = [&server, analyze_receive_params, predefined_query, headers_name_with_regex, http_response_headers_override] + auto creator = [ + &server, + analyze_receive_params, + predefined_query, + headers_name_with_regex, + http_response_headers_override] -> std::unique_ptr { return std::make_unique( - server, analyze_receive_params, predefined_query, CompiledRegexPtr{}, headers_name_with_regex, http_response_headers_override); + server, analyze_receive_params, predefined_query, CompiledRegexPtr{}, + headers_name_with_regex, http_response_headers_override); }; factory = std::make_shared>(std::move(creator)); diff --git a/src/Server/HTTPHandler.h b/src/Server/HTTPHandler.h index 1bf1dbdebf8..c5551102f7a 100644 --- a/src/Server/HTTPHandler.h +++ b/src/Server/HTTPHandler.h @@ -3,27 +3,24 @@ #include #include #include -#include #include -#include #include #include #include #include #include +#include +#include #include #include "HTTPResponseHeaderWriter.h" namespace CurrentMetrics { -extern const Metric HTTPConnection; + extern const Metric HTTPConnection; } -namespace Poco -{ -class Logger; -} +namespace Poco { class Logger; } namespace DB { @@ -45,7 +42,7 @@ public: void handleRequest(HTTPServerRequest & request, HTTPServerResponse & response, const ProfileEvents::Event & write_event) override; /// This method is called right before the query execution. - virtual void customizeContext(HTTPServerRequest & /* request */, ContextMutablePtr /* context */, ReadBuffer & /* body */) { } + virtual void customizeContext(HTTPServerRequest & /* request */, ContextMutablePtr /* context */, ReadBuffer & /* body */) {} virtual bool customizeQueryParam(ContextMutablePtr context, const std::string & key, const std::string & value) = 0; @@ -85,7 +82,10 @@ private: bool exception_is_written = false; std::function exception_writer; - bool hasDelayed() const { return out_maybe_delayed_and_compressed != out_maybe_compressed.get(); } + bool hasDelayed() const + { + return out_maybe_delayed_and_compressed != out_maybe_compressed.get(); + } void finalize() { @@ -99,7 +99,10 @@ private: out->finalize(); } - bool isFinalized() const { return finalized; } + bool isFinalized() const + { + return finalized; + } }; IServer & server; @@ -130,7 +133,10 @@ private: // Returns false when the user is not authenticated yet, and the 'Negotiate' response is sent, // the session and request_credentials instances are preserved. // Throws an exception if authentication failed. - bool authenticateUser(HTTPServerRequest & request, HTMLForm & params, HTTPServerResponse & response); + bool authenticateUser( + HTTPServerRequest & request, + HTMLForm & params, + HTTPServerResponse & response); /// Also initializes 'used_output'. void processQuery( @@ -142,9 +148,17 @@ private: const ProfileEvents::Event & write_event); void trySendExceptionToClient( - const std::string & s, int exception_code, HTTPServerRequest & request, HTTPServerResponse & response, Output & used_output); + const std::string & s, + int exception_code, + HTTPServerRequest & request, + HTTPServerResponse & response, + Output & used_output); - void formatExceptionForClient(int exception_code, HTTPServerRequest & request, HTTPServerResponse & response, Output & used_output); + void formatExceptionForClient( + int exception_code, + HTTPServerRequest & request, + HTTPServerResponse & response, + Output & used_output); static void pushDelayedResults(Output & used_output); }; @@ -162,7 +176,7 @@ public: std::string getQuery(HTTPServerRequest & request, HTMLForm & params, ContextMutablePtr context) override; - bool customizeQueryParam(ContextMutablePtr context, const std::string & key, const std::string & value) override; + bool customizeQueryParam(ContextMutablePtr context, const std::string &key, const std::string &value) override; }; class PredefinedQueryHandler : public HTTPHandler diff --git a/src/Server/HTTPHandlerFactory.cpp b/src/Server/HTTPHandlerFactory.cpp index d125e08c704..5344b2d024b 100644 --- a/src/Server/HTTPHandlerFactory.cpp +++ b/src/Server/HTTPHandlerFactory.cpp @@ -1,18 +1,18 @@ #include #include -#include #include #include +#include #include #include "HTTPHandler.h" -#include "InterserverIOHTTPHandler.h" -#include "PrometheusRequestHandler.h" -#include "ReplicasStatusHandler.h" #include "Server/PrometheusMetricsWriter.h" #include "StaticRequestHandler.h" +#include "ReplicasStatusHandler.h" +#include "InterserverIOHTTPHandler.h" +#include "PrometheusRequestHandler.h" #include "WebUIRequestHandler.h" @@ -21,9 +21,9 @@ namespace DB namespace ErrorCodes { -extern const int LOGICAL_ERROR; -extern const int UNKNOWN_ELEMENT_IN_CONFIG; -extern const int INVALID_CONFIG_PARAMETER; + extern const int LOGICAL_ERROR; + extern const int UNKNOWN_ELEMENT_IN_CONFIG; + extern const int INVALID_CONFIG_PARAMETER; } namespace @@ -35,7 +35,10 @@ private: std::string url; public: - explicit RedirectRequestHandler(std::string url_) : url(std::move(url_)) { } + explicit RedirectRequestHandler(std::string url_) + : url(std::move(url_)) + { + } void handleRequest(HTTPServerRequest &, HTTPServerResponse & response, const ProfileEvents::Event &) override { @@ -43,8 +46,9 @@ public: } }; -HTTPRequestHandlerFactoryPtr -createRedirectHandlerFactory(const Poco::Util::AbstractConfiguration & config, const std::string & config_prefix) +HTTPRequestHandlerFactoryPtr createRedirectHandlerFactory( + const Poco::Util::AbstractConfiguration & config, + const std::string & config_prefix) { std::string url = config.getString(config_prefix + ".handler.location"); @@ -99,12 +103,8 @@ static inline auto createHandlersFactoryFromConfig( const auto & handler_type = config.getString(prefix + "." + key + ".handler.type", ""); if (handler_type.empty()) - throw Exception( - ErrorCodes::INVALID_CONFIG_PARAMETER, - "Handler type in config is not specified here: " - "{}.{}.handler.type", - prefix, - key); + throw Exception(ErrorCodes::INVALID_CONFIG_PARAMETER, "Handler type in config is not specified here: " + "{}.{}.handler.type", prefix, key); if (handler_type == "static") { @@ -155,27 +155,19 @@ static inline auto createHandlersFactoryFromConfig( main_handler_factory->addHandler(std::move(handler)); } else - throw Exception( - ErrorCodes::INVALID_CONFIG_PARAMETER, - "Unknown handler type '{}' in config here: {}.{}.handler.type", - handler_type, - prefix, - key); + throw Exception(ErrorCodes::INVALID_CONFIG_PARAMETER, "Unknown handler type '{}' in config here: {}.{}.handler.type", + handler_type, prefix, key); } else - throw Exception( - ErrorCodes::UNKNOWN_ELEMENT_IN_CONFIG, - "Unknown element in config: " - "{}.{}, must be 'rule' or 'defaults'", - prefix, - key); + throw Exception(ErrorCodes::UNKNOWN_ELEMENT_IN_CONFIG, "Unknown element in config: " + "{}.{}, must be 'rule' or 'defaults'", prefix, key); } return main_handler_factory; } -static inline HTTPRequestHandlerFactoryPtr createHTTPHandlerFactory( - IServer & server, const Poco::Util::AbstractConfiguration & config, const std::string & name, AsynchronousMetrics & async_metrics) +static inline HTTPRequestHandlerFactoryPtr +createHTTPHandlerFactory(IServer & server, const Poco::Util::AbstractConfiguration & config, const std::string & name, AsynchronousMetrics & async_metrics) { if (config.has("http_handlers")) { @@ -202,8 +194,7 @@ static inline HTTPRequestHandlerFactoryPtr createInterserverHTTPHandlerFactory(I } -HTTPRequestHandlerFactoryPtr createHandlerFactory( - IServer & server, const Poco::Util::AbstractConfiguration & config, AsynchronousMetrics & async_metrics, const std::string & name) +HTTPRequestHandlerFactoryPtr createHandlerFactory(IServer & server, const Poco::Util::AbstractConfiguration & config, AsynchronousMetrics & async_metrics, const std::string & name) { if (name == "HTTPHandler-factory" || name == "HTTPSHandler-factory") return createHTTPHandlerFactory(server, config, name, async_metrics); @@ -276,23 +267,28 @@ void addDefaultHandlersFactory( { addCommonDefaultHandlersFactory(factory, server); - auto dynamic_creator - = [&server]() -> std::unique_ptr { return std::make_unique(server, "query"); }; + auto dynamic_creator = [&server] () -> std::unique_ptr + { + return std::make_unique(server, "query"); + }; auto query_handler = std::make_shared>(std::move(dynamic_creator)); - query_handler->addFilter( - [](const auto & request) + query_handler->addFilter([](const auto & request) { - bool path_matches_get_or_head - = startsWith(request.getURI(), "?") || startsWith(request.getURI(), "/?") || startsWith(request.getURI(), "/query?"); - bool is_get_or_head_request - = request.getMethod() == Poco::Net::HTTPRequest::HTTP_GET || request.getMethod() == Poco::Net::HTTPRequest::HTTP_HEAD; + bool path_matches_get_or_head = startsWith(request.getURI(), "?") + || startsWith(request.getURI(), "/?") + || startsWith(request.getURI(), "/query?"); + bool is_get_or_head_request = request.getMethod() == Poco::Net::HTTPRequest::HTTP_GET + || request.getMethod() == Poco::Net::HTTPRequest::HTTP_HEAD; - bool path_matches_post_or_options = path_matches_get_or_head || request.getURI() == "/" || request.getURI().empty(); - bool is_post_or_options_request - = request.getMethod() == Poco::Net::HTTPRequest::HTTP_POST || request.getMethod() == Poco::Net::HTTPRequest::HTTP_OPTIONS; + bool path_matches_post_or_options = path_matches_get_or_head + || request.getURI() == "/" + || request.getURI().empty(); + bool is_post_or_options_request = request.getMethod() == Poco::Net::HTTPRequest::HTTP_POST + || request.getMethod() == Poco::Net::HTTPRequest::HTTP_OPTIONS; return (path_matches_get_or_head && is_get_or_head_request) || (path_matches_post_or_options && is_post_or_options_request); - }); + } + ); factory.addHandler(query_handler); /// We check that prometheus handler will be served on current (default) port. @@ -300,8 +296,10 @@ void addDefaultHandlersFactory( if (config.has("prometheus") && config.getInt("prometheus.port", 0) == 0) { auto writer = std::make_shared(config, "prometheus", async_metrics); - auto creator = [&server, writer]() -> std::unique_ptr - { return std::make_unique(server, writer); }; + auto creator = [&server, writer] () -> std::unique_ptr + { + return std::make_unique(server, writer); + }; auto prometheus_handler = std::make_shared>(std::move(creator)); prometheus_handler->attachStrictPath(config.getString("prometheus.endpoint", "/metrics")); prometheus_handler->allowGetAndHeadRequest(); diff --git a/src/Server/StaticRequestHandler.cpp b/src/Server/StaticRequestHandler.cpp index e320507fc66..331b7a84857 100644 --- a/src/Server/StaticRequestHandler.cpp +++ b/src/Server/StaticRequestHandler.cpp @@ -7,19 +7,19 @@ #include #include #include -#include #include -#include +#include #include +#include #include -#include #include -#include #include #include +#include #include +#include namespace fs = std::filesystem; @@ -29,16 +29,15 @@ namespace DB namespace ErrorCodes { -extern const int INCORRECT_FILE_NAME; -extern const int HTTP_LENGTH_REQUIRED; -extern const int INVALID_CONFIG_PARAMETER; + extern const int INCORRECT_FILE_NAME; + extern const int HTTP_LENGTH_REQUIRED; + extern const int INVALID_CONFIG_PARAMETER; } static inline std::unique_ptr responseWriteBuffer(HTTPServerRequest & request, HTTPServerResponse & response, UInt64 keep_alive_timeout) { - auto buf = std::unique_ptr( - new WriteBufferFromHTTPServerResponse(response, request.getMethod() == HTTPRequest::HTTP_HEAD, keep_alive_timeout)); + auto buf = std::unique_ptr(new WriteBufferFromHTTPServerResponse(response, request.getMethod() == HTTPRequest::HTTP_HEAD, keep_alive_timeout)); /// The client can pass a HTTP header indicating supported compression method (gzip or deflate). String http_response_compression_methods = request.get("Accept-Encoding", ""); @@ -63,8 +62,8 @@ static inline void trySendExceptionToClient( /// If HTTP method is POST and Keep-Alive is turned on, we should read the whole request body /// to avoid reading part of the current request body in the next request. - if (request.getMethod() == Poco::Net::HTTPRequest::HTTP_POST && response.getKeepAlive() && !request.getStream().eof() - && exception_code != ErrorCodes::HTTP_LENGTH_REQUIRED) + if (request.getMethod() == Poco::Net::HTTPRequest::HTTP_POST + && response.getKeepAlive() && !request.getStream().eof() && exception_code != ErrorCodes::HTTP_LENGTH_REQUIRED) request.getStream().ignore(std::numeric_limits::max()); response.setStatusAndReason(Poco::Net::HTTPResponse::HTTP_INTERNAL_SERVER_ERROR); @@ -89,8 +88,7 @@ static inline void trySendExceptionToClient( } } -void StaticRequestHandler::handleRequest( - HTTPServerRequest & request, HTTPServerResponse & response, const ProfileEvents::Event & /*write_event*/) +void StaticRequestHandler::handleRequest(HTTPServerRequest & request, HTTPServerResponse & response, const ProfileEvents::Event & /*write_event*/) { auto keep_alive_timeout = server.context()->getServerSettings().keep_alive_timeout.totalSeconds(); auto out = responseWriteBuffer(request, response, keep_alive_timeout); @@ -103,12 +101,10 @@ void StaticRequestHandler::handleRequest( response.setChunkedTransferEncoding(true); /// Workaround. Poco does not detect 411 Length Required case. - if (request.getMethod() == Poco::Net::HTTPRequest::HTTP_POST && !request.getChunkedTransferEncoding() - && !request.hasContentLength()) - throw Exception( - ErrorCodes::HTTP_LENGTH_REQUIRED, - "The Transfer-Encoding is not chunked and there " - "is no Content-Length header for POST request"); + if (request.getMethod() == Poco::Net::HTTPRequest::HTTP_POST && !request.getChunkedTransferEncoding() && !request.hasContentLength()) + throw Exception(ErrorCodes::HTTP_LENGTH_REQUIRED, + "The Transfer-Encoding is not chunked and there " + "is no Content-Length header for POST request"); setResponseDefaultHeaders(response, keep_alive_timeout); response.setStatusAndReason(Poco::Net::HTTPResponse::HTTPStatus(status)); @@ -149,10 +145,9 @@ void StaticRequestHandler::writeResponse(WriteBuffer & out) else if (startsWith(response_expression, config_prefix)) { if (response_expression.size() <= config_prefix.size()) - throw Exception( - ErrorCodes::INVALID_CONFIG_PARAMETER, - "Static handling rule handler must contain a complete configuration path, for example: " - "config://config_key"); + throw Exception(ErrorCodes::INVALID_CONFIG_PARAMETER, + "Static handling rule handler must contain a complete configuration path, for example: " + "config://config_key"); const auto & config_path = response_expression.substr(config_prefix.size(), response_expression.size() - config_prefix.size()); writeString(server.config().getRawString(config_path, "Ok.\n"), out); @@ -167,8 +162,9 @@ StaticRequestHandler::StaticRequestHandler( { } -HTTPRequestHandlerFactoryPtr -createStaticHandlerFactory(IServer & server, const Poco::Util::AbstractConfiguration & config, const std::string & config_prefix) +HTTPRequestHandlerFactoryPtr createStaticHandlerFactory(IServer & server, + const Poco::Util::AbstractConfiguration & config, + const std::string & config_prefix) { int status = config.getInt(config_prefix + ".handler.status", 200); std::string response_content = config.getRawString(config_prefix + ".handler.response_content", "Ok.\n"); From 80575fe122a75b4914e9e94fa558e2250c68b456 Mon Sep 17 00:00:00 2001 From: Grigorii Sokolik Date: Thu, 13 Jun 2024 14:15:19 +0300 Subject: [PATCH 24/40] Fix CI failure --- src/Server/HTTPResponseHeaderWriter.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Server/HTTPResponseHeaderWriter.cpp b/src/Server/HTTPResponseHeaderWriter.cpp index f5ab196c4b0..fd29af5bdc7 100644 --- a/src/Server/HTTPResponseHeaderWriter.cpp +++ b/src/Server/HTTPResponseHeaderWriter.cpp @@ -56,13 +56,13 @@ std::unordered_map parseHTTPResponseHeaders(const std::string & void applyHTTPResponseHeaders(Poco::Net::HTTPResponse & response, const HTTPResponseHeaderSetup & setup) { if (setup) - for (auto [header_name, header_value] : *setup) + for (const auto & [header_name, header_value] : *setup) response.set(header_name, header_value); } void applyHTTPResponseHeaders(Poco::Net::HTTPResponse & response, const std::unordered_map & setup) { - for (auto [header_name, header_value] : setup) + for (const auto & [header_name, header_value] : setup) response.set(header_name, header_value); } From 3506ed14f447a3e1ffbbda0b38c097569d2da202 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Mar=C3=ADn?= Date: Thu, 13 Jun 2024 19:56:54 +0200 Subject: [PATCH 25/40] Add validation when creating a user with bcrypt_hash --- src/Access/AuthenticationData.cpp | 14 +++++++++++++- .../0_stateless/03172_bcrypt_validation.reference | 0 .../0_stateless/03172_bcrypt_validation.sql | 2 ++ 3 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 tests/queries/0_stateless/03172_bcrypt_validation.reference create mode 100644 tests/queries/0_stateless/03172_bcrypt_validation.sql diff --git a/src/Access/AuthenticationData.cpp b/src/Access/AuthenticationData.cpp index a32215f3d92..814ee72c74b 100644 --- a/src/Access/AuthenticationData.cpp +++ b/src/Access/AuthenticationData.cpp @@ -31,6 +31,7 @@ namespace DB { namespace ErrorCodes { + extern const int AUTHENTICATION_FAILED; extern const int SUPPORT_IS_DISABLED; extern const int BAD_ARGUMENTS; extern const int LOGICAL_ERROR; @@ -90,8 +91,10 @@ bool AuthenticationData::Util::checkPasswordBcrypt(std::string_view password [[m { #if USE_BCRYPT int ret = bcrypt_checkpw(password.data(), reinterpret_cast(password_bcrypt.data())); + /// Before 24.6 we didn't validate hashes on creation, so it could be that the stored hash is invalid + /// and it could not be decoded by the library if (ret == -1) - throw Exception(ErrorCodes::LOGICAL_ERROR, "BCrypt library failed: bcrypt_checkpw returned {}", ret); + throw Exception(ErrorCodes::AUTHENTICATION_FAILED, "Internal failure decoding Bcrypt hash"); return (ret == 0); #else throw Exception( @@ -230,6 +233,15 @@ void AuthenticationData::setPasswordHashBinary(const Digest & hash) throw Exception(ErrorCodes::BAD_ARGUMENTS, "Password hash for the 'BCRYPT_PASSWORD' authentication type has length {} " "but must be 59 or 60 bytes.", hash.size()); + + auto resized = hash; + resized.resize(64); + + /// Verify that it is a valid hash + int ret = bcrypt_checkpw("", reinterpret_cast(resized.data())); + if (ret == -1) + throw Exception(ErrorCodes::BAD_ARGUMENTS, "Could not decode the provided hash with 'bcrypt_hash'"); + password_hash = hash; password_hash.resize(64); return; diff --git a/tests/queries/0_stateless/03172_bcrypt_validation.reference b/tests/queries/0_stateless/03172_bcrypt_validation.reference new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/queries/0_stateless/03172_bcrypt_validation.sql b/tests/queries/0_stateless/03172_bcrypt_validation.sql new file mode 100644 index 00000000000..2c34a7d60d1 --- /dev/null +++ b/tests/queries/0_stateless/03172_bcrypt_validation.sql @@ -0,0 +1,2 @@ +DROP USER IF EXISTS 03172_user_invalid_bcrypt_hash; +CREATE USER 03172_user_invalid_bcrypt_hash IDENTIFIED WITH bcrypt_hash BY '012345678901234567890123456789012345678901234567890123456789'; -- { serverError BAD_ARGUMENTS } From 39261d09a27e9ecaea1c6c3892f04a187dc730a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Mar=C3=ADn?= Date: Thu, 13 Jun 2024 23:32:25 +0200 Subject: [PATCH 26/40] Fix compilation without libraries --- src/Access/AuthenticationData.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Access/AuthenticationData.cpp b/src/Access/AuthenticationData.cpp index 814ee72c74b..70355fadfbd 100644 --- a/src/Access/AuthenticationData.cpp +++ b/src/Access/AuthenticationData.cpp @@ -237,10 +237,12 @@ void AuthenticationData::setPasswordHashBinary(const Digest & hash) auto resized = hash; resized.resize(64); +#if USE_BCRYPT /// Verify that it is a valid hash int ret = bcrypt_checkpw("", reinterpret_cast(resized.data())); if (ret == -1) throw Exception(ErrorCodes::BAD_ARGUMENTS, "Could not decode the provided hash with 'bcrypt_hash'"); +#endif password_hash = hash; password_hash.resize(64); From a6f05df749e8c6720172db2b4e09899a9b5a0498 Mon Sep 17 00:00:00 2001 From: unashi Date: Wed, 5 Jun 2024 19:46:23 +0800 Subject: [PATCH 27/40] [feature] Add an asynchronous_metric jemalloc.profile.active to show whether sampling is currently active for the calling thread. This is an activation mechanism in addition to prof.active; both must be active for the calling thread to sample. --- .../system-tables/asynchronous_metrics.md | 4 ++ src/Common/AsynchronousMetrics.cpp | 10 ++++ .../__init__.py | 0 .../asynchronous_metrics_update_period_s.xml | 3 ++ .../test.py | 49 +++++++++++++++++++ 5 files changed, 66 insertions(+) create mode 100644 tests/integration/test_asynchronous_metric_jemalloc_profile_active/__init__.py create mode 100644 tests/integration/test_asynchronous_metric_jemalloc_profile_active/configs/asynchronous_metrics_update_period_s.xml create mode 100644 tests/integration/test_asynchronous_metric_jemalloc_profile_active/test.py diff --git a/docs/en/operations/system-tables/asynchronous_metrics.md b/docs/en/operations/system-tables/asynchronous_metrics.md index 81725b97e41..762d187917c 100644 --- a/docs/en/operations/system-tables/asynchronous_metrics.md +++ b/docs/en/operations/system-tables/asynchronous_metrics.md @@ -639,6 +639,10 @@ An internal metric of the low-level memory allocator (jemalloc). See https://jem An internal metric of the low-level memory allocator (jemalloc). See https://jemalloc.net/jemalloc.3.html +### jemalloc.prof.active + +An internal metric of the low-level memory allocator (jemalloc). See https://jemalloc.net/jemalloc.3.html + **See Also** - [Monitoring](../../operations/monitoring.md) — Base concepts of ClickHouse monitoring. diff --git a/src/Common/AsynchronousMetrics.cpp b/src/Common/AsynchronousMetrics.cpp index 4c71b9846c7..d6c8b36f171 100644 --- a/src/Common/AsynchronousMetrics.cpp +++ b/src/Common/AsynchronousMetrics.cpp @@ -415,6 +415,15 @@ Value saveAllArenasMetric(AsynchronousMetricValues & values, fmt::format("jemalloc.arenas.all.{}", metric_name)); } +template +static Value saveJemallocProf(AsynchronousMetricValues & values, + const std::string & metric_name) +{ + return saveJemallocMetricImpl(values, + fmt::format("prof.{}", metric_name), + fmt::format("jemalloc.prof.{}", metric_name)); +} + } #endif @@ -607,6 +616,7 @@ void AsynchronousMetrics::update(TimePoint update_time, bool force_update) saveJemallocMetric(new_values, "background_thread.num_threads"); saveJemallocMetric(new_values, "background_thread.num_runs"); saveJemallocMetric(new_values, "background_thread.run_intervals"); + saveJemallocProf(new_values, "active"); saveAllArenasMetric(new_values, "pactive"); [[maybe_unused]] size_t je_malloc_pdirty = saveAllArenasMetric(new_values, "pdirty"); [[maybe_unused]] size_t je_malloc_pmuzzy = saveAllArenasMetric(new_values, "pmuzzy"); diff --git a/tests/integration/test_asynchronous_metric_jemalloc_profile_active/__init__.py b/tests/integration/test_asynchronous_metric_jemalloc_profile_active/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/integration/test_asynchronous_metric_jemalloc_profile_active/configs/asynchronous_metrics_update_period_s.xml b/tests/integration/test_asynchronous_metric_jemalloc_profile_active/configs/asynchronous_metrics_update_period_s.xml new file mode 100644 index 00000000000..47e88730482 --- /dev/null +++ b/tests/integration/test_asynchronous_metric_jemalloc_profile_active/configs/asynchronous_metrics_update_period_s.xml @@ -0,0 +1,3 @@ + + 1 + diff --git a/tests/integration/test_asynchronous_metric_jemalloc_profile_active/test.py b/tests/integration/test_asynchronous_metric_jemalloc_profile_active/test.py new file mode 100644 index 00000000000..245b1fd3bb9 --- /dev/null +++ b/tests/integration/test_asynchronous_metric_jemalloc_profile_active/test.py @@ -0,0 +1,49 @@ +import time + +import pytest +from helpers.cluster import ClickHouseCluster + +cluster = ClickHouseCluster(__file__) +node1 = cluster.add_instance( + "node1", + main_configs=["configs/asynchronous_metrics_update_period_s.xml"], + env_variables={"MALLOC_CONF": "background_thread:true,prof:true"}, +) + + +@pytest.fixture(scope="module") +def started_cluster(): + try: + cluster.start() + yield cluster + + finally: + cluster.shutdown() + + +# Tests that the system.asynchronous_metric_log table gets populated. +# asynchronous metrics are updated once every 60s by default. To make the test run faster, the setting +# asynchronous_metric_update_period_s is being set to 2s so that the metrics are populated faster and +# are available for querying during the test. +def test_event_time_microseconds_field(started_cluster): + res_t = node1.query("SYSTEM JEMALLOC ENABLE PROFILE") + res_o = node1.query("SELECT * FROM system.asynchronous_metrics WHERE metric ILIKE '%jemalloc.prof.active%' FORMAT Vertical;") + assert ( + res_o== """Row 1: +────── +metric: jemalloc.prof.active +value: 1 +description: An internal metric of the low-level memory allocator (jemalloc). See https://jemalloc.net/jemalloc.3.html +""" + ) + node1.query("SYSTEM JEMALLOC DISABLE PROFILE") + time.sleep(5) + res_t = node1.query("SELECT * FROM system.asynchronous_metrics WHERE metric ILIKE '%jemalloc.prof.active%' FORMAT Vertical;") + assert ( + res_t== """Row 1: +────── +metric: jemalloc.prof.active +value: 0 +description: An internal metric of the low-level memory allocator (jemalloc). See https://jemalloc.net/jemalloc.3.html +""" + ) From 83359af8b75c4d23ff89e71b1f9130027588b735 Mon Sep 17 00:00:00 2001 From: unashi Date: Wed, 5 Jun 2024 20:38:18 +0800 Subject: [PATCH 28/40] [update] rm useless res --- .../test_asynchronous_metric_jemalloc_profile_active/test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_asynchronous_metric_jemalloc_profile_active/test.py b/tests/integration/test_asynchronous_metric_jemalloc_profile_active/test.py index 245b1fd3bb9..fe0ff46cedb 100644 --- a/tests/integration/test_asynchronous_metric_jemalloc_profile_active/test.py +++ b/tests/integration/test_asynchronous_metric_jemalloc_profile_active/test.py @@ -26,7 +26,7 @@ def started_cluster(): # asynchronous_metric_update_period_s is being set to 2s so that the metrics are populated faster and # are available for querying during the test. def test_event_time_microseconds_field(started_cluster): - res_t = node1.query("SYSTEM JEMALLOC ENABLE PROFILE") + node1.query("SYSTEM JEMALLOC ENABLE PROFILE") res_o = node1.query("SELECT * FROM system.asynchronous_metrics WHERE metric ILIKE '%jemalloc.prof.active%' FORMAT Vertical;") assert ( res_o== """Row 1: From 761d8e327c3c27b246aefce15afb7061aeb986e7 Mon Sep 17 00:00:00 2001 From: unashi Date: Thu, 6 Jun 2024 10:20:09 +0800 Subject: [PATCH 29/40] [update] 1. black test.py 2. adjust test.py to cover all situation --- .../test.py | 32 ++++++++++++++++--- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/tests/integration/test_asynchronous_metric_jemalloc_profile_active/test.py b/tests/integration/test_asynchronous_metric_jemalloc_profile_active/test.py index fe0ff46cedb..80165a28c76 100644 --- a/tests/integration/test_asynchronous_metric_jemalloc_profile_active/test.py +++ b/tests/integration/test_asynchronous_metric_jemalloc_profile_active/test.py @@ -26,24 +26,46 @@ def started_cluster(): # asynchronous_metric_update_period_s is being set to 2s so that the metrics are populated faster and # are available for querying during the test. def test_event_time_microseconds_field(started_cluster): - node1.query("SYSTEM JEMALLOC ENABLE PROFILE") - res_o = node1.query("SELECT * FROM system.asynchronous_metrics WHERE metric ILIKE '%jemalloc.prof.active%' FORMAT Vertical;") + # prof:true -> default open + res_o = node1.query( + "SELECT * FROM system.asynchronous_metrics WHERE metric ILIKE '%jemalloc.prof.active%' FORMAT Vertical;" + ) assert ( - res_o== """Row 1: + res_o + == """Row 1: ────── metric: jemalloc.prof.active value: 1 description: An internal metric of the low-level memory allocator (jemalloc). See https://jemalloc.net/jemalloc.3.html """ ) + # disable node1.query("SYSTEM JEMALLOC DISABLE PROFILE") time.sleep(5) - res_t = node1.query("SELECT * FROM system.asynchronous_metrics WHERE metric ILIKE '%jemalloc.prof.active%' FORMAT Vertical;") + res_t = node1.query( + "SELECT * FROM system.asynchronous_metrics WHERE metric ILIKE '%jemalloc.prof.active%' FORMAT Vertical;" + ) assert ( - res_t== """Row 1: + res_t + == """Row 1: ────── metric: jemalloc.prof.active value: 0 description: An internal metric of the low-level memory allocator (jemalloc). See https://jemalloc.net/jemalloc.3.html +""" + ) + # enable + node1.query("SYSTEM JEMALLOC ENABLE PROFILE") + time.sleep(5) + res_f = node1.query( + "SELECT * FROM system.asynchronous_metrics WHERE metric ILIKE '%jemalloc.prof.active%' FORMAT Vertical;" + ) + assert ( + res_f + == """Row 1: +────── +metric: jemalloc.prof.active +value: 1 +description: An internal metric of the low-level memory allocator (jemalloc). See https://jemalloc.net/jemalloc.3.html """ ) From 13bdcc335f03c5e6a40c285029acc646432227e2 Mon Sep 17 00:00:00 2001 From: unashi Date: Thu, 6 Jun 2024 17:44:20 +0800 Subject: [PATCH 30/40] [fix] remove static from function --- src/Common/AsynchronousMetrics.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Common/AsynchronousMetrics.cpp b/src/Common/AsynchronousMetrics.cpp index d6c8b36f171..6309f6079f6 100644 --- a/src/Common/AsynchronousMetrics.cpp +++ b/src/Common/AsynchronousMetrics.cpp @@ -416,7 +416,7 @@ Value saveAllArenasMetric(AsynchronousMetricValues & values, } template -static Value saveJemallocProf(AsynchronousMetricValues & values, +Value saveJemallocProf(AsynchronousMetricValues & values, const std::string & metric_name) { return saveJemallocMetricImpl(values, From 17b03c7df9d23e0a6656a9652e73e6ea5fecf251 Mon Sep 17 00:00:00 2001 From: unashi Date: Sat, 8 Jun 2024 00:41:47 +0800 Subject: [PATCH 31/40] [fix] skip sanitizers --- .../test_asynchronous_metric_jemalloc_profile_active/test.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/integration/test_asynchronous_metric_jemalloc_profile_active/test.py b/tests/integration/test_asynchronous_metric_jemalloc_profile_active/test.py index 80165a28c76..1283becca6e 100644 --- a/tests/integration/test_asynchronous_metric_jemalloc_profile_active/test.py +++ b/tests/integration/test_asynchronous_metric_jemalloc_profile_active/test.py @@ -27,6 +27,9 @@ def started_cluster(): # are available for querying during the test. def test_event_time_microseconds_field(started_cluster): # prof:true -> default open + if node1.is_built_with_sanitizer(): + pytest.skip("Disabled for sanitizers") + res_o = node1.query( "SELECT * FROM system.asynchronous_metrics WHERE metric ILIKE '%jemalloc.prof.active%' FORMAT Vertical;" ) From 3c8f3c1930d126295544d46dd88ad912dad74174 Mon Sep 17 00:00:00 2001 From: unashi Date: Sat, 8 Jun 2024 00:53:44 +0800 Subject: [PATCH 32/40] [fix] fix the name of function in test.py --- .../test.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration/test_asynchronous_metric_jemalloc_profile_active/test.py b/tests/integration/test_asynchronous_metric_jemalloc_profile_active/test.py index 1283becca6e..218b3e2ec6a 100644 --- a/tests/integration/test_asynchronous_metric_jemalloc_profile_active/test.py +++ b/tests/integration/test_asynchronous_metric_jemalloc_profile_active/test.py @@ -21,11 +21,11 @@ def started_cluster(): cluster.shutdown() -# Tests that the system.asynchronous_metric_log table gets populated. + # asynchronous metrics are updated once every 60s by default. To make the test run faster, the setting -# asynchronous_metric_update_period_s is being set to 2s so that the metrics are populated faster and +# asynchronous_metric_update_period_s is being set to 1s so that the metrics are populated faster and # are available for querying during the test. -def test_event_time_microseconds_field(started_cluster): +def test_asynchronous_metric_jemalloc_profile_active(started_cluster): # prof:true -> default open if node1.is_built_with_sanitizer(): pytest.skip("Disabled for sanitizers") From f707c0d1ebfc1cd67ba5c5b76f2c055f4ab23a07 Mon Sep 17 00:00:00 2001 From: unashi Date: Sat, 8 Jun 2024 13:52:13 +0800 Subject: [PATCH 33/40] [black] --- .../test_asynchronous_metric_jemalloc_profile_active/test.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration/test_asynchronous_metric_jemalloc_profile_active/test.py b/tests/integration/test_asynchronous_metric_jemalloc_profile_active/test.py index 218b3e2ec6a..627285a2038 100644 --- a/tests/integration/test_asynchronous_metric_jemalloc_profile_active/test.py +++ b/tests/integration/test_asynchronous_metric_jemalloc_profile_active/test.py @@ -21,7 +21,6 @@ def started_cluster(): cluster.shutdown() - # asynchronous metrics are updated once every 60s by default. To make the test run faster, the setting # asynchronous_metric_update_period_s is being set to 1s so that the metrics are populated faster and # are available for querying during the test. From 06e11752fde54b4d14ac0634b32646496e5039db Mon Sep 17 00:00:00 2001 From: unashi Date: Sat, 8 Jun 2024 18:19:15 +0800 Subject: [PATCH 34/40] [retry test] --- .../test_asynchronous_metric_jemalloc_profile_active/test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_asynchronous_metric_jemalloc_profile_active/test.py b/tests/integration/test_asynchronous_metric_jemalloc_profile_active/test.py index 627285a2038..a8f4ab05888 100644 --- a/tests/integration/test_asynchronous_metric_jemalloc_profile_active/test.py +++ b/tests/integration/test_asynchronous_metric_jemalloc_profile_active/test.py @@ -25,7 +25,7 @@ def started_cluster(): # asynchronous_metric_update_period_s is being set to 1s so that the metrics are populated faster and # are available for querying during the test. def test_asynchronous_metric_jemalloc_profile_active(started_cluster): - # prof:true -> default open + # default open if node1.is_built_with_sanitizer(): pytest.skip("Disabled for sanitizers") From 0b8035a6cd8a8d1aa8a3a6a22e8b0f7db48e0a53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Mar=C3=ADn?= Date: Fri, 14 Jun 2024 13:35:12 +0200 Subject: [PATCH 35/40] Test requires libraries --- tests/queries/0_stateless/03172_bcrypt_validation.sql | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/queries/0_stateless/03172_bcrypt_validation.sql b/tests/queries/0_stateless/03172_bcrypt_validation.sql index 2c34a7d60d1..37dd0c9bb5d 100644 --- a/tests/queries/0_stateless/03172_bcrypt_validation.sql +++ b/tests/queries/0_stateless/03172_bcrypt_validation.sql @@ -1,2 +1,3 @@ +-- Tags: no-fasttest DROP USER IF EXISTS 03172_user_invalid_bcrypt_hash; CREATE USER 03172_user_invalid_bcrypt_hash IDENTIFIED WITH bcrypt_hash BY '012345678901234567890123456789012345678901234567890123456789'; -- { serverError BAD_ARGUMENTS } From aacb85495b826d26c84a17b28c3a479124aaf319 Mon Sep 17 00:00:00 2001 From: Han Fei Date: Fri, 14 Jun 2024 13:58:04 +0200 Subject: [PATCH 36/40] Update tests/integration/test_manipulate_statistics/test.py --- tests/integration/test_manipulate_statistics/test.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration/test_manipulate_statistics/test.py b/tests/integration/test_manipulate_statistics/test.py index 9485b611c01..a602cce63df 100644 --- a/tests/integration/test_manipulate_statistics/test.py +++ b/tests/integration/test_manipulate_statistics/test.py @@ -157,7 +157,6 @@ def test_replicated_table_ddl(started_cluster): ) node2.query("insert into test_stat values(1,2,3), (2,3,4)") - # check_stat_file_on_disk(node2, "test_stat", "all_0_0_0", "a", True) check_stat_file_on_disk(node2, "test_stat", "all_0_0_0", "a", True) check_stat_file_on_disk(node2, "test_stat", "all_0_0_0", "c", True) node1.query( From 36a575e124a3fe940cfcecb565270784b0f89ee4 Mon Sep 17 00:00:00 2001 From: Blargian Date: Fri, 14 Jun 2024 15:55:02 +0200 Subject: [PATCH 37/40] Fix float page --- docs/en/sql-reference/data-types/float.md | 32 +++++++++++++++-------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/docs/en/sql-reference/data-types/float.md b/docs/en/sql-reference/data-types/float.md index 23131d5b4fe..7c232a286ea 100644 --- a/docs/en/sql-reference/data-types/float.md +++ b/docs/en/sql-reference/data-types/float.md @@ -7,33 +7,43 @@ sidebar_label: Float32, Float64 # Float32, Float64 :::note -If you need accurate calculations, in particular if you work with financial or business data requiring a high precision you should consider using Decimal instead. Floats might lead to inaccurate results as illustrated below: +If you need accurate calculations, in particular if you work with financial or business data requiring a high precision, you should consider using [Decimal](../data-types/decimal.md) instead. -``` +[Floating Point Numbers](https://en.wikipedia.org/wiki/IEEE_754) might lead to inaccurate results as illustrated below: + +```sql CREATE TABLE IF NOT EXISTS float_vs_decimal ( my_float Float64, my_decimal Decimal64(3) -)Engine=MergeTree ORDER BY tuple() - -INSERT INTO float_vs_decimal SELECT round(randCanonical(), 3) AS res, res FROM system.numbers LIMIT 1000000; # Generate 1 000 000 random number with 2 decimal places and store them as a float and as a decimal +) +Engine=MergeTree +ORDER BY tuple(); +# Generate 1 000 000 random number with 2 decimal places and store them as a float and as a decimal +INSERT INTO float_vs_decimal SELECT round(randCanonical(), 3) AS res, res FROM system.numbers LIMIT 1000000; +``` +``` SELECT sum(my_float), sum(my_decimal) FROM float_vs_decimal; -> 500279.56300000014 500279.563 + +┌──────sum(my_float)─┬─sum(my_decimal)─┐ +│ 499693.60500000004 │ 499693.605 │ +└────────────────────┴─────────────────┘ SELECT sumKahan(my_float), sumKahan(my_decimal) FROM float_vs_decimal; -> 500279.563 500279.563 + +┌─sumKahan(my_float)─┬─sumKahan(my_decimal)─┐ +│ 499693.605 │ 499693.605 │ +└────────────────────┴──────────────────────┘ ``` ::: -[Floating point numbers](https://en.wikipedia.org/wiki/IEEE_754). - -Types are equivalent to types of C: +The equivalent types in ClickHouse and in C are given below: - `Float32` — `float`. - `Float64` — `double`. -Aliases: +Float types in ClickHouse have the following aliases: - `Float32` — `FLOAT`, `REAL`, `SINGLE`. - `Float64` — `DOUBLE`, `DOUBLE PRECISION`. From ad103b9db8dfeec41862ccb18acfe908b5b0d14b Mon Sep 17 00:00:00 2001 From: Shaun Struwig <41984034+Blargian@users.noreply.github.com> Date: Fri, 14 Jun 2024 16:06:40 +0200 Subject: [PATCH 38/40] Update float.md --- docs/en/sql-reference/data-types/float.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/en/sql-reference/data-types/float.md b/docs/en/sql-reference/data-types/float.md index 7c232a286ea..3c789076c1e 100644 --- a/docs/en/sql-reference/data-types/float.md +++ b/docs/en/sql-reference/data-types/float.md @@ -20,7 +20,7 @@ CREATE TABLE IF NOT EXISTS float_vs_decimal Engine=MergeTree ORDER BY tuple(); -# Generate 1 000 000 random number with 2 decimal places and store them as a float and as a decimal +# Generate 1 000 000 random numbers with 2 decimal places and store them as a float and as a decimal INSERT INTO float_vs_decimal SELECT round(randCanonical(), 3) AS res, res FROM system.numbers LIMIT 1000000; ``` ``` From a3469098e7abaa05d30c7eba58d9be3727c7771f Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Fri, 14 Jun 2024 20:35:59 +0200 Subject: [PATCH 39/40] Fix 01246_buffer_flush flakiness (by tunning timeouts) Signed-off-by: Azat Khuzhin --- tests/queries/0_stateless/01246_buffer_flush.sql | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/queries/0_stateless/01246_buffer_flush.sql b/tests/queries/0_stateless/01246_buffer_flush.sql index 36bcaae383f..66f93371c29 100644 --- a/tests/queries/0_stateless/01246_buffer_flush.sql +++ b/tests/queries/0_stateless/01246_buffer_flush.sql @@ -9,14 +9,14 @@ create table data_01256 as system.numbers Engine=Memory(); select 'min'; create table buffer_01256 as system.numbers Engine=Buffer(currentDatabase(), data_01256, 1, - 2, 100, /* time */ + 5, 100, /* time */ 4, 100, /* rows */ 1, 1e6 /* bytes */ ); insert into buffer_01256 select * from system.numbers limit 5; select count() from data_01256; --- sleep 2 (min time) + 1 (round up) + bias (1) = 4 -select sleepEachRow(2) from numbers(2) FORMAT Null; +-- It is enough to ensure that the buffer will be flushed earlier then 2*min_time (10 sec) +select sleepEachRow(9) FORMAT Null SETTINGS function_sleep_max_microseconds_per_block=10e6; select count() from data_01256; drop table buffer_01256; From b2947d9b4939ec3440b955578fd7e1d2430f2185 Mon Sep 17 00:00:00 2001 From: Kruglov Pavel <48961922+Avogar@users.noreply.github.com> Date: Mon, 17 Jun 2024 13:52:30 +0200 Subject: [PATCH 40/40] Fix crash in 03036_dynamic_read_subcolumns --- src/DataTypes/Serializations/SerializationVariantElement.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/DataTypes/Serializations/SerializationVariantElement.cpp b/src/DataTypes/Serializations/SerializationVariantElement.cpp index 1f9a81ac671..ec0b4019c2f 100644 --- a/src/DataTypes/Serializations/SerializationVariantElement.cpp +++ b/src/DataTypes/Serializations/SerializationVariantElement.cpp @@ -146,7 +146,7 @@ void SerializationVariantElement::deserializeBinaryBulkWithMultipleStreams( } /// If we started to read a new column, reinitialize variant column in deserialization state. - if (!variant_element_state->variant || result_column->empty()) + if (!variant_element_state->variant || mutable_column->empty()) { variant_element_state->variant = mutable_column->cloneEmpty();