From 0992b2c2b6b2800105a6046f895da0aa5104d08b Mon Sep 17 00:00:00 2001 From: Vladimir Cherkasov Date: Tue, 26 Nov 2024 11:38:23 +0000 Subject: [PATCH 01/10] Merge pull request #72392 from ClickHouse/vdimir/sqashing_rata_race Fix data race in Squashing with LowCardinality --- src/Columns/ColumnLowCardinality.h | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/Columns/ColumnLowCardinality.h b/src/Columns/ColumnLowCardinality.h index 0b12b50b97d34..2c4d3b2bb8017 100644 --- a/src/Columns/ColumnLowCardinality.h +++ b/src/Columns/ColumnLowCardinality.h @@ -190,6 +190,26 @@ class ColumnLowCardinality final : public COWHelperforEachSubcolumnRecursively(callback); + + /// Column doesn't own dictionary if it's shared. + if (!dictionary.isShared()) + { + callback(*dictionary.getColumnUniquePtr()); + dictionary.getColumnUniquePtr()->forEachSubcolumnRecursively(callback); + } + } + void forEachSubcolumnRecursively(RecursiveMutableColumnCallback callback) override { callback(*idx.getPositionsPtr()); From 34cf5e95864f2972b6baa00c5298eaa6149fd230 Mon Sep 17 00:00:00 2001 From: Mikhail Koviazin Date: Tue, 8 Sep 2026 18:02:44 +0200 Subject: [PATCH 02/10] docker: fix libssl CVEs for alpine --- docker/server/Dockerfile.alpine | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docker/server/Dockerfile.alpine b/docker/server/Dockerfile.alpine index 0f57d169d0c07..a85950f58383d 100644 --- a/docker/server/Dockerfile.alpine +++ b/docker/server/Dockerfile.alpine @@ -88,6 +88,9 @@ RUN mkdir -p \ && chown root:clickhouse "${DEFAULT_LOG_DIR}" \ && chmod ugo+Xrw -R "${DEFAULT_DATA_DIR}" "${DEFAULT_LOG_DIR}" "${DEFAULT_CLIENT_CONFIG_DIR}" "${DEFAULT_SERVER_CONFIG_DIR}" +RUN apk update +RUN apk add --upgrade libssl3 libcrypto3 + VOLUME "${DEFAULT_DATA_DIR}" EXPOSE 9000 8123 9009 From 5c2baa67cf6ca9363eb9c95626b4f0bca578da70 Mon Sep 17 00:00:00 2001 From: MyroTk <44327070+MyroTk@users.noreply.github.com> Date: Wed, 9 Sep 2026 11:53:12 -0400 Subject: [PATCH 03/10] Bump to 24.8.14.10548 --- cmake/autogenerated_versions.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmake/autogenerated_versions.txt b/cmake/autogenerated_versions.txt index cf67c605638e6..c01f1680ba5cd 100644 --- a/cmake/autogenerated_versions.txt +++ b/cmake/autogenerated_versions.txt @@ -10,10 +10,10 @@ SET(VERSION_GITHASH c8a1e828dcf9832dc2d71adcbd50c698f93bb69b) #10000 for altinitystable candidates #20000 for altinityedge candidates -SET(VERSION_TWEAK 10547) +SET(VERSION_TWEAK 10548) SET(VERSION_FLAVOUR altinitytest) -SET(VERSION_DESCRIBE v24.8.14.10547.altinitytest) -SET(VERSION_STRING 24.8.14.10547.altinitytest) +SET(VERSION_DESCRIBE v24.8.14.10548.altinitytest) +SET(VERSION_STRING 24.8.14.10548.altinitytest) # end of autochange From 30b16f35c150929e6f4c0fb6c570d3f0b89fbc84 Mon Sep 17 00:00:00 2001 From: CarlosFelipeOR Date: Fri, 25 Sep 2026 10:03:30 -0300 Subject: [PATCH 04/10] CI: use the clickhouse/minio-* mirror image minio/minio was deleted from Docker Hub, so every with_minio=True test fails in cluster.start() on the image pull. Point docker_compose_minio.yml at the clickhouse/minio-minio mirror, same tag. Upstream #119999 was not backported to 24.8. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/integration/compose/docker_compose_minio.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/compose/docker_compose_minio.yml b/tests/integration/compose/docker_compose_minio.yml index b709161296d93..de48c6b24f2a7 100644 --- a/tests/integration/compose/docker_compose_minio.yml +++ b/tests/integration/compose/docker_compose_minio.yml @@ -2,7 +2,7 @@ version: '2.3' services: minio1: - image: minio/minio:RELEASE.2024-07-31T05-46-26Z + image: clickhouse/minio-minio:RELEASE.2024-07-31T05-46-26Z volumes: - data1-1:/data1 - ${MINIO_CERTS_DIR:-}:/certs From 14081deb9867a0fcf4f2bc74a957028d300cafa3 Mon Sep 17 00:00:00 2001 From: CarlosFelipeOR Date: Wed, 30 Sep 2026 10:55:03 -0300 Subject: [PATCH 05/10] Do not report builds pruned by the CI cache as missing Signed-off-by: CarlosFelipeOR --- tests/ci/build_report_check.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/ci/build_report_check.py b/tests/ci/build_report_check.py index 29dc809052c26..db4f8835e3c48 100644 --- a/tests/ci/build_report_check.py +++ b/tests/ci/build_report_check.py @@ -70,6 +70,25 @@ def main(): for job in builds_for_check: assert job in CI.BuildNames, "Builds must be known build job names" print("NOTE: builds for check taken from input arguments") + if Path(CI_CONFIG_PATH).is_file(): + # In PRs the CI cache drops builds that no remaining test job needs + # (see CiCache.filter_out_not_affected_jobs), they must not be reported as missing + with open(CI_CONFIG_PATH, encoding="utf-8") as jfd: + ci_config = json.load(jfd) + all_ci_jobs = ( + ci_config["jobs_data"]["jobs_to_skip"] + + ci_config["jobs_data"]["jobs_to_do"] + ) + dropped_builds = [ + job for job in builds_for_check if job not in all_ci_jobs + ] + if dropped_builds: + print( + f"NOTE: builds not in the current CI workflow, skip: [{dropped_builds}]" + ) + builds_for_check = [ + job for job in builds_for_check if job in all_ci_jobs + ] print(f"NOTE: following build reports will be checked: [{builds_for_check}]") From b0329ca78d66204c3c646166f5c1998275945f7a Mon Sep 17 00:00:00 2001 From: "clickhouse-gh[bot]" <173705893+clickhouse-gh[bot]@users.noreply.github.com> Date: Fri, 2 Oct 2026 18:37:52 +0000 Subject: [PATCH 06/10] Merge pull request #123431 from mkmkme/fix/view-analyzer-distributed-26.8 Fix analyzer setting in views over `Distributed` --- src/Interpreters/InterpreterSelectQuery.cpp | 8 +++ ...nalyzer_setting_over_distributed.reference | 16 ++++++ ...view_analyzer_setting_over_distributed.sql | 49 +++++++++++++++++++ 3 files changed, 73 insertions(+) create mode 100644 tests/queries/0_stateless/05315_view_analyzer_setting_over_distributed.reference create mode 100644 tests/queries/0_stateless/05315_view_analyzer_setting_over_distributed.sql diff --git a/src/Interpreters/InterpreterSelectQuery.cpp b/src/Interpreters/InterpreterSelectQuery.cpp index a70c5667a36b7..4a91fae27c82d 100644 --- a/src/Interpreters/InterpreterSelectQuery.cpp +++ b/src/Interpreters/InterpreterSelectQuery.cpp @@ -3200,8 +3200,16 @@ void InterpreterSelectQuery::initSettings() { auto & query = getSelectQuery(); if (query.settings()) + { InterpreterSetQuery(query.settings(), context).executeForCurrentContext(options.ignore_setting_constraints); + /// The old interpreter disabled the analyzer in `IInterpreterUnionOrSelectQuery`, but a `SELECT` + /// stored in a `VIEW` may enable it again here. Storages must not use analyzer-only query tree + /// paths (for example, `StorageDistributed::read`) when this interpreter built the query. + if (context->getSettingsRef().allow_experimental_analyzer) + context->setSetting("allow_experimental_analyzer", false); + } + const auto & client_info = context->getClientInfo(); auto min_major = DBMS_MIN_MAJOR_VERSION_WITH_CURRENT_AGGREGATION_VARIANT_SELECTION_METHOD; auto min_minor = DBMS_MIN_MINOR_VERSION_WITH_CURRENT_AGGREGATION_VARIANT_SELECTION_METHOD; diff --git a/tests/queries/0_stateless/05315_view_analyzer_setting_over_distributed.reference b/tests/queries/0_stateless/05315_view_analyzer_setting_over_distributed.reference new file mode 100644 index 0000000000000..071137aa6b5d3 --- /dev/null +++ b/tests/queries/0_stateless/05315_view_analyzer_setting_over_distributed.reference @@ -0,0 +1,16 @@ +view analyzer 1, read analyzer 0 +2 +s1 2.5 +s2 7 +view analyzer 1, read analyzer 0, remote shard +s1 2.5 +s2 7 +view analyzer 1, read analyzer 1 +s1 2.5 +s2 7 +view analyzer 0, read analyzer 1 +s1 2.5 +s2 7 +view analyzer 0, read analyzer 0 +s1 2.5 +s2 7 diff --git a/tests/queries/0_stateless/05315_view_analyzer_setting_over_distributed.sql b/tests/queries/0_stateless/05315_view_analyzer_setting_over_distributed.sql new file mode 100644 index 0000000000000..a85ac964d1592 --- /dev/null +++ b/tests/queries/0_stateless/05315_view_analyzer_setting_over_distributed.sql @@ -0,0 +1,49 @@ +-- Tags: distributed + +-- The old interpreter must not use the analyzer-only `Distributed` path when a view's +-- stored `SELECT` has `SETTINGS enable_analyzer = 1`. + +DROP VIEW IF EXISTS v_new; +DROP VIEW IF EXISTS v_old; +DROP TABLE IF EXISTS dist; +DROP TABLE IF EXISTS src; + +CREATE TABLE src (trade_date Date, strategy String, pnl Float64, ts DateTime) ENGINE = Memory; +INSERT INTO src VALUES + ('2026-09-17', 's1', 1.5, '2026-09-17 10:00:00'), + ('2026-09-17', 's1', 2.5, '2026-09-17 11:00:00'), + ('2026-09-17', 's2', 7, '2026-09-17 09:00:00'), + ('2026-09-16', 's3', 9, '2026-09-16 10:00:00'); + +CREATE TABLE dist AS src ENGINE = Distributed(test_shard_localhost, currentDatabase(), src); + +CREATE VIEW v_new AS + SELECT strategy, argMax(pnl, ts) AS pnl + FROM dist WHERE trade_date = '2026-09-17' + GROUP BY strategy SETTINGS enable_analyzer = 1; + +CREATE VIEW v_old AS + SELECT strategy, argMax(pnl, ts) AS pnl + FROM dist WHERE trade_date = '2026-09-17' + GROUP BY strategy SETTINGS enable_analyzer = 0; + +SELECT 'view analyzer 1, read analyzer 0'; +SELECT count() FROM v_new SETTINGS enable_analyzer = 0; +SELECT strategy, pnl FROM v_new ORDER BY strategy SETTINGS enable_analyzer = 0; + +SELECT 'view analyzer 1, read analyzer 0, remote shard'; +SELECT strategy, pnl FROM v_new ORDER BY strategy SETTINGS enable_analyzer = 0, prefer_localhost_replica = 0; + +SELECT 'view analyzer 1, read analyzer 1'; +SELECT strategy, pnl FROM v_new ORDER BY strategy SETTINGS enable_analyzer = 1; + +SELECT 'view analyzer 0, read analyzer 1'; +SELECT strategy, pnl FROM v_old ORDER BY strategy SETTINGS enable_analyzer = 1; + +SELECT 'view analyzer 0, read analyzer 0'; +SELECT strategy, pnl FROM v_old ORDER BY strategy SETTINGS enable_analyzer = 0; + +DROP VIEW v_new; +DROP VIEW v_old; +DROP TABLE dist; +DROP TABLE src; From 3f9853c910c0127684b358ef7924da133f2459ba Mon Sep 17 00:00:00 2001 From: "clickhouse-gh[bot]" <173705893+clickhouse-gh[bot]@users.noreply.github.com> Date: Sat, 3 Oct 2026 12:09:45 +0000 Subject: [PATCH 07/10] Merge pull request #123436 from mkmkme/fix/row-policy-alias-distributed-26.8 Fix `ALIAS` row policies on legacy distributed reads --- src/Interpreters/InterpreterSelectQuery.cpp | 25 ++--- ..._policy_alias_column_distributed.reference | 38 ++++++++ ...16_row_policy_alias_column_distributed.sql | 92 +++++++++++++++++++ 3 files changed, 143 insertions(+), 12 deletions(-) create mode 100644 tests/queries/0_stateless/05316_row_policy_alias_column_distributed.reference create mode 100644 tests/queries/0_stateless/05316_row_policy_alias_column_distributed.sql diff --git a/src/Interpreters/InterpreterSelectQuery.cpp b/src/Interpreters/InterpreterSelectQuery.cpp index a70c5667a36b7..53ed2dd4d8a84 100644 --- a/src/Interpreters/InterpreterSelectQuery.cpp +++ b/src/Interpreters/InterpreterSelectQuery.cpp @@ -151,10 +151,13 @@ FilterDAGInfoPtr generateFilterActions( select_ast->setExpression(ASTSelectQuery::Expression::SELECT, std::make_shared()); auto expr_list = select_ast->select(); - /// The first column is our filter expression. - /// the row_policy_filter_expression should be cloned, because it may be changed by TreeRewriter. - /// which make it possible an invalid expression, although it may be valid in whole select. - expr_list->children.push_back(row_policy_filter_expression->clone()); + /// The first column is our filter expression. Clone it because `TreeRewriter` can change the AST. + auto filter_expression = row_policy_filter_expression->clone(); + + /// `TreeRewriter` expands table aliases only on the initiator, but these filters are also + /// created on shards. An `ALIAS` used by a row policy must be evaluated before the read filter. + replaceAliasColumnsInQuery(filter_expression, metadata_snapshot->getColumns(), {}, context); + expr_list->children.push_back(std::move(filter_expression)); /// Keep columns that are required after the filter actions. for (const auto & column_str : prerequisite_columns) @@ -2127,7 +2130,7 @@ void InterpreterSelectQuery::addPrewhereAliasActions() } /// Set of all (including ALIAS) required columns for PREWHERE - auto get_prewhere_columns = [&]() + auto get_prewhere_columns = [&](bool include_row_level_filter) { NameSet columns; @@ -2138,7 +2141,7 @@ void InterpreterSelectQuery::addPrewhereAliasActions() columns.insert(prewhere_required_columns.begin(), prewhere_required_columns.end()); } - if (row_level_filter) + if (row_level_filter && include_row_level_filter) { auto row_level_required_columns = row_level_filter->actions.getRequiredColumns().getNames(); columns.insert(row_level_required_columns.begin(), row_level_required_columns.end()); @@ -2156,14 +2159,14 @@ void InterpreterSelectQuery::addPrewhereAliasActions() /// before any other executions. if (alias_columns_required) { - NameSet required_columns_from_prewhere = get_prewhere_columns(); + /// The row-level filter runs before `PREWHERE`, but its inputs are not produced by + /// `PREWHERE`. Keep them in the alias actions for queries that still need them. + NameSet required_columns_from_prewhere = get_prewhere_columns(/*include_row_level_filter=*/ false); NameSet required_aliases_from_prewhere; /// Set of ALIAS required columns for PREWHERE /// Expression, that contains all raw required columns ASTPtr required_columns_all_expr = std::make_shared(); - /// Expression, that contains raw required columns for PREWHERE - ASTPtr required_columns_from_prewhere_expr = std::make_shared(); /// Sort out already known required columns between expressions, /// also populate `required_aliases_from_prewhere`. @@ -2189,8 +2192,6 @@ void InterpreterSelectQuery::addPrewhereAliasActions() if (required_columns_from_prewhere.contains(column)) { - required_columns_from_prewhere_expr->children.emplace_back(std::move(column_expr)); - if (is_alias) required_aliases_from_prewhere.insert(column); } @@ -2258,7 +2259,7 @@ void InterpreterSelectQuery::addPrewhereAliasActions() const auto & supported_prewhere_columns = storage->supportedPrewhereColumns(); if (supported_prewhere_columns.has_value()) { - NameSet required_columns_from_prewhere = get_prewhere_columns(); + NameSet required_columns_from_prewhere = get_prewhere_columns(/*include_row_level_filter=*/ true); for (const auto & column_name : required_columns_from_prewhere) { diff --git a/tests/queries/0_stateless/05316_row_policy_alias_column_distributed.reference b/tests/queries/0_stateless/05316_row_policy_alias_column_distributed.reference new file mode 100644 index 0000000000000..565151239d363 --- /dev/null +++ b/tests/queries/0_stateless/05316_row_policy_alias_column_distributed.reference @@ -0,0 +1,38 @@ +baseline local +1 +2 +3 +4 +baseline distributed +1 +2 +3 +4 +policy local +2 +policy distributed local replica +2 +policy distributed remote replica +2 +policy distributed with PREWHERE +2 +policy local without alias optimization +2 +policy local with aliases selected +2 20 ok20 +policy local with aliases selected and PREWHERE +2 20 ok20 +policy distributed with aliases selected +2 20 ok20 +policy distributed with aliases selected and PREWHERE +2 20 ok20 +policy distributed new analyzer +2 +policy with only an ALIAS predicate +1 2 +policy with inline expression +2 +additional filter on ALIAS +2 +3 +4 diff --git a/tests/queries/0_stateless/05316_row_policy_alias_column_distributed.sql b/tests/queries/0_stateless/05316_row_policy_alias_column_distributed.sql new file mode 100644 index 0000000000000..91e7d1360b03f --- /dev/null +++ b/tests/queries/0_stateless/05316_row_policy_alias_column_distributed.sql @@ -0,0 +1,92 @@ +-- Tags: distributed + +SET enable_analyzer = 0; + +DROP ROW POLICY IF EXISTS rp ON t; +DROP TABLE IF EXISTS t_dist; +DROP TABLE IF EXISTS t; + +CREATE TABLE t +( + k UInt64, + team String, + a UInt64 ALIAS k * 10, + a2 String ALIAS concat(team, toString(a)) +) ENGINE = MergeTree ORDER BY k; + +INSERT INTO t VALUES (1, 'ok'), (2, 'ok'), (3, 'ok'), (4, 'no'); + +CREATE TABLE t_dist AS t +ENGINE = Distributed('test_shard_localhost', currentDatabase(), 't', rand()); + +SELECT 'baseline local'; +SELECT k FROM t ORDER BY k; + +SELECT 'baseline distributed'; +SELECT k FROM t_dist ORDER BY k SETTINGS prefer_localhost_replica = 0; + +-- 1 fails the `ALIAS` condition; 3 fails the nested `ALIAS` condition; +-- 4 passes both `ALIAS` conditions but fails the physical team condition. +CREATE ROW POLICY rp ON t FOR SELECT +USING team = 'ok' AND a >= 20 AND a2 != 'ok30' TO ALL; + +SELECT 'policy local'; +SELECT k FROM t ORDER BY k; + +SELECT 'policy distributed local replica'; +SELECT k FROM t_dist ORDER BY k SETTINGS prefer_localhost_replica = 1; + +SELECT 'policy distributed remote replica'; +SELECT k FROM t_dist ORDER BY k SETTINGS prefer_localhost_replica = 0; + +SELECT 'policy distributed with PREWHERE'; +SELECT k FROM t_dist PREWHERE k >= 1 WHERE k <= 4 ORDER BY k +SETTINGS prefer_localhost_replica = 0; + +SELECT 'policy local without alias optimization'; +SELECT k FROM t ORDER BY k SETTINGS optimize_respect_aliases = 0; + +SELECT 'policy local with aliases selected'; +SELECT k, a, a2 FROM t ORDER BY k SETTINGS optimize_respect_aliases = 0; + +-- `PREWHERE` uses team, while both the policy and the selected aliases need k. +-- The alias step must preserve k even when a separate `PREWHERE` step exists. +SELECT 'policy local with aliases selected and PREWHERE'; +SELECT k, a, a2 FROM t PREWHERE team != 'no' ORDER BY k +SETTINGS optimize_respect_aliases = 0; + +SELECT 'policy distributed with aliases selected'; +SELECT k, a, a2 FROM t_dist ORDER BY k SETTINGS prefer_localhost_replica = 0; + +SELECT 'policy distributed with aliases selected and PREWHERE'; +SELECT k, a, a2 FROM t_dist PREWHERE team != 'no' ORDER BY k +SETTINGS prefer_localhost_replica = 0, optimize_respect_aliases = 0; + +SELECT 'policy distributed new analyzer'; +SELECT k FROM t_dist ORDER BY k +SETTINGS enable_analyzer = 1, prefer_localhost_replica = 0; + +-- A policy whose only predicate references an `ALIAS` still needs its value +-- before the row-level filter, even when the `SELECT` only needs `count`. +CREATE ROW POLICY OR REPLACE rp ON t FOR SELECT +USING a2 = 'ok20' TO ALL; + +SELECT 'policy with only an ALIAS predicate'; +SELECT count(), sum(k) FROM t_dist SETTINGS prefer_localhost_replica = 0; + +-- The equivalent expression is a control for the alias substitution. +CREATE ROW POLICY OR REPLACE rp ON t FOR SELECT +USING team = 'ok' AND k * 10 >= 20 AND concat(team, toString(k * 10)) != 'ok30' TO ALL; + +SELECT 'policy with inline expression'; +SELECT k FROM t_dist ORDER BY k SETTINGS prefer_localhost_replica = 0; + +DROP ROW POLICY rp ON t; + +-- Additional table filters use the same filter-action construction path. +SELECT 'additional filter on ALIAS'; +SELECT k FROM t ORDER BY k +SETTINGS optimize_respect_aliases = 0, additional_table_filters = {'t': 'a >= 20'}; + +DROP TABLE t_dist; +DROP TABLE t; From bcee9ffff72fe57427299297f8b572b8c770f716 Mon Sep 17 00:00:00 2001 From: MyroTk <44327070+MyroTk@users.noreply.github.com> Date: Thu, 8 Oct 2026 14:50:57 -0400 Subject: [PATCH 08/10] Update Dockerfile --- docker/keeper/Dockerfile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docker/keeper/Dockerfile b/docker/keeper/Dockerfile index d683602d0221b..49b68fd43bece 100644 --- a/docker/keeper/Dockerfile +++ b/docker/keeper/Dockerfile @@ -79,6 +79,9 @@ RUN mkdir -p "${DEFAULT_DATA_DIR}" "${DEFAULT_LOG_DIR}" "${DEFAULT_CONFIG_DIR}" && chown root:clickhouse "${DEFAULT_LOG_DIR}" \ && chmod ugo+Xrw -R "${DEFAULT_DATA_DIR}" "${DEFAULT_LOG_DIR}" "${DEFAULT_CONFIG_DIR}" +RUN apk update +RUN apk add --upgrade libssl3 libcrypto3 + # /var/lib/clickhouse is necessary due to the current default configuration for Keeper VOLUME "${DEFAULT_DATA_DIR}" /var/lib/clickhouse EXPOSE 2181 10181 44444 9181 From e1e2d755d322118d34b22033d40bd9e92c08ecd6 Mon Sep 17 00:00:00 2001 From: MyroTk Date: Fri, 9 Oct 2026 10:47:10 -0400 Subject: [PATCH 09/10] update all apk packages --- docker/keeper/Dockerfile | 5 ++--- docker/server/Dockerfile.alpine | 3 +-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/docker/keeper/Dockerfile b/docker/keeper/Dockerfile index 49b68fd43bece..72d7a70b185df 100644 --- a/docker/keeper/Dockerfile +++ b/docker/keeper/Dockerfile @@ -12,7 +12,7 @@ RUN arch=${TARGETARCH:-amd64} \ && ln -s "${rarch}-linux-gnu" /lib/linux-gnu -FROM alpine:3.21.7 +FROM alpine ENV LANG=en_US.UTF-8 \ LANGUAGE=en_US:en \ @@ -79,8 +79,7 @@ RUN mkdir -p "${DEFAULT_DATA_DIR}" "${DEFAULT_LOG_DIR}" "${DEFAULT_CONFIG_DIR}" && chown root:clickhouse "${DEFAULT_LOG_DIR}" \ && chmod ugo+Xrw -R "${DEFAULT_DATA_DIR}" "${DEFAULT_LOG_DIR}" "${DEFAULT_CONFIG_DIR}" -RUN apk update -RUN apk add --upgrade libssl3 libcrypto3 +RUN apk update && apk upgrade --no-cache # /var/lib/clickhouse is necessary due to the current default configuration for Keeper VOLUME "${DEFAULT_DATA_DIR}" /var/lib/clickhouse diff --git a/docker/server/Dockerfile.alpine b/docker/server/Dockerfile.alpine index a85950f58383d..7f0b4d953b19d 100644 --- a/docker/server/Dockerfile.alpine +++ b/docker/server/Dockerfile.alpine @@ -88,8 +88,7 @@ RUN mkdir -p \ && chown root:clickhouse "${DEFAULT_LOG_DIR}" \ && chmod ugo+Xrw -R "${DEFAULT_DATA_DIR}" "${DEFAULT_LOG_DIR}" "${DEFAULT_CLIENT_CONFIG_DIR}" "${DEFAULT_SERVER_CONFIG_DIR}" -RUN apk update -RUN apk add --upgrade libssl3 libcrypto3 +RUN apk update && apk upgrade --no-cache VOLUME "${DEFAULT_DATA_DIR}" EXPOSE 9000 8123 9009 From bf22ca69046f3b39a8ce08c4ecc5df8fd840dc61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Mar=C3=ADn?= Date: Thu, 25 Sep 2025 11:30:38 +0000 Subject: [PATCH 10/10] Merge pull request #83096 from Algunenano/convertToSubcolumns A bunch of micro-optimizations to speed up small queries Signed-off-by: Ilya Golshtein --- src/Processors/Executors/ExecutingGraph.cpp | 26 +++--- src/Processors/Executors/ExecutingGraph.h | 6 +- src/Processors/Executors/ExecutorTasks.h | 2 +- src/Processors/Executors/PipelineExecutor.h | 5 +- src/Storages/MergeTree/IMergeTreeReader.cpp | 92 ++++++++++++++------- src/Storages/MergeTree/IMergeTreeReader.h | 6 +- 6 files changed, 84 insertions(+), 53 deletions(-) diff --git a/src/Processors/Executors/ExecutingGraph.cpp b/src/Processors/Executors/ExecutingGraph.cpp index 6d5b60d815911..174108fc9776a 100644 --- a/src/Processors/Executors/ExecutingGraph.cpp +++ b/src/Processors/Executors/ExecutingGraph.cpp @@ -96,7 +96,7 @@ bool ExecutingGraph::addEdges(uint64_t node) return was_edge_added; } -bool ExecutingGraph::expandPipeline(std::stack & stack, uint64_t pid) +bool ExecutingGraph::expandPipeline(boost::container::devector & stack, uint64_t pid) { auto & cur_node = *nodes[pid]; Processors new_processors; @@ -174,7 +174,7 @@ bool ExecutingGraph::expandPipeline(std::stack & stack, uint64_t pid) if (node.status == ExecutingGraph::ExecStatus::Idle) { node.status = ExecutingGraph::ExecStatus::Preparing; - stack.push(updated_node); + stack.push_front(updated_node); } } @@ -215,9 +215,9 @@ void ExecutingGraph::initializeExecution(Queue & queue) bool ExecutingGraph::updateNode(uint64_t pid, Queue & queue, Queue & async_queue) { - std::stack updated_edges; - std::stack updated_processors; - updated_processors.push(pid); + boost::container::devector updated_edges; + boost::container::devector updated_processors; + updated_processors.push_back(pid); std::shared_lock read_lock(nodes_mutex); @@ -227,8 +227,8 @@ bool ExecutingGraph::updateNode(uint64_t pid, Queue & queue, Queue & async_queue if (updated_processors.empty()) { - auto * edge = updated_edges.top(); - updated_edges.pop(); + auto * edge = updated_edges.front(); + updated_edges.pop_front(); /// Here we have ownership on edge, but node can be concurrently accessed. @@ -248,7 +248,7 @@ bool ExecutingGraph::updateNode(uint64_t pid, Queue & queue, Queue & async_queue if (status == ExecutingGraph::ExecStatus::Idle) { node.status = ExecutingGraph::ExecStatus::Preparing; - updated_processors.push(edge->to); + updated_processors.push_front(edge->to); stack_top_lock = std::move(lock); } else @@ -258,8 +258,8 @@ bool ExecutingGraph::updateNode(uint64_t pid, Queue & queue, Queue & async_queue if (!updated_processors.empty()) { - pid = updated_processors.top(); - updated_processors.pop(); + pid = updated_processors.front(); + updated_processors.pop_front(); /// In this method we have ownership on node. auto & node = *nodes[pid]; @@ -364,14 +364,14 @@ bool ExecutingGraph::updateNode(uint64_t pid, Queue & queue, Queue & async_queue for (auto it = node.post_updated_output_ports.rbegin(); it != node.post_updated_output_ports.rend(); ++it) { auto * edge = static_cast(*it); - updated_edges.push(edge); + updated_edges.push_front(edge); edge->update_info.trigger(); } for (auto it = node.post_updated_input_ports.rbegin(); it != node.post_updated_input_ports.rend(); ++it) { auto * edge = static_cast(*it); - updated_edges.push(edge); + updated_edges.push_front(edge); edge->update_info.trigger(); } @@ -392,7 +392,7 @@ bool ExecutingGraph::updateNode(uint64_t pid, Queue & queue, Queue & async_queue read_lock.lock(); /// Add itself back to be prepared again. - updated_processors.push(pid); + updated_processors.push_front(pid); } } } diff --git a/src/Processors/Executors/ExecutingGraph.h b/src/Processors/Executors/ExecutingGraph.h index 71dcd360a2c03..e30e28549be45 100644 --- a/src/Processors/Executors/ExecutingGraph.h +++ b/src/Processors/Executors/ExecutingGraph.h @@ -6,9 +6,9 @@ #include #include #include -#include #include +#include namespace DB { @@ -120,7 +120,7 @@ class ExecutingGraph /// This queue can grow a lot and lead to OOM. That is why we use non-default /// allocator for container which throws exceptions in operator new - using DequeWithMemoryTracker = std::deque>; + using DequeWithMemoryTracker = boost::container::devector>; using Queue = std::queue; using NodePtr = std::unique_ptr; @@ -155,7 +155,7 @@ class ExecutingGraph /// Update graph after processor (pid) returned ExpandPipeline status. /// All new nodes and nodes with updated ports are pushed into stack. - bool expandPipeline(std::stack & stack, uint64_t pid); + bool expandPipeline(boost::container::devector & stack, uint64_t pid); std::shared_ptr processors; std::vector source_processors; diff --git a/src/Processors/Executors/ExecutorTasks.h b/src/Processors/Executors/ExecutorTasks.h index b2201873edf6a..c0426c8c97e38 100644 --- a/src/Processors/Executors/ExecutorTasks.h +++ b/src/Processors/Executors/ExecutorTasks.h @@ -49,7 +49,7 @@ class ExecutorTasks using Stack = std::stack; /// This queue can grow a lot and lead to OOM. That is why we use non-default /// allocator for container which throws exceptions in operator new - using DequeWithMemoryTracker = std::deque>; + using DequeWithMemoryTracker = boost::container::devector>; using Queue = std::queue; void finish(); diff --git a/src/Processors/Executors/PipelineExecutor.h b/src/Processors/Executors/PipelineExecutor.h index ae119355cb5a8..f54315e6ac5d2 100644 --- a/src/Processors/Executors/PipelineExecutor.h +++ b/src/Processors/Executors/PipelineExecutor.h @@ -7,10 +7,11 @@ #include #include -#include #include #include +#include + namespace DB { @@ -93,7 +94,7 @@ class PipelineExecutor /// This queue can grow a lot and lead to OOM. That is why we use non-default /// allocator for container which throws exceptions in operator new - using DequeWithMemoryTracker = std::deque>; + using DequeWithMemoryTracker = boost::container::devector>; using Queue = std::queue; void initializeExecution(size_t num_threads, bool concurrency_control); /// Initialize executor contexts and task_queue. diff --git a/src/Storages/MergeTree/IMergeTreeReader.cpp b/src/Storages/MergeTree/IMergeTreeReader.cpp index 5916f50005d52..4b2b9d25eeb51 100644 --- a/src/Storages/MergeTree/IMergeTreeReader.cpp +++ b/src/Storages/MergeTree/IMergeTreeReader.cpp @@ -45,18 +45,14 @@ IMergeTreeReader::IMergeTreeReader( , storage_snapshot(storage_snapshot_) , all_mark_ranges(all_mark_ranges_) , alter_conversions(data_part_info_for_read->getAlterConversions()) - /// For wide parts convert plain arrays of Nested to subcolumns - /// to allow to use shared offset column from cache. , original_requested_columns(columns_) - , requested_columns(data_part_info_for_read->isWidePart() - ? Nested::convertToSubcolumns(columns_) - : columns_) + , converted_requested_columns(Nested::convertToSubcolumns(columns_)) , virtual_fields(virtual_fields_) { - columns_to_read.reserve(requested_columns.size()); - serializations.reserve(requested_columns.size()); + columns_to_read.reserve(getColumns().size()); + serializations.reserve(getColumns().size()); - for (const auto & column : requested_columns) + for (const auto & column : getColumns()) { columns_to_read.emplace_back(getColumnInPart(column)); serializations.emplace_back(getSerializationInPart(column)); @@ -77,7 +73,7 @@ const IMergeTreeReader::ValueSizeMap & IMergeTreeReader::getAvgValueSizeHints() void IMergeTreeReader::fillVirtualColumns(Columns & columns, size_t rows) const { - chassert(columns.size() == requested_columns.size()); + chassert(columns.size() == getColumns().size()); const auto * loaded_part_info = typeid_cast(data_part_info_for_read.get()); if (!loaded_part_info) @@ -87,7 +83,7 @@ void IMergeTreeReader::fillVirtualColumns(Columns & columns, size_t rows) const const auto & storage_columns = storage_snapshot->metadata->getColumns(); const auto & virtual_columns = storage_snapshot->virtual_columns; - auto it = requested_columns.begin(); + auto it = getColumns().begin(); for (size_t pos = 0; pos < columns.size(); ++pos, ++it) { if (columns[pos] || storage_columns.has(it->name)) @@ -121,15 +117,38 @@ void IMergeTreeReader::fillMissingColumns(Columns & res_columns, bool & should_e { try { - NamesAndTypesList available_columns(columns_to_read.begin(), columns_to_read.end()); - DB::fillMissingColumns( - res_columns, num_rows, - Nested::convertToSubcolumns(requested_columns), - Nested::convertToSubcolumns(available_columns), - partially_read_columns, storage_snapshot->metadata); - - should_evaluate_missing_defaults = std::any_of( - res_columns.begin(), res_columns.end(), [](const auto & column) { return column == nullptr; }); + /// Do a quick check to see if doing any work is necessary to begin with + size_t num_columns_to_read = res_columns.size(); + bool missing = false; + auto requested_column = converted_requested_columns.begin(); + for (size_t i = 0; i < num_columns_to_read; i++, requested_column++) + { + if (!res_columns[i] || partially_read_columns.contains(requested_column->name)) + { + missing = true; + break; + } + } + + if (!missing) + { + should_evaluate_missing_defaults = false; + } + else + { + NamesAndTypesList available_columns(columns_to_read.begin(), columns_to_read.end()); + + DB::fillMissingColumns( + res_columns, + num_rows, + converted_requested_columns, + Nested::convertToSubcolumns(available_columns), + partially_read_columns, + storage_snapshot->metadata); + + should_evaluate_missing_defaults + = std::any_of(res_columns.begin(), res_columns.end(), [](const auto & column) { return column == nullptr; }); + } } catch (Exception & e) { @@ -284,7 +303,8 @@ void IMergeTreeReader::performRequiredConversions(Columns & res_columns) const { try { - size_t num_columns = requested_columns.size(); + const NamesAndTypesList & required_columns = getColumns(); + size_t num_columns = required_columns.size(); if (res_columns.size() != num_columns) { @@ -293,23 +313,29 @@ void IMergeTreeReader::performRequiredConversions(Columns & res_columns) const "Expected {}, got {}", num_columns, res_columns.size()); } - Block copy_block; - auto name_and_type = requested_columns.begin(); + const auto & column_defaults = storage_snapshot->metadata->getColumns().getDefaults(); + /// Keep columns used by DEFAULT expressions when converting Nullable to non-Nullable. + /// Otherwise, check types manually to avoid unnecessary copies. + Block copy_block; + auto name_and_type = getColumns().begin(); for (size_t pos = 0; pos < num_columns; ++pos, ++name_and_type) { if (res_columns[pos] == nullptr) continue; - - copy_block.insert({res_columns[pos], getColumnInPart(*name_and_type).type, name_and_type->name}); + auto column_in_part = getColumnInPart(*name_and_type); + if (column_defaults.empty() && column_in_part.type->equals(*name_and_type->type)) + continue; + copy_block.insert({res_columns[pos], column_in_part.type, name_and_type->name}); } - DB::performRequiredConversions(copy_block, requested_columns, - data_part_info_for_read->getContext(), - storage_snapshot->metadata->getColumns().getDefaults()); + if (!copy_block.columns()) + return; + + DB::performRequiredConversions(copy_block, getColumns(), data_part_info_for_read->getContext(), column_defaults); /// Move columns from block. - name_and_type = requested_columns.begin(); + name_and_type = getColumns().begin(); for (size_t pos = 0; pos < num_columns; ++pos, ++name_and_type) if (copy_block.has(name_and_type->name)) res_columns[pos] = std::move(copy_block.getByName(name_and_type->name).column); @@ -385,9 +411,13 @@ IMergeTreeReader::findColumnForOffsets(const NameAndTypePair & required_column) void IMergeTreeReader::checkNumberOfColumns(size_t num_columns_to_read) const { - if (num_columns_to_read != requested_columns.size()) - throw Exception(ErrorCodes::LOGICAL_ERROR, "invalid number of columns passed to MergeTreeReader::readRows. " - "Expected {}, got {}", requested_columns.size(), num_columns_to_read); + if (num_columns_to_read != getColumns().size()) + throw Exception( + ErrorCodes::LOGICAL_ERROR, + "invalid number of columns passed to MergeTreeReader::readRows. " + "Expected {}, got {}", + converted_requested_columns.size(), + num_columns_to_read); } String IMergeTreeReader::getMessageForDiagnosticOfBrokenPart(size_t from_mark, size_t max_rows_to_read) const diff --git a/src/Storages/MergeTree/IMergeTreeReader.h b/src/Storages/MergeTree/IMergeTreeReader.h index 4be4c109a1afa..c5b039e7b0afd 100644 --- a/src/Storages/MergeTree/IMergeTreeReader.h +++ b/src/Storages/MergeTree/IMergeTreeReader.h @@ -56,8 +56,8 @@ class IMergeTreeReader : private boost::noncopyable /// then try to perform conversions of columns. void performRequiredConversions(Columns & res_columns) const; - const NamesAndTypesList & getColumns() const { return requested_columns; } - size_t numColumnsInResult() const { return requested_columns.size(); } + ALWAYS_INLINE const NamesAndTypesList & getColumns() const { return data_part_info_for_read->isWidePart() ? converted_requested_columns : original_requested_columns; } + size_t numColumnsInResult() const { return getColumns().size(); } size_t getFirstMarkToRead() const { return all_mark_ranges.front().begin; } @@ -126,7 +126,7 @@ class IMergeTreeReader : private boost::noncopyable NamesAndTypesList original_requested_columns; /// The same as above but with converted Arrays to subcolumns of Nested. - NamesAndTypesList requested_columns; + NamesAndTypesList converted_requested_columns; /// Fields of virtual columns that were filled in previous stages. VirtualFields virtual_fields;