From 1f636379d8dcb92449cd2038146651c64d2f9937 Mon Sep 17 00:00:00 2001 From: JackyWoo Date: Tue, 18 Jul 2023 11:25:32 +0800 Subject: [PATCH 001/530] Add aggregators_of_group_by_keys pass to new analyzer --- .../AggregateFunctionOfGroupByKeysPass.cpp | 188 ++++++++ .../AggregateFunctionOfGroupByKeysPass.h | 28 ++ src/Analyzer/QueryTreePassManager.cpp | 4 + ...egate_functions_of_group_by_keys.reference | 441 ++++++++++++++++++ ...r_aggregate_functions_of_group_by_keys.sql | 29 ++ 5 files changed, 690 insertions(+) create mode 100644 src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp create mode 100644 src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.h create mode 100644 tests/queries/0_stateless/02815_analyzer_aggregate_functions_of_group_by_keys.reference create mode 100644 tests/queries/0_stateless/02815_analyzer_aggregate_functions_of_group_by_keys.sql diff --git a/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp b/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp new file mode 100644 index 00000000000..714d6138dc9 --- /dev/null +++ b/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp @@ -0,0 +1,188 @@ +#include "AggregateFunctionOfGroupByKeysPass.h" + +#include + +#include +#include +#include +#include +#include +#include + +namespace DB +{ + +namespace +{ + + /// Check whether we should keep aggregator. + class KeepEliminateFunctionVisitor : public ConstInDepthQueryTreeVisitor + { + public: + using Base = ConstInDepthQueryTreeVisitor; + using Base::Base; + + KeepEliminateFunctionVisitor(const QueryTreeNodes & group_by_keys_, bool & keep_aggregator_) + : group_by_keys(group_by_keys_), keep_aggregator(keep_aggregator_) + { + } + + static bool needChildVisit(VisitQueryTreeNodeType & parent, VisitQueryTreeNodeType & child [[maybe_unused]]) + { + return parent->as(); + } + + void visitFunction(const FunctionNode * function_node) + { + if (function_node->getArguments().getNodes().empty()) + { + keep_aggregator = true; + return; + } + auto it = std::find_if( + group_by_keys.begin(), + group_by_keys.end(), + [function_node](const QueryTreeNodePtr & group_by_ele) { return function_node->isEqual(*group_by_ele); }); + + if (it == group_by_keys.end()) + { + KeepEliminateFunctionVisitor visitor(group_by_keys, keep_aggregator); + visitor.visit(function_node->getArgumentsNode()); + } + } + + void visitColumn(const ColumnNode * column) + { + /// if variable of a function is not in GROUP BY keys, this function should not be deleted + auto it = std::find_if( + group_by_keys.begin(), + group_by_keys.end(), + [column](const QueryTreeNodePtr & group_by_ele) { return column->isEqual(*group_by_ele); }); + + if (it == group_by_keys.end()) + keep_aggregator = true; + } + + void visitImpl(const QueryTreeNodePtr & node) + { + if (keep_aggregator) + return; + + if (node->as()) + return; + + if (auto * function_node = node->as()) + { + visitFunction(function_node); + } + else if (auto * column = node->as()) + { + visitColumn(column); + } + } + + private : + const QueryTreeNodes & group_by_keys; + bool & keep_aggregator; + }; + + /// Try to eliminate min/max/any/anyLast which will not decent into subqueries. + class EliminateFunctionVisitor : public InDepthQueryTreeVisitor + { + public: + using Base = InDepthQueryTreeVisitor; + using Base::Base; + + EliminateFunctionVisitor(const QueryTreeNodes & group_by_keys_) : group_by_keys(group_by_keys_) { } + + void visitImpl(QueryTreeNodePtr & node) + { + + /// Check if function is min/max/any/anyLast + auto * function_node = node->as(); + if (!function_node + || !(function_node->getFunctionName() == "min" || function_node->getFunctionName() == "max" + || function_node->getFunctionName() == "any" || function_node->getFunctionName() == "anyLast")) + return; + + if (!function_node->getArguments().getNodes().empty()) + { + bool keep_aggregator = false; + + KeepEliminateFunctionVisitor visitor(group_by_keys, keep_aggregator); + visitor.visit(function_node->getArgumentsNode()); + + /// Place argument of an aggregate function instead of function + if (!keep_aggregator) + node = function_node->getArguments().getNodes()[0]; + } + } + + static bool needChildVisit(VisitQueryTreeNodeType & parent [[maybe_unused]], VisitQueryTreeNodeType & child) + { + /// Don't descent into table functions and subqueries and special case for ArrayJoin. + return !child->as() && !child->as() && !child->as(); + } + + private: + const QueryTreeNodes & group_by_keys; + }; + +} + +/// Collect QueryNode and its group by keys. +class CollectQueryAndGroupByKeysVisitor : public InDepthQueryTreeVisitor +{ +public: + using Base = InDepthQueryTreeVisitor; + using Base::Base; + + using Data = std::unordered_map; + Data data; + + CollectQueryAndGroupByKeysVisitor() = default; + + void visitImpl(QueryTreeNodePtr & node) + { + auto * query_node = node->as(); + if (!query_node) + return; + + if (!query_node->hasGroupBy()) + return; + + if (query_node->isGroupByWithTotals() || query_node->isGroupByWithCube() || query_node->isGroupByWithRollup()) + return; + + QueryTreeNodes group_by_keys; + for (auto & group_key : query_node->getGroupBy().getNodes()) + { + /// for grouping sets case + if (auto * list = group_key->as()) + { + for (auto & group_elem : list->getNodes()) + group_by_keys.push_back(group_elem); + } + else + { + group_by_keys.push_back(group_key); + } + } + data.insert({node, std::move(group_by_keys)}); + } +}; + +void AggregateFunctionOfGroupByKeysPass::run(QueryTreeNodePtr query_tree_node, ContextPtr /*context*/) +{ + CollectQueryAndGroupByKeysVisitor collector; + collector.visit(query_tree_node); + + for (auto it = collector.data.begin(); it != collector.data.end(); it++) + { + EliminateFunctionVisitor eliminator(it->second); + auto query_node = it->first; + eliminator.visit(query_node); + } +} + +}; diff --git a/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.h b/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.h new file mode 100644 index 00000000000..d0bd545cbbb --- /dev/null +++ b/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.h @@ -0,0 +1,28 @@ +#pragma once + +#include + +namespace DB +{ + +/** Eliminates min/max/any/anyLast aggregators of GROUP BY keys in SELECT section. + * + * Example: SELECT max(column) FROM table GROUP BY column; + * Result: SELECT column FROM table GROUP BY column; + */ +class AggregateFunctionOfGroupByKeysPass final : public IQueryTreePass +{ +public: + String getName() override { return "AggregateFunctionOfGroupByKeys"; } + + String getDescription() override + { + return "Eliminates min/max/any/anyLast aggregators of GROUP BY keys in SELECT section."; + } + + void run(QueryTreeNodePtr query_tree_node, ContextPtr context) override; + +}; + +} + diff --git a/src/Analyzer/QueryTreePassManager.cpp b/src/Analyzer/QueryTreePassManager.cpp index a6da2a66615..7d0a041e22c 100644 --- a/src/Analyzer/QueryTreePassManager.cpp +++ b/src/Analyzer/QueryTreePassManager.cpp @@ -42,6 +42,7 @@ #include #include #include +#include namespace DB { @@ -251,6 +252,9 @@ void addQueryTreePasses(QueryTreePassManager & manager) manager.addPass(std::make_unique()); manager.addPass(std::make_unique()); + /// should before AggregateFunctionsArithmericOperationsPass + manager.addPass(std::make_unique()); + manager.addPass(std::make_unique()); manager.addPass(std::make_unique()); manager.addPass(std::make_unique()); diff --git a/tests/queries/0_stateless/02815_analyzer_aggregate_functions_of_group_by_keys.reference b/tests/queries/0_stateless/02815_analyzer_aggregate_functions_of_group_by_keys.reference new file mode 100644 index 00000000000..2164bfa5b19 --- /dev/null +++ b/tests/queries/0_stateless/02815_analyzer_aggregate_functions_of_group_by_keys.reference @@ -0,0 +1,441 @@ +set optimize_aggregators_of_group_by_keys = 1 +0 0 +0 1 +0 2 +1 0 +1 1 +1 2 +0 0 +0 1 +0 2 +1 0 +1 1 +1 2 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +1 +2 +2 +3 +3 +4 +4 +4 +5 +6 +6 +6 +8 +8 +9 +10 +12 +12 +12 +15 +16 +18 +20 +24 +0 +0 +QUERY id: 0 + PROJECTION COLUMNS + a UInt8 + b UInt8 + PROJECTION + LIST id: 1, nodes: 2 + FUNCTION id: 2, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 3, nodes: 2 + COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 + CONSTANT id: 6, constant_value: UInt64_2, constant_value_type: UInt8 + FUNCTION id: 7, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 8, nodes: 2 + COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 + CONSTANT id: 9, constant_value: UInt64_3, constant_value_type: UInt8 + JOIN TREE + TABLE_FUNCTION id: 5, table_function_name: numbers + ARGUMENTS + LIST id: 10, nodes: 1 + CONSTANT id: 11, constant_value: UInt64_10000000, constant_value_type: UInt32 + GROUP BY + LIST id: 12, nodes: 2 + FUNCTION id: 13, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 14, nodes: 2 + COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 + CONSTANT id: 15, constant_value: UInt64_2, constant_value_type: UInt8 + FUNCTION id: 16, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 17, nodes: 2 + COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 + CONSTANT id: 18, constant_value: UInt64_3, constant_value_type: UInt8 + ORDER BY + LIST id: 19, nodes: 2 + SORT id: 20, sort_direction: ASCENDING, with_fill: 0 + EXPRESSION + FUNCTION id: 2, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 3, nodes: 2 + COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 + CONSTANT id: 6, constant_value: UInt64_2, constant_value_type: UInt8 + SORT id: 21, sort_direction: ASCENDING, with_fill: 0 + EXPRESSION + FUNCTION id: 7, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 8, nodes: 2 + COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 + CONSTANT id: 9, constant_value: UInt64_3, constant_value_type: UInt8 +QUERY id: 0 + PROJECTION COLUMNS + a UInt8 + b UInt8 + PROJECTION + LIST id: 1, nodes: 2 + FUNCTION id: 2, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 3, nodes: 2 + COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 + CONSTANT id: 6, constant_value: UInt64_2, constant_value_type: UInt8 + FUNCTION id: 7, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 8, nodes: 2 + COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 + CONSTANT id: 9, constant_value: UInt64_3, constant_value_type: UInt8 + JOIN TREE + TABLE_FUNCTION id: 5, table_function_name: numbers + ARGUMENTS + LIST id: 10, nodes: 1 + CONSTANT id: 11, constant_value: UInt64_10000000, constant_value_type: UInt32 + GROUP BY + LIST id: 12, nodes: 2 + FUNCTION id: 13, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 14, nodes: 2 + COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 + CONSTANT id: 15, constant_value: UInt64_2, constant_value_type: UInt8 + FUNCTION id: 16, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 17, nodes: 2 + COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 + CONSTANT id: 18, constant_value: UInt64_3, constant_value_type: UInt8 + ORDER BY + LIST id: 19, nodes: 2 + SORT id: 20, sort_direction: ASCENDING, with_fill: 0 + EXPRESSION + FUNCTION id: 2, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 3, nodes: 2 + COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 + CONSTANT id: 6, constant_value: UInt64_2, constant_value_type: UInt8 + SORT id: 21, sort_direction: ASCENDING, with_fill: 0 + EXPRESSION + FUNCTION id: 7, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 8, nodes: 2 + COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 + CONSTANT id: 9, constant_value: UInt64_3, constant_value_type: UInt8 +QUERY id: 0 + PROJECTION COLUMNS + a UInt16 + PROJECTION + LIST id: 1, nodes: 1 + FUNCTION id: 2, function_name: multiply, function_type: ordinary, result_type: UInt16 + ARGUMENTS + LIST id: 3, nodes: 2 + FUNCTION id: 4, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 5, nodes: 2 + COLUMN id: 6, column_name: number, result_type: UInt64, source_id: 7 + CONSTANT id: 8, constant_value: UInt64_5, constant_value_type: UInt8 + FUNCTION id: 9, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 10, nodes: 2 + COLUMN id: 6, column_name: number, result_type: UInt64, source_id: 7 + CONSTANT id: 11, constant_value: UInt64_7, constant_value_type: UInt8 + JOIN TREE + TABLE_FUNCTION id: 7, table_function_name: numbers + ARGUMENTS + LIST id: 12, nodes: 1 + CONSTANT id: 13, constant_value: UInt64_10000000, constant_value_type: UInt32 + GROUP BY + LIST id: 14, nodes: 2 + FUNCTION id: 15, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 16, nodes: 2 + COLUMN id: 6, column_name: number, result_type: UInt64, source_id: 7 + CONSTANT id: 17, constant_value: UInt64_7, constant_value_type: UInt8 + FUNCTION id: 18, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 19, nodes: 2 + COLUMN id: 6, column_name: number, result_type: UInt64, source_id: 7 + CONSTANT id: 20, constant_value: UInt64_5, constant_value_type: UInt8 + ORDER BY + LIST id: 21, nodes: 1 + SORT id: 22, sort_direction: ASCENDING, with_fill: 0 + EXPRESSION + FUNCTION id: 2, function_name: multiply, function_type: ordinary, result_type: UInt16 + ARGUMENTS + LIST id: 3, nodes: 2 + FUNCTION id: 4, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 5, nodes: 2 + COLUMN id: 6, column_name: number, result_type: UInt64, source_id: 7 + CONSTANT id: 8, constant_value: UInt64_5, constant_value_type: UInt8 + FUNCTION id: 9, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 10, nodes: 2 + COLUMN id: 6, column_name: number, result_type: UInt64, source_id: 7 + CONSTANT id: 11, constant_value: UInt64_7, constant_value_type: UInt8 +QUERY id: 0 + PROJECTION COLUMNS + foo UInt64 + PROJECTION + LIST id: 1, nodes: 1 + COLUMN id: 2, column_name: foo, result_type: UInt64, source_id: 3 + JOIN TREE + QUERY id: 3, is_subquery: 1 + PROJECTION COLUMNS + foo UInt64 + PROJECTION + LIST id: 4, nodes: 1 + COLUMN id: 5, column_name: number, result_type: UInt64, source_id: 6 + JOIN TREE + TABLE_FUNCTION id: 6, table_function_name: numbers + ARGUMENTS + LIST id: 7, nodes: 1 + CONSTANT id: 8, constant_value: UInt64_1, constant_value_type: UInt8 + GROUP BY + LIST id: 9, nodes: 1 + COLUMN id: 5, column_name: number, result_type: UInt64, source_id: 6 +set optimize_aggregators_of_group_by_keys = 0 +0 0 +0 1 +0 2 +1 0 +1 1 +1 2 +0 0 +0 1 +0 2 +1 0 +1 1 +1 2 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +0 +1 +2 +2 +3 +3 +4 +4 +4 +5 +6 +6 +6 +8 +8 +9 +10 +12 +12 +12 +15 +16 +18 +20 +24 +0 +QUERY id: 0 + PROJECTION COLUMNS + a UInt8 + b UInt8 + PROJECTION + LIST id: 1, nodes: 2 + FUNCTION id: 2, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 3, nodes: 2 + COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 + CONSTANT id: 6, constant_value: UInt64_2, constant_value_type: UInt8 + FUNCTION id: 7, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 8, nodes: 2 + COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 + CONSTANT id: 9, constant_value: UInt64_3, constant_value_type: UInt8 + JOIN TREE + TABLE_FUNCTION id: 5, table_function_name: numbers + ARGUMENTS + LIST id: 10, nodes: 1 + CONSTANT id: 11, constant_value: UInt64_10000000, constant_value_type: UInt32 + GROUP BY + LIST id: 12, nodes: 2 + FUNCTION id: 13, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 14, nodes: 2 + COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 + CONSTANT id: 15, constant_value: UInt64_2, constant_value_type: UInt8 + FUNCTION id: 16, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 17, nodes: 2 + COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 + CONSTANT id: 18, constant_value: UInt64_3, constant_value_type: UInt8 + ORDER BY + LIST id: 19, nodes: 2 + SORT id: 20, sort_direction: ASCENDING, with_fill: 0 + EXPRESSION + FUNCTION id: 2, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 3, nodes: 2 + COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 + CONSTANT id: 6, constant_value: UInt64_2, constant_value_type: UInt8 + SORT id: 21, sort_direction: ASCENDING, with_fill: 0 + EXPRESSION + FUNCTION id: 7, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 8, nodes: 2 + COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 + CONSTANT id: 9, constant_value: UInt64_3, constant_value_type: UInt8 +QUERY id: 0 + PROJECTION COLUMNS + a UInt8 + b UInt8 + PROJECTION + LIST id: 1, nodes: 2 + FUNCTION id: 2, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 3, nodes: 2 + COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 + CONSTANT id: 6, constant_value: UInt64_2, constant_value_type: UInt8 + FUNCTION id: 7, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 8, nodes: 2 + COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 + CONSTANT id: 9, constant_value: UInt64_3, constant_value_type: UInt8 + JOIN TREE + TABLE_FUNCTION id: 5, table_function_name: numbers + ARGUMENTS + LIST id: 10, nodes: 1 + CONSTANT id: 11, constant_value: UInt64_10000000, constant_value_type: UInt32 + GROUP BY + LIST id: 12, nodes: 2 + FUNCTION id: 13, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 14, nodes: 2 + COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 + CONSTANT id: 15, constant_value: UInt64_2, constant_value_type: UInt8 + FUNCTION id: 16, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 17, nodes: 2 + COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 + CONSTANT id: 18, constant_value: UInt64_3, constant_value_type: UInt8 + ORDER BY + LIST id: 19, nodes: 2 + SORT id: 20, sort_direction: ASCENDING, with_fill: 0 + EXPRESSION + FUNCTION id: 2, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 3, nodes: 2 + COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 + CONSTANT id: 6, constant_value: UInt64_2, constant_value_type: UInt8 + SORT id: 21, sort_direction: ASCENDING, with_fill: 0 + EXPRESSION + FUNCTION id: 7, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 8, nodes: 2 + COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 + CONSTANT id: 9, constant_value: UInt64_3, constant_value_type: UInt8 +QUERY id: 0 + PROJECTION COLUMNS + a UInt16 + PROJECTION + LIST id: 1, nodes: 1 + FUNCTION id: 2, function_name: multiply, function_type: ordinary, result_type: UInt16 + ARGUMENTS + LIST id: 3, nodes: 2 + FUNCTION id: 4, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 5, nodes: 2 + COLUMN id: 6, column_name: number, result_type: UInt64, source_id: 7 + CONSTANT id: 8, constant_value: UInt64_5, constant_value_type: UInt8 + FUNCTION id: 9, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 10, nodes: 2 + COLUMN id: 6, column_name: number, result_type: UInt64, source_id: 7 + CONSTANT id: 11, constant_value: UInt64_7, constant_value_type: UInt8 + JOIN TREE + TABLE_FUNCTION id: 7, table_function_name: numbers + ARGUMENTS + LIST id: 12, nodes: 1 + CONSTANT id: 13, constant_value: UInt64_10000000, constant_value_type: UInt32 + GROUP BY + LIST id: 14, nodes: 2 + FUNCTION id: 15, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 16, nodes: 2 + COLUMN id: 6, column_name: number, result_type: UInt64, source_id: 7 + CONSTANT id: 17, constant_value: UInt64_7, constant_value_type: UInt8 + FUNCTION id: 18, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 19, nodes: 2 + COLUMN id: 6, column_name: number, result_type: UInt64, source_id: 7 + CONSTANT id: 20, constant_value: UInt64_5, constant_value_type: UInt8 + ORDER BY + LIST id: 21, nodes: 1 + SORT id: 22, sort_direction: ASCENDING, with_fill: 0 + EXPRESSION + FUNCTION id: 2, function_name: multiply, function_type: ordinary, result_type: UInt16 + ARGUMENTS + LIST id: 3, nodes: 2 + FUNCTION id: 4, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 5, nodes: 2 + COLUMN id: 6, column_name: number, result_type: UInt64, source_id: 7 + CONSTANT id: 8, constant_value: UInt64_5, constant_value_type: UInt8 + FUNCTION id: 9, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 10, nodes: 2 + COLUMN id: 6, column_name: number, result_type: UInt64, source_id: 7 + CONSTANT id: 11, constant_value: UInt64_7, constant_value_type: UInt8 +QUERY id: 0 + PROJECTION COLUMNS + foo UInt64 + PROJECTION + LIST id: 1, nodes: 1 + COLUMN id: 2, column_name: foo, result_type: UInt64, source_id: 3 + JOIN TREE + QUERY id: 3, is_subquery: 1 + PROJECTION COLUMNS + foo UInt64 + PROJECTION + LIST id: 4, nodes: 1 + COLUMN id: 5, column_name: number, result_type: UInt64, source_id: 6 + JOIN TREE + TABLE_FUNCTION id: 6, table_function_name: numbers + ARGUMENTS + LIST id: 7, nodes: 1 + CONSTANT id: 8, constant_value: UInt64_1, constant_value_type: UInt8 + GROUP BY + LIST id: 9, nodes: 1 + COLUMN id: 5, column_name: number, result_type: UInt64, source_id: 6 diff --git a/tests/queries/0_stateless/02815_analyzer_aggregate_functions_of_group_by_keys.sql b/tests/queries/0_stateless/02815_analyzer_aggregate_functions_of_group_by_keys.sql new file mode 100644 index 00000000000..f2f937a819b --- /dev/null +++ b/tests/queries/0_stateless/02815_analyzer_aggregate_functions_of_group_by_keys.sql @@ -0,0 +1,29 @@ +set allow_experimental_analyzer = 1; +set optimize_move_functions_out_of_any = 0; + +SELECT 'set optimize_aggregators_of_group_by_keys = 1'; +set optimize_aggregators_of_group_by_keys = 1; + +SELECT min(number % 2) AS a, max(number % 3) AS b FROM numbers(10000000) GROUP BY number % 2, number % 3 ORDER BY a, b; +SELECT any(number % 2) AS a, anyLast(number % 3) AS b FROM numbers(10000000) GROUP BY number % 2, number % 3 ORDER BY a, b; +SELECT max((number % 5) * (number % 7)) AS a FROM numbers(10000000) GROUP BY number % 7, number % 5 ORDER BY a; +SELECT foo FROM (SELECT anyLast(number) AS foo FROM numbers(1) GROUP BY number); +SELECT anyLast(number) FROM numbers(1) GROUP BY number; + +EXPLAIN QUERY TREE SELECT min(number % 2) AS a, max(number % 3) AS b FROM numbers(10000000) GROUP BY number % 2, number % 3 ORDER BY a, b; +EXPLAIN QUERY TREE SELECT any(number % 2) AS a, anyLast(number % 3) AS b FROM numbers(10000000) GROUP BY number % 2, number % 3 ORDER BY a, b; +EXPLAIN QUERY TREE SELECT max((number % 5) * (number % 7)) AS a FROM numbers(10000000) GROUP BY number % 7, number % 5 ORDER BY a; +EXPLAIN QUERY TREE SELECT foo FROM (SELECT anyLast(number) AS foo FROM numbers(1) GROUP BY number); + +SELECT 'set optimize_aggregators_of_group_by_keys = 0'; +set optimize_aggregators_of_group_by_keys = 0; + +SELECT min(number % 2) AS a, max(number % 3) AS b FROM numbers(10000000) GROUP BY number % 2, number % 3 ORDER BY a, b; +SELECT any(number % 2) AS a, anyLast(number % 3) AS b FROM numbers(10000000) GROUP BY number % 2, number % 3 ORDER BY a, b; +SELECT max((number % 5) * (number % 7)) AS a FROM numbers(10000000) GROUP BY number % 7, number % 5 ORDER BY a; +SELECT foo FROM (SELECT anyLast(number) AS foo FROM numbers(1) GROUP BY number); + +EXPLAIN QUERY TREE SELECT min(number % 2) AS a, max(number % 3) AS b FROM numbers(10000000) GROUP BY number % 2, number % 3 ORDER BY a, b; +EXPLAIN QUERY TREE SELECT any(number % 2) AS a, anyLast(number % 3) AS b FROM numbers(10000000) GROUP BY number % 2, number % 3 ORDER BY a, b; +EXPLAIN QUERY TREE SELECT max((number % 5) * (number % 7)) AS a FROM numbers(10000000) GROUP BY number % 7, number % 5 ORDER BY a; +EXPLAIN QUERY TREE SELECT foo FROM (SELECT anyLast(number) AS foo FROM numbers(1) GROUP BY number); From 3d467252a8dcf201f44dbb5930530ecc6cfe2ad2 Mon Sep 17 00:00:00 2001 From: JackyWoo Date: Tue, 18 Jul 2023 11:42:51 +0800 Subject: [PATCH 002/530] fix style --- .../AggregateFunctionOfGroupByKeysPass.cpp | 206 +++++++++--------- 1 file changed, 103 insertions(+), 103 deletions(-) diff --git a/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp b/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp index 714d6138dc9..0becf2164cf 100644 --- a/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp +++ b/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp @@ -15,120 +15,118 @@ namespace DB namespace { - /// Check whether we should keep aggregator. - class KeepEliminateFunctionVisitor : public ConstInDepthQueryTreeVisitor +/// Check whether we should keep aggregator. +class KeepEliminateFunctionVisitor : public ConstInDepthQueryTreeVisitor +{ +public: + using Base = ConstInDepthQueryTreeVisitor; + using Base::Base; + + KeepEliminateFunctionVisitor(const QueryTreeNodes & group_by_keys_, bool & keep_aggregator_) + : group_by_keys(group_by_keys_), keep_aggregator(keep_aggregator_) { - public: - using Base = ConstInDepthQueryTreeVisitor; - using Base::Base; + } - KeepEliminateFunctionVisitor(const QueryTreeNodes & group_by_keys_, bool & keep_aggregator_) - : group_by_keys(group_by_keys_), keep_aggregator(keep_aggregator_) - { - } - - static bool needChildVisit(VisitQueryTreeNodeType & parent, VisitQueryTreeNodeType & child [[maybe_unused]]) - { - return parent->as(); - } - - void visitFunction(const FunctionNode * function_node) - { - if (function_node->getArguments().getNodes().empty()) - { - keep_aggregator = true; - return; - } - auto it = std::find_if( - group_by_keys.begin(), - group_by_keys.end(), - [function_node](const QueryTreeNodePtr & group_by_ele) { return function_node->isEqual(*group_by_ele); }); - - if (it == group_by_keys.end()) - { - KeepEliminateFunctionVisitor visitor(group_by_keys, keep_aggregator); - visitor.visit(function_node->getArgumentsNode()); - } - } - - void visitColumn(const ColumnNode * column) - { - /// if variable of a function is not in GROUP BY keys, this function should not be deleted - auto it = std::find_if( - group_by_keys.begin(), - group_by_keys.end(), - [column](const QueryTreeNodePtr & group_by_ele) { return column->isEqual(*group_by_ele); }); - - if (it == group_by_keys.end()) - keep_aggregator = true; - } - - void visitImpl(const QueryTreeNodePtr & node) - { - if (keep_aggregator) - return; - - if (node->as()) - return; - - if (auto * function_node = node->as()) - { - visitFunction(function_node); - } - else if (auto * column = node->as()) - { - visitColumn(column); - } - } - - private : - const QueryTreeNodes & group_by_keys; - bool & keep_aggregator; - }; - - /// Try to eliminate min/max/any/anyLast which will not decent into subqueries. - class EliminateFunctionVisitor : public InDepthQueryTreeVisitor + static bool needChildVisit(VisitQueryTreeNodeType & parent, VisitQueryTreeNodeType & child [[maybe_unused]]) { - public: - using Base = InDepthQueryTreeVisitor; - using Base::Base; + return parent->as(); + } - EliminateFunctionVisitor(const QueryTreeNodes & group_by_keys_) : group_by_keys(group_by_keys_) { } - - void visitImpl(QueryTreeNodePtr & node) + void visitFunction(const FunctionNode * function_node) + { + if (function_node->getArguments().getNodes().empty()) { - - /// Check if function is min/max/any/anyLast - auto * function_node = node->as(); - if (!function_node - || !(function_node->getFunctionName() == "min" || function_node->getFunctionName() == "max" - || function_node->getFunctionName() == "any" || function_node->getFunctionName() == "anyLast")) - return; - - if (!function_node->getArguments().getNodes().empty()) - { - bool keep_aggregator = false; - - KeepEliminateFunctionVisitor visitor(group_by_keys, keep_aggregator); - visitor.visit(function_node->getArgumentsNode()); - - /// Place argument of an aggregate function instead of function - if (!keep_aggregator) - node = function_node->getArguments().getNodes()[0]; - } + keep_aggregator = true; + return; } + auto it = std::find_if( + group_by_keys.begin(), + group_by_keys.end(), + [function_node](const QueryTreeNodePtr & group_by_ele) { return function_node->isEqual(*group_by_ele); }); - static bool needChildVisit(VisitQueryTreeNodeType & parent [[maybe_unused]], VisitQueryTreeNodeType & child) + if (it == group_by_keys.end()) { - /// Don't descent into table functions and subqueries and special case for ArrayJoin. - return !child->as() && !child->as() && !child->as(); + KeepEliminateFunctionVisitor visitor(group_by_keys, keep_aggregator); + visitor.visit(function_node->getArgumentsNode()); } + } - private: - const QueryTreeNodes & group_by_keys; - }; + void visitColumn(const ColumnNode * column) + { + /// if variable of a function is not in GROUP BY keys, this function should not be deleted + auto it = std::find_if( + group_by_keys.begin(), + group_by_keys.end(), + [column](const QueryTreeNodePtr & group_by_ele) { return column->isEqual(*group_by_ele); }); -} + if (it == group_by_keys.end()) + keep_aggregator = true; + } + + void visitImpl(const QueryTreeNodePtr & node) + { + if (keep_aggregator) + return; + + if (node->as()) + return; + + if (auto * function_node = node->as()) + { + visitFunction(function_node); + } + else if (auto * column = node->as()) + { + visitColumn(column); + } + } + +private : + const QueryTreeNodes & group_by_keys; + bool & keep_aggregator; +}; + +/// Try to eliminate min/max/any/anyLast which will not decent into subqueries. +class EliminateFunctionVisitor : public InDepthQueryTreeVisitor +{ +public: + using Base = InDepthQueryTreeVisitor; + using Base::Base; + + EliminateFunctionVisitor(const QueryTreeNodes & group_by_keys_) : group_by_keys(group_by_keys_) { } + + void visitImpl(QueryTreeNodePtr & node) + { + + /// Check if function is min/max/any/anyLast + auto * function_node = node->as(); + if (!function_node + || !(function_node->getFunctionName() == "min" || function_node->getFunctionName() == "max" + || function_node->getFunctionName() == "any" || function_node->getFunctionName() == "anyLast")) + return; + + if (!function_node->getArguments().getNodes().empty()) + { + bool keep_aggregator = false; + + KeepEliminateFunctionVisitor visitor(group_by_keys, keep_aggregator); + visitor.visit(function_node->getArgumentsNode()); + + /// Place argument of an aggregate function instead of function + if (!keep_aggregator) + node = function_node->getArguments().getNodes()[0]; + } + } + + static bool needChildVisit(VisitQueryTreeNodeType & parent [[maybe_unused]], VisitQueryTreeNodeType & child) + { + /// Don't descent into table functions and subqueries and special case for ArrayJoin. + return !child->as() && !child->as() && !child->as(); + } + +private: + const QueryTreeNodes & group_by_keys; +}; /// Collect QueryNode and its group by keys. class CollectQueryAndGroupByKeysVisitor : public InDepthQueryTreeVisitor @@ -172,6 +170,8 @@ public: } }; +} + void AggregateFunctionOfGroupByKeysPass::run(QueryTreeNodePtr query_tree_node, ContextPtr /*context*/) { CollectQueryAndGroupByKeysVisitor collector; From a2001e70874f2d2eb79f746a716a1bda67742bef Mon Sep 17 00:00:00 2001 From: JackyWoo Date: Wed, 19 Jul 2023 15:29:36 +0800 Subject: [PATCH 003/530] use switch --- src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp b/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp index 0becf2164cf..3a89ab3a1fc 100644 --- a/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp +++ b/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp @@ -172,8 +172,11 @@ public: } -void AggregateFunctionOfGroupByKeysPass::run(QueryTreeNodePtr query_tree_node, ContextPtr /*context*/) +void AggregateFunctionOfGroupByKeysPass::run(QueryTreeNodePtr query_tree_node, ContextPtr context) { + if (!context->getSettingsRef().optimize_aggregators_of_group_by_keys) + return; + CollectQueryAndGroupByKeysVisitor collector; collector.visit(query_tree_node); From 7290b9e51596a72deacebd469df8bb1944f1ecb1 Mon Sep 17 00:00:00 2001 From: JackyWoo Date: Wed, 19 Jul 2023 15:29:50 +0800 Subject: [PATCH 004/530] fix 02496_remove_redundant_sorting_analyzer --- ...emove_redundant_sorting_analyzer.reference | 23 +++++++------------ 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/tests/queries/0_stateless/02496_remove_redundant_sorting_analyzer.reference b/tests/queries/0_stateless/02496_remove_redundant_sorting_analyzer.reference index ddc89a72821..17e8820e0fa 100644 --- a/tests/queries/0_stateless/02496_remove_redundant_sorting_analyzer.reference +++ b/tests/queries/0_stateless/02496_remove_redundant_sorting_analyzer.reference @@ -220,10 +220,8 @@ GROUP BY number -- explain Expression ((Project names + Projection)) Aggregating - Expression ((Before GROUP BY + (Change column names to column identifiers + Project names))) - Sorting (Sorting for ORDER BY) - Expression ((Before ORDER BY + (Projection + (Change column names to column identifiers + (Project names + (Before ORDER BY + (Projection + Change column names to column identifiers))))))) - ReadFromStorage (SystemNumbers) + Expression ((Before GROUP BY + (Change column names to column identifiers + (Project names + (Before ORDER BY + (Projection + (Change column names to column identifiers + (Project names + (Before ORDER BY + (Projection + Change column names to column identifiers)))))))))) + ReadFromStorage (SystemNumbers) -- execute 0 2 @@ -291,10 +289,8 @@ Expression (Project names) Sorting (Sorting for ORDER BY) Expression ((Before ORDER BY + (Projection + (Change column names to column identifiers + (Project names + Projection))))) Aggregating - Expression ((Before GROUP BY + (Change column names to column identifiers + Project names))) - Sorting (Sorting for ORDER BY) - Expression ((Before ORDER BY + (Projection + Change column names to column identifiers))) - ReadFromStorage (SystemNumbers) + Expression ((Before GROUP BY + (Change column names to column identifiers + (Project names + (Before ORDER BY + (Projection + Change column names to column identifiers)))))) + ReadFromStorage (SystemNumbers) -- execute 0 1 @@ -320,13 +316,10 @@ FROM ) WHERE a > 0 -- explain -Expression ((Project names + Projection)) - Filter ((WHERE + (Change column names to column identifiers + (Project names + Projection)))) - Aggregating - Expression ((Before GROUP BY + (Change column names to column identifiers + Project names))) - Sorting (Sorting for ORDER BY) - Expression ((Before ORDER BY + (Projection + (Change column names to column identifiers + (Project names + (Before ORDER BY + (Projection + Change column names to column identifiers))))))) - ReadFromStorage (SystemNumbers) +Expression ((Project names + (Projection + ))) + Aggregating + Filter (( + (Before GROUP BY + (Change column names to column identifiers + (Project names + (Before ORDER BY + (Projection + (Change column names to column identifiers + (Project names + (Before ORDER BY + (Projection + Change column names to column identifiers))))))))))) + ReadFromStorage (SystemNumbers) -- execute 2 1 From 8e1dbdfef49b3e964022c8a4c3b44956e920619d Mon Sep 17 00:00:00 2001 From: JackyWoo Date: Wed, 19 Jul 2023 20:09:13 +0800 Subject: [PATCH 005/530] fix tests --- ...egate_functions_of_group_by_keys.reference | 237 ++++++++++-------- 1 file changed, 135 insertions(+), 102 deletions(-) diff --git a/tests/queries/0_stateless/02815_analyzer_aggregate_functions_of_group_by_keys.reference b/tests/queries/0_stateless/02815_analyzer_aggregate_functions_of_group_by_keys.reference index 2164bfa5b19..1f3e458a60d 100644 --- a/tests/queries/0_stateless/02815_analyzer_aggregate_functions_of_group_by_keys.reference +++ b/tests/queries/0_stateless/02815_analyzer_aggregate_functions_of_group_by_keys.reference @@ -274,150 +274,180 @@ QUERY id: 0 b UInt8 PROJECTION LIST id: 1, nodes: 2 - FUNCTION id: 2, function_name: modulo, function_type: ordinary, result_type: UInt8 + FUNCTION id: 2, function_name: min, function_type: aggregate, result_type: UInt8 ARGUMENTS - LIST id: 3, nodes: 2 - COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 - CONSTANT id: 6, constant_value: UInt64_2, constant_value_type: UInt8 - FUNCTION id: 7, function_name: modulo, function_type: ordinary, result_type: UInt8 + LIST id: 3, nodes: 1 + FUNCTION id: 4, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 5, nodes: 2 + COLUMN id: 6, column_name: number, result_type: UInt64, source_id: 7 + CONSTANT id: 8, constant_value: UInt64_2, constant_value_type: UInt8 + FUNCTION id: 9, function_name: max, function_type: aggregate, result_type: UInt8 ARGUMENTS - LIST id: 8, nodes: 2 - COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 - CONSTANT id: 9, constant_value: UInt64_3, constant_value_type: UInt8 + LIST id: 10, nodes: 1 + FUNCTION id: 11, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 12, nodes: 2 + COLUMN id: 6, column_name: number, result_type: UInt64, source_id: 7 + CONSTANT id: 13, constant_value: UInt64_3, constant_value_type: UInt8 JOIN TREE - TABLE_FUNCTION id: 5, table_function_name: numbers + TABLE_FUNCTION id: 7, table_function_name: numbers ARGUMENTS - LIST id: 10, nodes: 1 - CONSTANT id: 11, constant_value: UInt64_10000000, constant_value_type: UInt32 + LIST id: 14, nodes: 1 + CONSTANT id: 15, constant_value: UInt64_10000000, constant_value_type: UInt32 GROUP BY - LIST id: 12, nodes: 2 - FUNCTION id: 13, function_name: modulo, function_type: ordinary, result_type: UInt8 + LIST id: 16, nodes: 2 + FUNCTION id: 17, function_name: modulo, function_type: ordinary, result_type: UInt8 ARGUMENTS - LIST id: 14, nodes: 2 - COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 - CONSTANT id: 15, constant_value: UInt64_2, constant_value_type: UInt8 - FUNCTION id: 16, function_name: modulo, function_type: ordinary, result_type: UInt8 + LIST id: 18, nodes: 2 + COLUMN id: 6, column_name: number, result_type: UInt64, source_id: 7 + CONSTANT id: 19, constant_value: UInt64_2, constant_value_type: UInt8 + FUNCTION id: 20, function_name: modulo, function_type: ordinary, result_type: UInt8 ARGUMENTS - LIST id: 17, nodes: 2 - COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 - CONSTANT id: 18, constant_value: UInt64_3, constant_value_type: UInt8 + LIST id: 21, nodes: 2 + COLUMN id: 6, column_name: number, result_type: UInt64, source_id: 7 + CONSTANT id: 22, constant_value: UInt64_3, constant_value_type: UInt8 ORDER BY - LIST id: 19, nodes: 2 - SORT id: 20, sort_direction: ASCENDING, with_fill: 0 + LIST id: 23, nodes: 2 + SORT id: 24, sort_direction: ASCENDING, with_fill: 0 EXPRESSION - FUNCTION id: 2, function_name: modulo, function_type: ordinary, result_type: UInt8 + FUNCTION id: 2, function_name: min, function_type: aggregate, result_type: UInt8 ARGUMENTS - LIST id: 3, nodes: 2 - COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 - CONSTANT id: 6, constant_value: UInt64_2, constant_value_type: UInt8 - SORT id: 21, sort_direction: ASCENDING, with_fill: 0 + LIST id: 3, nodes: 1 + FUNCTION id: 4, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 5, nodes: 2 + COLUMN id: 6, column_name: number, result_type: UInt64, source_id: 7 + CONSTANT id: 8, constant_value: UInt64_2, constant_value_type: UInt8 + SORT id: 25, sort_direction: ASCENDING, with_fill: 0 EXPRESSION - FUNCTION id: 7, function_name: modulo, function_type: ordinary, result_type: UInt8 + FUNCTION id: 9, function_name: max, function_type: aggregate, result_type: UInt8 ARGUMENTS - LIST id: 8, nodes: 2 - COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 - CONSTANT id: 9, constant_value: UInt64_3, constant_value_type: UInt8 + LIST id: 10, nodes: 1 + FUNCTION id: 11, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 12, nodes: 2 + COLUMN id: 6, column_name: number, result_type: UInt64, source_id: 7 + CONSTANT id: 13, constant_value: UInt64_3, constant_value_type: UInt8 QUERY id: 0 PROJECTION COLUMNS a UInt8 b UInt8 PROJECTION LIST id: 1, nodes: 2 - FUNCTION id: 2, function_name: modulo, function_type: ordinary, result_type: UInt8 + FUNCTION id: 2, function_name: any, function_type: aggregate, result_type: UInt8 ARGUMENTS - LIST id: 3, nodes: 2 - COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 - CONSTANT id: 6, constant_value: UInt64_2, constant_value_type: UInt8 - FUNCTION id: 7, function_name: modulo, function_type: ordinary, result_type: UInt8 + LIST id: 3, nodes: 1 + FUNCTION id: 4, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 5, nodes: 2 + COLUMN id: 6, column_name: number, result_type: UInt64, source_id: 7 + CONSTANT id: 8, constant_value: UInt64_2, constant_value_type: UInt8 + FUNCTION id: 9, function_name: anyLast, function_type: aggregate, result_type: UInt8 ARGUMENTS - LIST id: 8, nodes: 2 - COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 - CONSTANT id: 9, constant_value: UInt64_3, constant_value_type: UInt8 + LIST id: 10, nodes: 1 + FUNCTION id: 11, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 12, nodes: 2 + COLUMN id: 6, column_name: number, result_type: UInt64, source_id: 7 + CONSTANT id: 13, constant_value: UInt64_3, constant_value_type: UInt8 JOIN TREE - TABLE_FUNCTION id: 5, table_function_name: numbers + TABLE_FUNCTION id: 7, table_function_name: numbers ARGUMENTS - LIST id: 10, nodes: 1 - CONSTANT id: 11, constant_value: UInt64_10000000, constant_value_type: UInt32 + LIST id: 14, nodes: 1 + CONSTANT id: 15, constant_value: UInt64_10000000, constant_value_type: UInt32 GROUP BY - LIST id: 12, nodes: 2 - FUNCTION id: 13, function_name: modulo, function_type: ordinary, result_type: UInt8 + LIST id: 16, nodes: 2 + FUNCTION id: 17, function_name: modulo, function_type: ordinary, result_type: UInt8 ARGUMENTS - LIST id: 14, nodes: 2 - COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 - CONSTANT id: 15, constant_value: UInt64_2, constant_value_type: UInt8 - FUNCTION id: 16, function_name: modulo, function_type: ordinary, result_type: UInt8 + LIST id: 18, nodes: 2 + COLUMN id: 6, column_name: number, result_type: UInt64, source_id: 7 + CONSTANT id: 19, constant_value: UInt64_2, constant_value_type: UInt8 + FUNCTION id: 20, function_name: modulo, function_type: ordinary, result_type: UInt8 ARGUMENTS - LIST id: 17, nodes: 2 - COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 - CONSTANT id: 18, constant_value: UInt64_3, constant_value_type: UInt8 + LIST id: 21, nodes: 2 + COLUMN id: 6, column_name: number, result_type: UInt64, source_id: 7 + CONSTANT id: 22, constant_value: UInt64_3, constant_value_type: UInt8 ORDER BY - LIST id: 19, nodes: 2 - SORT id: 20, sort_direction: ASCENDING, with_fill: 0 + LIST id: 23, nodes: 2 + SORT id: 24, sort_direction: ASCENDING, with_fill: 0 EXPRESSION - FUNCTION id: 2, function_name: modulo, function_type: ordinary, result_type: UInt8 + FUNCTION id: 2, function_name: any, function_type: aggregate, result_type: UInt8 ARGUMENTS - LIST id: 3, nodes: 2 - COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 - CONSTANT id: 6, constant_value: UInt64_2, constant_value_type: UInt8 - SORT id: 21, sort_direction: ASCENDING, with_fill: 0 + LIST id: 3, nodes: 1 + FUNCTION id: 4, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 5, nodes: 2 + COLUMN id: 6, column_name: number, result_type: UInt64, source_id: 7 + CONSTANT id: 8, constant_value: UInt64_2, constant_value_type: UInt8 + SORT id: 25, sort_direction: ASCENDING, with_fill: 0 EXPRESSION - FUNCTION id: 7, function_name: modulo, function_type: ordinary, result_type: UInt8 + FUNCTION id: 9, function_name: anyLast, function_type: aggregate, result_type: UInt8 ARGUMENTS - LIST id: 8, nodes: 2 - COLUMN id: 4, column_name: number, result_type: UInt64, source_id: 5 - CONSTANT id: 9, constant_value: UInt64_3, constant_value_type: UInt8 + LIST id: 10, nodes: 1 + FUNCTION id: 11, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 12, nodes: 2 + COLUMN id: 6, column_name: number, result_type: UInt64, source_id: 7 + CONSTANT id: 13, constant_value: UInt64_3, constant_value_type: UInt8 QUERY id: 0 PROJECTION COLUMNS a UInt16 PROJECTION LIST id: 1, nodes: 1 - FUNCTION id: 2, function_name: multiply, function_type: ordinary, result_type: UInt16 + FUNCTION id: 2, function_name: max, function_type: aggregate, result_type: UInt16 ARGUMENTS - LIST id: 3, nodes: 2 - FUNCTION id: 4, function_name: modulo, function_type: ordinary, result_type: UInt8 + LIST id: 3, nodes: 1 + FUNCTION id: 4, function_name: multiply, function_type: ordinary, result_type: UInt16 ARGUMENTS LIST id: 5, nodes: 2 - COLUMN id: 6, column_name: number, result_type: UInt64, source_id: 7 - CONSTANT id: 8, constant_value: UInt64_5, constant_value_type: UInt8 - FUNCTION id: 9, function_name: modulo, function_type: ordinary, result_type: UInt8 - ARGUMENTS - LIST id: 10, nodes: 2 - COLUMN id: 6, column_name: number, result_type: UInt64, source_id: 7 - CONSTANT id: 11, constant_value: UInt64_7, constant_value_type: UInt8 + FUNCTION id: 6, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 7, nodes: 2 + COLUMN id: 8, column_name: number, result_type: UInt64, source_id: 9 + CONSTANT id: 10, constant_value: UInt64_5, constant_value_type: UInt8 + FUNCTION id: 11, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 12, nodes: 2 + COLUMN id: 8, column_name: number, result_type: UInt64, source_id: 9 + CONSTANT id: 13, constant_value: UInt64_7, constant_value_type: UInt8 JOIN TREE - TABLE_FUNCTION id: 7, table_function_name: numbers + TABLE_FUNCTION id: 9, table_function_name: numbers ARGUMENTS - LIST id: 12, nodes: 1 - CONSTANT id: 13, constant_value: UInt64_10000000, constant_value_type: UInt32 + LIST id: 14, nodes: 1 + CONSTANT id: 15, constant_value: UInt64_10000000, constant_value_type: UInt32 GROUP BY - LIST id: 14, nodes: 2 - FUNCTION id: 15, function_name: modulo, function_type: ordinary, result_type: UInt8 + LIST id: 16, nodes: 2 + FUNCTION id: 17, function_name: modulo, function_type: ordinary, result_type: UInt8 ARGUMENTS - LIST id: 16, nodes: 2 - COLUMN id: 6, column_name: number, result_type: UInt64, source_id: 7 - CONSTANT id: 17, constant_value: UInt64_7, constant_value_type: UInt8 - FUNCTION id: 18, function_name: modulo, function_type: ordinary, result_type: UInt8 + LIST id: 18, nodes: 2 + COLUMN id: 8, column_name: number, result_type: UInt64, source_id: 9 + CONSTANT id: 19, constant_value: UInt64_7, constant_value_type: UInt8 + FUNCTION id: 20, function_name: modulo, function_type: ordinary, result_type: UInt8 ARGUMENTS - LIST id: 19, nodes: 2 - COLUMN id: 6, column_name: number, result_type: UInt64, source_id: 7 - CONSTANT id: 20, constant_value: UInt64_5, constant_value_type: UInt8 + LIST id: 21, nodes: 2 + COLUMN id: 8, column_name: number, result_type: UInt64, source_id: 9 + CONSTANT id: 22, constant_value: UInt64_5, constant_value_type: UInt8 ORDER BY - LIST id: 21, nodes: 1 - SORT id: 22, sort_direction: ASCENDING, with_fill: 0 + LIST id: 23, nodes: 1 + SORT id: 24, sort_direction: ASCENDING, with_fill: 0 EXPRESSION - FUNCTION id: 2, function_name: multiply, function_type: ordinary, result_type: UInt16 + FUNCTION id: 2, function_name: max, function_type: aggregate, result_type: UInt16 ARGUMENTS - LIST id: 3, nodes: 2 - FUNCTION id: 4, function_name: modulo, function_type: ordinary, result_type: UInt8 + LIST id: 3, nodes: 1 + FUNCTION id: 4, function_name: multiply, function_type: ordinary, result_type: UInt16 ARGUMENTS LIST id: 5, nodes: 2 - COLUMN id: 6, column_name: number, result_type: UInt64, source_id: 7 - CONSTANT id: 8, constant_value: UInt64_5, constant_value_type: UInt8 - FUNCTION id: 9, function_name: modulo, function_type: ordinary, result_type: UInt8 - ARGUMENTS - LIST id: 10, nodes: 2 - COLUMN id: 6, column_name: number, result_type: UInt64, source_id: 7 - CONSTANT id: 11, constant_value: UInt64_7, constant_value_type: UInt8 + FUNCTION id: 6, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 7, nodes: 2 + COLUMN id: 8, column_name: number, result_type: UInt64, source_id: 9 + CONSTANT id: 10, constant_value: UInt64_5, constant_value_type: UInt8 + FUNCTION id: 11, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 12, nodes: 2 + COLUMN id: 8, column_name: number, result_type: UInt64, source_id: 9 + CONSTANT id: 13, constant_value: UInt64_7, constant_value_type: UInt8 QUERY id: 0 PROJECTION COLUMNS foo UInt64 @@ -430,12 +460,15 @@ QUERY id: 0 foo UInt64 PROJECTION LIST id: 4, nodes: 1 - COLUMN id: 5, column_name: number, result_type: UInt64, source_id: 6 + FUNCTION id: 5, function_name: anyLast, function_type: aggregate, result_type: UInt64 + ARGUMENTS + LIST id: 6, nodes: 1 + COLUMN id: 7, column_name: number, result_type: UInt64, source_id: 8 JOIN TREE - TABLE_FUNCTION id: 6, table_function_name: numbers + TABLE_FUNCTION id: 8, table_function_name: numbers ARGUMENTS - LIST id: 7, nodes: 1 - CONSTANT id: 8, constant_value: UInt64_1, constant_value_type: UInt8 + LIST id: 9, nodes: 1 + CONSTANT id: 10, constant_value: UInt64_1, constant_value_type: UInt8 GROUP BY - LIST id: 9, nodes: 1 - COLUMN id: 5, column_name: number, result_type: UInt64, source_id: 6 + LIST id: 11, nodes: 1 + COLUMN id: 7, column_name: number, result_type: UInt64, source_id: 8 From afc6bccbcec3d7acaf2d4d718ca333e434090854 Mon Sep 17 00:00:00 2001 From: JackyWoo Date: Fri, 21 Jul 2023 17:34:19 +0800 Subject: [PATCH 006/530] fix build error --- .../Passes/AggregateFunctionOfGroupByKeysPass.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp b/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp index 3a89ab3a1fc..6da0234c4e7 100644 --- a/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp +++ b/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp @@ -180,11 +180,11 @@ void AggregateFunctionOfGroupByKeysPass::run(QueryTreeNodePtr query_tree_node, C CollectQueryAndGroupByKeysVisitor collector; collector.visit(query_tree_node); - for (auto it = collector.data.begin(); it != collector.data.end(); it++) + for (auto & [query_node, group_by_keys] : collector.data) { - EliminateFunctionVisitor eliminator(it->second); - auto query_node = it->first; - eliminator.visit(query_node); + EliminateFunctionVisitor eliminator(group_by_keys); + auto mutable_query_node = query_node; + eliminator.visit(mutable_query_node); } } From 626088731b8971201284186436c8b95d234b6816 Mon Sep 17 00:00:00 2001 From: JackyWoo Date: Fri, 21 Jul 2023 18:41:27 +0800 Subject: [PATCH 007/530] add explicit to constructor --- src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp b/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp index 6da0234c4e7..bd036afe5e4 100644 --- a/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp +++ b/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp @@ -22,7 +22,7 @@ public: using Base = ConstInDepthQueryTreeVisitor; using Base::Base; - KeepEliminateFunctionVisitor(const QueryTreeNodes & group_by_keys_, bool & keep_aggregator_) + explicit KeepEliminateFunctionVisitor(const QueryTreeNodes & group_by_keys_, bool & keep_aggregator_) : group_by_keys(group_by_keys_), keep_aggregator(keep_aggregator_) { } From f512b7a21746f8142cafafab63c780da33a3beb8 Mon Sep 17 00:00:00 2001 From: JackyWoo Date: Fri, 28 Jul 2023 17:53:55 +0800 Subject: [PATCH 008/530] use settings in visitor --- .../AggregateFunctionOfGroupByKeysPass.cpp | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp b/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp index bd036afe5e4..f7b0e80a7a7 100644 --- a/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp +++ b/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp @@ -87,17 +87,16 @@ private : }; /// Try to eliminate min/max/any/anyLast which will not decent into subqueries. -class EliminateFunctionVisitor : public InDepthQueryTreeVisitor +class EliminateFunctionVisitor : public InDepthQueryTreeVisitorWithContext { public: - using Base = InDepthQueryTreeVisitor; + using Base = InDepthQueryTreeVisitorWithContext; using Base::Base; - EliminateFunctionVisitor(const QueryTreeNodes & group_by_keys_) : group_by_keys(group_by_keys_) { } + explicit EliminateFunctionVisitor(const QueryTreeNodes & group_by_keys_, ContextPtr context) : Base(context), group_by_keys(group_by_keys_) { } - void visitImpl(QueryTreeNodePtr & node) + void enterImpl(QueryTreeNodePtr & node) { - /// Check if function is min/max/any/anyLast auto * function_node = node->as(); if (!function_node @@ -129,19 +128,20 @@ private: }; /// Collect QueryNode and its group by keys. -class CollectQueryAndGroupByKeysVisitor : public InDepthQueryTreeVisitor +class CollectQueryAndGroupByKeysVisitor : public InDepthQueryTreeVisitorWithContext { public: - using Base = InDepthQueryTreeVisitor; + using Base = InDepthQueryTreeVisitorWithContext; using Base::Base; using Data = std::unordered_map; Data data; - CollectQueryAndGroupByKeysVisitor() = default; - - void visitImpl(QueryTreeNodePtr & node) + void enterImpl(QueryTreeNodePtr & node) { + if (!getSettings().optimize_aggregators_of_group_by_keys) + return; + auto * query_node = node->as(); if (!query_node) return; @@ -174,15 +174,12 @@ public: void AggregateFunctionOfGroupByKeysPass::run(QueryTreeNodePtr query_tree_node, ContextPtr context) { - if (!context->getSettingsRef().optimize_aggregators_of_group_by_keys) - return; - - CollectQueryAndGroupByKeysVisitor collector; + CollectQueryAndGroupByKeysVisitor collector(context); collector.visit(query_tree_node); for (auto & [query_node, group_by_keys] : collector.data) { - EliminateFunctionVisitor eliminator(group_by_keys); + EliminateFunctionVisitor eliminator(group_by_keys, query_node->as()->getContext()); auto mutable_query_node = query_node; eliminator.visit(mutable_query_node); } From 461c2fba8b9cd8ab58b3448e9438d961f9ed3660 Mon Sep 17 00:00:00 2001 From: JackyWoo Date: Wed, 2 Aug 2023 16:45:16 +0800 Subject: [PATCH 009/530] merge 2 visitors to 1 --- .../AggregateFunctionOfGroupByKeysPass.cpp | 140 +++++++++--------- 1 file changed, 69 insertions(+), 71 deletions(-) diff --git a/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp b/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp index f7b0e80a7a7..de2551a049c 100644 --- a/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp +++ b/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp @@ -86,103 +86,101 @@ private : bool & keep_aggregator; }; -/// Try to eliminate min/max/any/anyLast which will not decent into subqueries. +/// Try to eliminate min/max/any/anyLast. class EliminateFunctionVisitor : public InDepthQueryTreeVisitorWithContext { public: using Base = InDepthQueryTreeVisitorWithContext; using Base::Base; - explicit EliminateFunctionVisitor(const QueryTreeNodes & group_by_keys_, ContextPtr context) : Base(context), group_by_keys(group_by_keys_) { } - - void enterImpl(QueryTreeNodePtr & node) - { - /// Check if function is min/max/any/anyLast - auto * function_node = node->as(); - if (!function_node - || !(function_node->getFunctionName() == "min" || function_node->getFunctionName() == "max" - || function_node->getFunctionName() == "any" || function_node->getFunctionName() == "anyLast")) - return; - - if (!function_node->getArguments().getNodes().empty()) - { - bool keep_aggregator = false; - - KeepEliminateFunctionVisitor visitor(group_by_keys, keep_aggregator); - visitor.visit(function_node->getArgumentsNode()); - - /// Place argument of an aggregate function instead of function - if (!keep_aggregator) - node = function_node->getArguments().getNodes()[0]; - } - } - - static bool needChildVisit(VisitQueryTreeNodeType & parent [[maybe_unused]], VisitQueryTreeNodeType & child) - { - /// Don't descent into table functions and subqueries and special case for ArrayJoin. - return !child->as() && !child->as() && !child->as(); - } - -private: - const QueryTreeNodes & group_by_keys; -}; - -/// Collect QueryNode and its group by keys. -class CollectQueryAndGroupByKeysVisitor : public InDepthQueryTreeVisitorWithContext -{ -public: - using Base = InDepthQueryTreeVisitorWithContext; - using Base::Base; - - using Data = std::unordered_map; - Data data; + using GroupByKeysStack = std::vector; void enterImpl(QueryTreeNodePtr & node) { if (!getSettings().optimize_aggregators_of_group_by_keys) return; - auto * query_node = node->as(); - if (!query_node) - return; - - if (!query_node->hasGroupBy()) - return; - - if (query_node->isGroupByWithTotals() || query_node->isGroupByWithCube() || query_node->isGroupByWithRollup()) - return; - - QueryTreeNodes group_by_keys; - for (auto & group_key : query_node->getGroupBy().getNodes()) + /// (1) collect group by keys + if (auto * query_node = node->as()) { - /// for grouping sets case - if (auto * list = group_key->as()) + if (!query_node->hasGroupBy()) { - for (auto & group_elem : list->getNodes()) - group_by_keys.push_back(group_elem); + group_by_keys_stack.push_back({}); + } + else if (query_node->isGroupByWithTotals() || query_node->isGroupByWithCube() || query_node->isGroupByWithRollup()) + { + /// Keep aggregator if group by is with totals/cube/rollup. + group_by_keys_stack.push_back({}); } else { - group_by_keys.push_back(group_key); + QueryTreeNodes group_by_keys; + for (auto & group_key : query_node->getGroupBy().getNodes()) + { + /// for grouping sets case + if (auto * list = group_key->as()) + { + for (auto & group_elem : list->getNodes()) + group_by_keys.push_back(group_elem); + } + else + { + group_by_keys.push_back(group_key); + } + } + group_by_keys_stack.push_back(std::move(group_by_keys)); } } - data.insert({node, std::move(group_by_keys)}); + /// (2) Try to eliminate any/min/max + else if (auto * function_node = node->as()) + { + if (!function_node + || !(function_node->getFunctionName() == "min" || function_node->getFunctionName() == "max" + || function_node->getFunctionName() == "any" || function_node->getFunctionName() == "anyLast")) + return; + + if (!function_node->getArguments().getNodes().empty()) + { + bool keep_aggregator = false; + + KeepEliminateFunctionVisitor visitor(group_by_keys_stack.back(), keep_aggregator); + visitor.visit(function_node->getArgumentsNode()); + + /// Place argument of an aggregate function instead of function + if (!keep_aggregator) + node = function_node->getArguments().getNodes()[0]; + } + } + } + + /// Now we visit all nodes in QueryNode, we should remove group_by_keys from stack. + void leaveImpl(QueryTreeNodePtr & node) + { + if (!getSettings().optimize_aggregators_of_group_by_keys) + return; + + if (auto * query_node = node->as()) + group_by_keys_stack.pop_back(); + } + + static bool needChildVisit(VisitQueryTreeNodeType & parent [[maybe_unused]], VisitQueryTreeNodeType & child) + { + /// Skip ArrayJoin. + return !child->as(); + } + +private: + GroupByKeysStack group_by_keys_stack; + }; } void AggregateFunctionOfGroupByKeysPass::run(QueryTreeNodePtr query_tree_node, ContextPtr context) { - CollectQueryAndGroupByKeysVisitor collector(context); - collector.visit(query_tree_node); - - for (auto & [query_node, group_by_keys] : collector.data) - { - EliminateFunctionVisitor eliminator(group_by_keys, query_node->as()->getContext()); - auto mutable_query_node = query_node; - eliminator.visit(mutable_query_node); - } + EliminateFunctionVisitor eliminator(context); + eliminator.visit(query_tree_node); } }; From 1302842c3968dae57ab9e732139f3aa1d415ddba Mon Sep 17 00:00:00 2001 From: JackyWoo Date: Wed, 2 Aug 2023 18:33:31 +0800 Subject: [PATCH 010/530] fix build error --- src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp b/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp index de2551a049c..a82ece52c7e 100644 --- a/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp +++ b/src/Analyzer/Passes/AggregateFunctionOfGroupByKeysPass.cpp @@ -93,7 +93,7 @@ public: using Base = InDepthQueryTreeVisitorWithContext; using Base::Base; - using GroupByKeysStack = std::vector; + using GroupByKeysStack = std::vector; void enterImpl(QueryTreeNodePtr & node) { From 1950bc4cd4a88b1feb29e965e516797c37fffd37 Mon Sep 17 00:00:00 2001 From: Marina Fathouat Date: Fri, 17 Nov 2023 17:32:33 +0100 Subject: [PATCH 011/530] Hello from Marina --- src/Interpreters/loadMetadata.cpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/Interpreters/loadMetadata.cpp b/src/Interpreters/loadMetadata.cpp index 3612dbfdc4e..dfa0a129773 100644 --- a/src/Interpreters/loadMetadata.cpp +++ b/src/Interpreters/loadMetadata.cpp @@ -221,13 +221,23 @@ void loadMetadata(ContextMutablePtr context, const String & default_database_nam databases.emplace(default_database_name, std::filesystem::path(path) / escapeForFileName(default_database_name)); } + bool has_ordinary_databases=false; TablesLoader::Databases loaded_databases; for (const auto & [name, db_path] : databases) { loadDatabase(context, name, db_path, has_force_restore_data_flag); loaded_databases.insert({name, DatabaseCatalog::instance().getDatabase(name)}); - } + }autodatabase=DatabaseCatalog::instance().getDatabase(name); + if (database->getEngineName()=="Ordinary") + + has_ordinary_databases = true; + loaded_databases.insert({name,database}) + + +if( has_ordinary_databases) + context->addWarningMessage("Server Has Databases with Ordinary Engine which is not optimal. \n " + "To convert this database to a new Atomic engine, please create a flag named `convert_ordinary_to_atomic` in flags directory"); auto mode = getLoadingStrictnessLevel(/* attach */ true, /* force_attach */ true, has_force_restore_data_flag); TablesLoader loader{context, std::move(loaded_databases), mode}; loader.loadTables(); From 31526d73235ae9a66845f337df77bc317d5eb564 Mon Sep 17 00:00:00 2001 From: shabroo <114589608+shabroo@users.noreply.github.com> Date: Fri, 17 Nov 2023 17:54:56 +0100 Subject: [PATCH 012/530] Me again! My first attempt at a contribution --- src/Interpreters/loadMetadata.cpp | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/Interpreters/loadMetadata.cpp b/src/Interpreters/loadMetadata.cpp index dfa0a129773..5b0666abfb4 100644 --- a/src/Interpreters/loadMetadata.cpp +++ b/src/Interpreters/loadMetadata.cpp @@ -221,23 +221,24 @@ void loadMetadata(ContextMutablePtr context, const String & default_database_nam databases.emplace(default_database_name, std::filesystem::path(path) / escapeForFileName(default_database_name)); } - bool has_ordinary_databases=false; + bool has_ordinary_databases = false; TablesLoader::Databases loaded_databases; for (const auto & [name, db_path] : databases) { loadDatabase(context, name, db_path, has_force_restore_data_flag); loaded_databases.insert({name, DatabaseCatalog::instance().getDatabase(name)}); - }autodatabase=DatabaseCatalog::instance().getDatabase(name); + auto database = DatabaseCatalog::instance().getDatabase(name); - if (database->getEngineName()=="Ordinary") + if (database->getEngineName() == "Ordinary") + has_ordinary_databases = true; + + loaded_databases.insert({name,database}); + } - has_ordinary_databases = true; - loaded_databases.insert({name,database}) - - -if( has_ordinary_databases) - context->addWarningMessage("Server Has Databases with Ordinary Engine which is not optimal. \n " - "To convert this database to a new Atomic engine, please create a flag named `convert_ordinary_to_atomic` in flags directory"); + if (has_ordinary_databases) + context->addWarningMessage("Server Has Databases with Ordinary Engine which is not optimal. \n " + "To convert this database to a new Atomic engine, please create a flag named `convert_ordinary_to_atomic` in flags directory"); + auto mode = getLoadingStrictnessLevel(/* attach */ true, /* force_attach */ true, has_force_restore_data_flag); TablesLoader loader{context, std::move(loaded_databases), mode}; loader.loadTables(); From 36709f2c024b8826bbcfd117543f3ef9b0a6e1d3 Mon Sep 17 00:00:00 2001 From: Nikita Mikhaylov Date: Fri, 17 Nov 2023 20:21:20 +0100 Subject: [PATCH 013/530] Update src/Interpreters/loadMetadata.cpp Co-authored-by: Nikolay Degterinsky <43110995+evillique@users.noreply.github.com> --- src/Interpreters/loadMetadata.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Interpreters/loadMetadata.cpp b/src/Interpreters/loadMetadata.cpp index 5b0666abfb4..76b2377be19 100644 --- a/src/Interpreters/loadMetadata.cpp +++ b/src/Interpreters/loadMetadata.cpp @@ -226,8 +226,10 @@ void loadMetadata(ContextMutablePtr context, const String & default_database_nam for (const auto & [name, db_path] : databases) { loadDatabase(context, name, db_path, has_force_restore_data_flag); - loaded_databases.insert({name, DatabaseCatalog::instance().getDatabase(name)}); + auto database = DatabaseCatalog::instance().getDatabase(name); + loaded_databases.insert({name, database}); + if (database->getEngineName() == "Ordinary") has_ordinary_databases = true; From aba54eeebcceee4b56ebe9ef5b68b2f1188ee62f Mon Sep 17 00:00:00 2001 From: shabroo <114589608+shabroo@users.noreply.github.com> Date: Mon, 20 Nov 2023 21:30:34 +0100 Subject: [PATCH 014/530] Update loadMetadata.cpp Fixing use of capitals in user message "Server has databases with Ordinary engine, which is not optimal." --- src/Interpreters/loadMetadata.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Interpreters/loadMetadata.cpp b/src/Interpreters/loadMetadata.cpp index 76b2377be19..48c4f13c026 100644 --- a/src/Interpreters/loadMetadata.cpp +++ b/src/Interpreters/loadMetadata.cpp @@ -238,7 +238,7 @@ void loadMetadata(ContextMutablePtr context, const String & default_database_nam } if (has_ordinary_databases) - context->addWarningMessage("Server Has Databases with Ordinary Engine which is not optimal. \n " + context->addWarningMessage("Server has databases with Ordinary engine, which is not optimal. \n " "To convert this database to a new Atomic engine, please create a flag named `convert_ordinary_to_atomic` in flags directory"); auto mode = getLoadingStrictnessLevel(/* attach */ true, /* force_attach */ true, has_force_restore_data_flag); From 600d293b6c49628a3c24a9afdccf09545491c158 Mon Sep 17 00:00:00 2001 From: shabroo <114589608+shabroo@users.noreply.github.com> Date: Wed, 22 Nov 2023 21:31:02 +0100 Subject: [PATCH 015/530] Update loadMetadata.cpp removed space at the beginning of the line --- src/Interpreters/loadMetadata.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Interpreters/loadMetadata.cpp b/src/Interpreters/loadMetadata.cpp index 48c4f13c026..16cf84da672 100644 --- a/src/Interpreters/loadMetadata.cpp +++ b/src/Interpreters/loadMetadata.cpp @@ -239,7 +239,7 @@ void loadMetadata(ContextMutablePtr context, const String & default_database_nam if (has_ordinary_databases) context->addWarningMessage("Server has databases with Ordinary engine, which is not optimal. \n " - "To convert this database to a new Atomic engine, please create a flag named `convert_ordinary_to_atomic` in flags directory"); + "To convert this database to a new Atomic engine, please create a flag named `convert_ordinary_to_atomic` in flags directory"); auto mode = getLoadingStrictnessLevel(/* attach */ true, /* force_attach */ true, has_force_restore_data_flag); TablesLoader loader{context, std::move(loaded_databases), mode}; From 524081570f697cb3e2ed4dc2e77703ac9bc6db43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9A=D0=B8=D1=80=D0=B8=D0=BB=D0=BB=20=D0=93=D0=B0=D1=80?= =?UTF-8?q?=D0=B1=D0=B0=D1=80?= Date: Tue, 12 Dec 2023 21:44:37 +0300 Subject: [PATCH 016/530] Convert merge tree to replicated on restart --- programs/server/Server.cpp | 2 + src/Interpreters/loadMetadata.cpp | 116 ++++++++++++++++++++++++++++++ src/Interpreters/loadMetadata.h | 4 ++ 3 files changed, 122 insertions(+) diff --git a/programs/server/Server.cpp b/programs/server/Server.cpp index 8076d108083..cd7e14d1596 100644 --- a/programs/server/Server.cpp +++ b/programs/server/Server.cpp @@ -1719,6 +1719,8 @@ try load_metadata_tasks = loadMetadata(global_context, default_database, server_settings.async_load_databases); /// If we need to convert database engines, disable async tables loading convertDatabasesEnginesIfNeed(load_metadata_tasks, global_context); + /// Convert MergeTree tables to replicated if flag is set + convertMergeTreeToReplicatedIfNeed(global_context); database_catalog.startupBackgroundTasks(); /// After loading validate that default database exists database_catalog.assertDatabaseExists(default_database); diff --git a/src/Interpreters/loadMetadata.cpp b/src/Interpreters/loadMetadata.cpp index 541f9c6ee89..1ef8694dcd0 100644 --- a/src/Interpreters/loadMetadata.cpp +++ b/src/Interpreters/loadMetadata.cpp @@ -4,7 +4,9 @@ #include #include #include +#include +#include #include #include #include @@ -14,6 +16,7 @@ #include #include #include +#include #include #include @@ -23,6 +26,9 @@ #include #include +#include +#include + #include #define ORDINARY_TO_ATOMIC_PREFIX ".tmp_convert." @@ -495,6 +501,116 @@ void convertDatabasesEnginesIfNeed(const LoadTaskPtrs & load_metadata, ContextMu fs::remove(convert_flag_path); } +static void convertMergeTreeToReplicated(Poco::Logger * log, ContextMutablePtr context, const DatabasePtr & database, StorageID id) +{ + ASTPtr as_create_ptr = database->getCreateTableQuery(id.table_name, context); + const auto & as_create = as_create_ptr->as(); + const auto & storage = as_create.storage->as(); + String storage_definition = queryToString(storage); + + /// Get storage definition + /// Set uuid explicitly, because it is forbidden to use the 'uuid' macro without ON CLUSTER + auto uuid = UUIDHelpers::generateV4(); + String replica_path = context->getConfigRef().getString("default_replica_path", "/clickhouse/tables/{uuid}/{shard}"); + replica_path = boost::algorithm::replace_all_copy(replica_path, "{uuid}", fmt::format("{}", uuid)); + String replica_name = context->getConfigRef().getString("default_replica_name", "{replica}"); + String replicated_args = fmt::format("('{}', '{}')", replica_path, replica_name); + String replicated_engine = "Replicated" + storage.engine->name + replicated_args; + + storage_definition = boost::algorithm::replace_first_copy(storage_definition, storage.engine->name, replicated_engine); + + /// Get names + String table_name = id.getTableName(); + String database_name = id.getDatabaseName(); + String qualified_quoted_name = id.getFullTableName(); + id.table_name = id.table_name + "_temp"; + String tmp_qualified_quoted_name = id.getFullTableName(); + + try + { + String create_table_query = fmt::format("CREATE TABLE {} UUID '{}' AS {} {}", tmp_qualified_quoted_name, uuid, qualified_quoted_name, storage_definition); + auto res = executeQuery(create_table_query, context, QueryFlags{ .internal = true }).second; + executeTrivialBlockIO(res, context); + + String exchange_tables_query = fmt::format("EXCHANGE TABLES {} AND {}", qualified_quoted_name, tmp_qualified_quoted_name); + res = executeQuery(exchange_tables_query, context, QueryFlags{ .internal = true }).second; + executeTrivialBlockIO(res, context); + + /// Get partition ids + String get_attach_queries_query = fmt::format("SELECT DISTINCT partition_id FROM system.parts WHERE table = '{}' AND database = '{}' AND active;", id.table_name, database_name); + WriteBufferFromOwnString buffer2; + ReadBufferFromOwnString buffer3 {std::move(get_attach_queries_query)}; + auto select_query_context = Context::createCopy(context); + select_query_context->makeQueryContext(); + select_query_context->setCurrentQueryId(""); + + executeQuery(buffer3, buffer2, false, select_query_context, {}, {.internal=true}); + + std::stringstream partition_ids_string{buffer2.str()}; + std::string line; + + /// Attach partitions + while (std::getline(partition_ids_string, line, '\n')) + { + String query3 = fmt::format("ALTER TABLE {} ATTACH PARTITION ID '{}' FROM {};", qualified_quoted_name, line, tmp_qualified_quoted_name); + executeQuery(query3, select_query_context, {.internal=true}); + } + + LOG_INFO(log, "Table {} is converted from MergeTree to replicated", qualified_quoted_name); + } + catch (Exception & e) + { + e.addMessage( + "Exception while trying to convert table {} from MergeTree to replicated. Tables may be in some intermediate state." + , qualified_quoted_name + ); + throw; + } +} + +static void findAndConvertMergeTreeTablesToReplicated(ContextMutablePtr context, const String & database_name) +{ + Poco::Logger * log = &Poco::Logger::get("loadMetadata"); + + auto database = DatabaseCatalog::instance().getDatabase(database_name); + if (!database) + { + LOG_WARNING(log, "Database {} not found (while trying to convert it's tables from MergeTree to ReplicatedMergeTree)", database_name); + return; + } + + auto local_context = Context::createCopy(context); + + for (auto iterator = database->getTablesIterator(context); iterator->isValid(); iterator->next()) + { + if (const auto * merge_tree = dynamic_cast(iterator->table().get())) + { + auto id = merge_tree->getStorageID(); + + /// Check if convert flag is set + auto convert_flag_path = fs::path(context->getPath()) / "data" / database_name / id.getTableName() / "flags" / "convert_to_replicated"; + if (fs::exists(convert_flag_path)) + { + LOG_INFO(log, "Will convert table {} from MergeTree to replicated", id.getFullTableName()); + convertMergeTreeToReplicated(log, local_context, database, id); + LOG_INFO(log, "Removing convert_to_replicated flag after convertation"); + convert_flag_path = fs::path(local_context->getPath()) / "data" / database_name / (id.getTableName() + "_temp") / "flags" / "convert_to_replicated"; + fs::remove(convert_flag_path); + } + } + } +} + +void convertMergeTreeToReplicatedIfNeed(ContextMutablePtr context) +{ + LOG_INFO(&Poco::Logger::get("loadMetadata"), "Start searching for MergeTree tables with convert_to_replicated flag"); + + for (const auto & [name, _] : DatabaseCatalog::instance().getDatabases()) + findAndConvertMergeTreeTablesToReplicated(context, name); + + LOG_INFO(&Poco::Logger::get("loadMetadata"), "All MergTree tables with convert_to_replicated flag are converted"); +} + LoadTaskPtrs loadMetadataSystem(ContextMutablePtr context) { loadSystemDatabaseImpl(context, DatabaseCatalog::SYSTEM_DATABASE, "Atomic"); diff --git a/src/Interpreters/loadMetadata.h b/src/Interpreters/loadMetadata.h index b0d97d53de3..80d960cfa60 100644 --- a/src/Interpreters/loadMetadata.h +++ b/src/Interpreters/loadMetadata.h @@ -26,4 +26,8 @@ void maybeConvertSystemDatabase(ContextMutablePtr context, LoadTaskPtrs & system /// Waits for `load_metadata` task before conversions void convertDatabasesEnginesIfNeed(const LoadTaskPtrs & load_metadata, ContextMutablePtr context); +/// Converts MergeTree tables to replicated if convert_to_replicated flag exists +/// Flag must be set at /clickhouse/data/database_name/table_name/flags/ +void convertMergeTreeToReplicatedIfNeed(ContextMutablePtr context); + } From a6549c106af54007ca28d369a8f5dde20a168485 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9A=D0=B8=D1=80=D0=B8=D0=BB=D0=BB=20=D0=93=D0=B0=D1=80?= =?UTF-8?q?=D0=B1=D0=B0=D1=80?= Date: Tue, 12 Dec 2023 21:45:00 +0300 Subject: [PATCH 017/530] Tests for engine convertation --- .../test_modify_engine_on_restart/__init__.py | 0 .../configs/config.d/clusters.xml | 22 ++ .../configs/config.d/distributed_ddl.xml | 5 + .../test_modify_engine_on_restart/test.py | 198 ++++++++++++++++++ .../test_modify_engine_on_restart/test_mv.py | 174 +++++++++++++++ 5 files changed, 399 insertions(+) create mode 100644 tests/integration/test_modify_engine_on_restart/__init__.py create mode 100644 tests/integration/test_modify_engine_on_restart/configs/config.d/clusters.xml create mode 100644 tests/integration/test_modify_engine_on_restart/configs/config.d/distributed_ddl.xml create mode 100644 tests/integration/test_modify_engine_on_restart/test.py create mode 100644 tests/integration/test_modify_engine_on_restart/test_mv.py diff --git a/tests/integration/test_modify_engine_on_restart/__init__.py b/tests/integration/test_modify_engine_on_restart/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/integration/test_modify_engine_on_restart/configs/config.d/clusters.xml b/tests/integration/test_modify_engine_on_restart/configs/config.d/clusters.xml new file mode 100644 index 00000000000..d3a9d4fb8f0 --- /dev/null +++ b/tests/integration/test_modify_engine_on_restart/configs/config.d/clusters.xml @@ -0,0 +1,22 @@ + + + + + true + + ch1 + 9000 + + + ch2 + 9000 + + + + + + + 01 + + + \ No newline at end of file diff --git a/tests/integration/test_modify_engine_on_restart/configs/config.d/distributed_ddl.xml b/tests/integration/test_modify_engine_on_restart/configs/config.d/distributed_ddl.xml new file mode 100644 index 00000000000..45555338de5 --- /dev/null +++ b/tests/integration/test_modify_engine_on_restart/configs/config.d/distributed_ddl.xml @@ -0,0 +1,5 @@ + + + /clickhouse/task_queue/ddl + + \ No newline at end of file diff --git a/tests/integration/test_modify_engine_on_restart/test.py b/tests/integration/test_modify_engine_on_restart/test.py new file mode 100644 index 00000000000..291b6859339 --- /dev/null +++ b/tests/integration/test_modify_engine_on_restart/test.py @@ -0,0 +1,198 @@ +import pytest +from helpers.cluster import ClickHouseCluster + +cluster = ClickHouseCluster(__file__) +ch1 = cluster.add_instance( + "ch1", + main_configs=[ + "configs/config.d/clusters.xml", + "configs/config.d/distributed_ddl.xml", + ], + with_zookeeper=True, + macros={"replica": "node1"}, + stay_alive=True, +) +ch2 = cluster.add_instance( + "ch2", + main_configs=[ + "configs/config.d/clusters.xml", + "configs/config.d/distributed_ddl.xml", + ], + with_zookeeper=True, + macros={"replica": "node2"}, +) + +database_name = "modify_engine" + +@pytest.fixture(scope="module") +def started_cluster(): + try: + cluster.start() + yield cluster + + finally: + cluster.shutdown() + +def q(node, query): + return node.query( + database=database_name, + sql=query + ) + +def create_tables(): + # MergeTree table that will be converted + q( + ch1, + "CREATE TABLE foo ( A Int64, D Date, S String ) ENGINE MergeTree() PARTITION BY toYYYYMM(D) ORDER BY A;" + ) + + q( + ch1, + "INSERT INTO foo SELECT number, today(), '' FROM numbers(1e6);" + ) + q( + ch1, + "INSERT INTO foo SELECT number, today()-60, '' FROM numbers(1e5);" + ) + + # ReplacingMergeTree table that will be converted to check unusual engine kinds + q( + ch1, + "CREATE TABLE replacing ( A Int64, D Date, S String ) ENGINE ReplacingMergeTree() PARTITION BY toYYYYMM(D) ORDER BY A;" + ) + + q( + ch1, + "INSERT INTO replacing SELECT number, today(), '' FROM numbers(1e6);" + ) + q( + ch1, + "INSERT INTO replacing SELECT number, today()-60, '' FROM numbers(1e5);" + ) + + # MergeTree table that will not be converted + q( + ch1, + "CREATE TABLE bar ( A Int64, D Date, S String ) ENGINE MergeTree() PARTITION BY toYYYYMM(D) ORDER BY A;" + ) + + # Not MergeTree table + q( + ch1, + "CREATE TABLE bar2 ( A Int64, D Date, S String ) ENGINE Log;" + ) + +def check_tables_not_converted(): + # Check tables exists + assert q( + ch1, + "SHOW TABLES", + ).strip() == "bar\nbar2\nfoo\nreplacing" + + # Check engines + assert q( + ch1, + f"SELECT name, engine FROM system.tables WHERE database = '{database_name}' AND (name LIKE 'foo%' OR name LIKE 'replacing%')", + ).strip() == "foo\tMergeTree\nreplacing\tReplacingMergeTree" + assert q( + ch1, + f"SELECT name, engine FROM system.tables WHERE database = '{database_name}' AND (name LIKE 'bar%')", + ).strip() == "bar\tMergeTree\nbar2\tLog" + + # Check values + for table in ["foo", "replacing"]: + assert q( + ch1, + f"SELECT count() FROM {table}", + ).strip() == "1100000" + +def check_tables_converted(): + # Check tables exists + assert q( + ch1, + "SHOW TABLES", + ).strip() == "bar\nbar2\nfoo\nfoo_temp\nreplacing\nreplacing_temp" + + # Check engines + assert q( + ch1, + f"SELECT name, engine FROM system.tables WHERE database = '{database_name}' AND (name LIKE 'foo%' OR name LIKE 'replacing%')", + ).strip() == "foo\tReplicatedMergeTree\nfoo_temp\tMergeTree\nreplacing\tReplicatedReplacingMergeTree\nreplacing_temp\tReplacingMergeTree" + assert q( + ch1, + f"SELECT name, engine FROM system.tables WHERE database = '{database_name}' AND (name LIKE 'bar%')", + ).strip() == "bar\tMergeTree\nbar2\tLog" + + # Check values + for table in ["foo", "replacing"]: + assert q( + ch1, + f"SELECT count() FROM {table}", + ).strip() == "1100000" + assert q( + ch1, + f"SELECT count() FROM {table}_temp", + ).strip() == "1100000" + +def set_convert_flags(): + + # Set convert flag on actually convertable tables + for table in ["foo", "replacing"]: + ch1.exec_in_container( + ["bash", "-c", f"mkdir /var/lib/clickhouse/data/{database_name}/{table}/flags"] + ) + ch1.exec_in_container( + ["bash", "-c", f"touch /var/lib/clickhouse/data/{database_name}/{table}/flags/convert_to_replicated"] + ) + + # Set flag to not MergeTree table to check that nothing happens + ch1.exec_in_container( + ["bash", "-c", f"mkdir /var/lib/clickhouse/data/{database_name}/bar2/flags"] + ) + ch1.exec_in_container( + ["bash", "-c", f"touch /var/lib/clickhouse/data/{database_name}/bar2/flags/convert_to_replicated"] + ) + +def check_replica_added(): + # Add replica to check if zookeeper path is correct and consistent with table uuid + + uuid = q( + ch1, + f"SELECT uuid FROM system.tables WHERE table = 'foo' AND database = '{database_name}'" + ).strip() + + q( + ch2, + f"CREATE TABLE foo ( A Int64, D Date, S String ) ENGINE ReplicatedMergeTree('/clickhouse/tables/{uuid}/{{shard}}', '{{replica}}') PARTITION BY toYYYYMM(D) ORDER BY A" + ) + + ch2.query(database=database_name, sql="SYSTEM SYNC REPLICA foo", timeout=20) + + # Check values + assert q( + ch2, + f"SELECT count() FROM foo", + ).strip() == "1100000" + +def test_modify_engine_on_restart(started_cluster): + ch1.query("CREATE DATABASE " + database_name + " ON CLUSTER cluster") + assert q( + ch1, + "SHOW TABLES" + ).strip() == "" + + create_tables() + + check_tables_not_converted() + + ch1.restart_clickhouse() + + check_tables_not_converted() + + set_convert_flags() + + ch1.restart_clickhouse() + + check_tables_converted() + + check_replica_added() diff --git a/tests/integration/test_modify_engine_on_restart/test_mv.py b/tests/integration/test_modify_engine_on_restart/test_mv.py new file mode 100644 index 00000000000..cba815a34a1 --- /dev/null +++ b/tests/integration/test_modify_engine_on_restart/test_mv.py @@ -0,0 +1,174 @@ +import pytest +from helpers.cluster import ClickHouseCluster + +cluster = ClickHouseCluster(__file__) +ch1 = cluster.add_instance( + "ch1", + main_configs=[ + "configs/config.d/clusters.xml", + "configs/config.d/distributed_ddl.xml", + ], + with_zookeeper=True, + macros={"replica": "node1"}, + stay_alive=True, +) + +@pytest.fixture(scope="module") +def started_cluster(): + try: + cluster.start() + yield cluster + + finally: + cluster.shutdown() + +def q(node, database, query): + return node.query( + database=database, + sql=query + ) + +def create_tables(database_name): + q( + ch1, + database_name, + "CREATE TABLE hourly_data(`domain_name` String, `event_time` DateTime, `count_views` UInt64) ENGINE = MergeTree ORDER BY (domain_name, event_time)" + ) + + q( + ch1, + database_name, + "CREATE TABLE monthly_aggregated_data\ + (`domain_name` String, `month` Date, `sumCountViews` AggregateFunction(sum, UInt64))\ + ENGINE = AggregatingMergeTree ORDER BY (domain_name, month)" + ) + + q( + ch1, + database_name, + "CREATE MATERIALIZED VIEW monthly_aggregated_data_mv\ + TO monthly_aggregated_data\ + AS\ + SELECT\ + toDate(toStartOfMonth(event_time)) AS month,\ + domain_name,\ + sumState(count_views) AS sumCountViews\ + FROM hourly_data\ + GROUP BY\ + domain_name,\ + month" + ) + + q( + ch1, + database_name, + "INSERT INTO hourly_data (domain_name, event_time, count_views)\ + VALUES ('clickhouse.com', '2019-01-01 10:00:00', 1),\ + ('clickhouse.com', '2019-02-02 00:00:00', 2),\ + ('clickhouse.com', '2019-02-01 00:00:00', 3),\ + ('clickhouse.com', '2020-01-01 00:00:00', 6)" + ) + +def check_tables_not_converted(database_name): + # Check engines + assert q( + ch1, + database_name, + f"SELECT name, engine FROM system.tables WHERE database = '{database_name}'", + ).strip() == "hourly_data\tMergeTree\nmonthly_aggregated_data\tAggregatingMergeTree\nmonthly_aggregated_data_mv\tMaterializedView" + + # Check values + assert q( + ch1, + database_name, + "SELECT sumMerge(sumCountViews) as sumCountViews\ + FROM monthly_aggregated_data_mv" + ).strip() == "12" + assert q( + ch1, + database_name, + "SELECT count() FROM hourly_data" + ).strip() == "4" + +def check_tables_converted(database_name): + # Check engines + assert q( + ch1, + database_name, + f"SELECT name, engine FROM system.tables WHERE database = '{database_name}' AND name NOT LIKE '%_temp'", + ).strip() == "hourly_data\tReplicatedMergeTree\nmonthly_aggregated_data\tReplicatedAggregatingMergeTree\nmonthly_aggregated_data_mv\tMaterializedView" + assert q( + ch1, + database_name, + f"SELECT name, engine FROM system.tables WHERE database = '{database_name}' AND name LIKE '%_temp'", + ).strip() == "hourly_data_temp\tMergeTree\nmonthly_aggregated_data_temp\tAggregatingMergeTree" + + # Check values + assert q( + ch1, + database_name, + "SELECT sumMerge(sumCountViews) as sumCountViews\ + FROM monthly_aggregated_data_mv" + ).strip() == "12" + assert q( + ch1, + database_name, + "SELECT count() FROM hourly_data" + ).strip() == "4" + + # Insert new values to check if new dependencies are set correctly + q( + ch1, + database_name, + "INSERT INTO hourly_data (domain_name, event_time, count_views)\ + VALUES ('clickhouse.com', '2019-01-01 10:00:00', 1),\ + ('clickhouse.com', '2019-02-02 00:00:00', 2),\ + ('clickhouse.com', '2019-02-01 00:00:00', 3),\ + ('clickhouse.com', '2020-01-01 00:00:00', 6)" + ) + + assert q( + ch1, + database_name, + "SELECT sumMerge(sumCountViews) as sumCountViews\ + FROM monthly_aggregated_data_mv" + ).strip() == "24" + assert q( + ch1, + database_name, + "SELECT count() FROM hourly_data" + ).strip() == "8" + +def set_convert_flags(database_name): + + for table in ["hourly_data", "monthly_aggregated_data"]: + ch1.exec_in_container( + ["bash", "-c", f"mkdir /var/lib/clickhouse/data/{database_name}/{table}/flags"] + ) + ch1.exec_in_container( + ["bash", "-c", f"touch /var/lib/clickhouse/data/{database_name}/{table}/flags/convert_to_replicated"] + ) + +def test_modify_engine_on_restart_with_materialized_view(started_cluster): + database_name = "modify_engine_with_mv" + q( + ch1, + "default", + f"CREATE DATABASE {database_name}", + ) + assert q( + ch1, + database_name, + "SHOW TABLES" + ).strip() == "" + + create_tables(database_name) + + check_tables_not_converted(database_name) + + set_convert_flags(database_name) + + ch1.restart_clickhouse() + + check_tables_converted(database_name) + \ No newline at end of file From 8d1dfebb4f900fd8ef65b05166315a65eb803dcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9A=D0=B8=D1=80=D0=B8=D0=BB=D0=BB=20=D0=93=D0=B0=D1=80?= =?UTF-8?q?=D0=B1=D0=B0=D1=80?= Date: Thu, 14 Dec 2023 23:01:50 +0300 Subject: [PATCH 018/530] Refactor tests --- .../test_modify_engine_on_restart/test.py | 87 +++++--------- .../test_modify_engine_on_restart/test_mv.py | 111 ++++++------------ 2 files changed, 68 insertions(+), 130 deletions(-) diff --git a/tests/integration/test_modify_engine_on_restart/test.py b/tests/integration/test_modify_engine_on_restart/test.py index 291b6859339..6c82cb93a4d 100644 --- a/tests/integration/test_modify_engine_on_restart/test.py +++ b/tests/integration/test_modify_engine_on_restart/test.py @@ -43,16 +43,16 @@ def create_tables(): # MergeTree table that will be converted q( ch1, - "CREATE TABLE foo ( A Int64, D Date, S String ) ENGINE MergeTree() PARTITION BY toYYYYMM(D) ORDER BY A;" + "CREATE TABLE rmt ( A Int64, D Date, S String ) ENGINE MergeTree() PARTITION BY toYYYYMM(D) ORDER BY A;" ) q( ch1, - "INSERT INTO foo SELECT number, today(), '' FROM numbers(1e6);" + "INSERT INTO rmt SELECT number, today(), '' FROM numbers(1e6);" ) q( ch1, - "INSERT INTO foo SELECT number, today()-60, '' FROM numbers(1e5);" + "INSERT INTO rmt SELECT number, today()-60, '' FROM numbers(1e5);" ) # ReplacingMergeTree table that will be converted to check unusual engine kinds @@ -73,71 +73,48 @@ def create_tables(): # MergeTree table that will not be converted q( ch1, - "CREATE TABLE bar ( A Int64, D Date, S String ) ENGINE MergeTree() PARTITION BY toYYYYMM(D) ORDER BY A;" + "CREATE TABLE mt ( A Int64, D Date, S String ) ENGINE MergeTree() PARTITION BY toYYYYMM(D) ORDER BY A;" ) # Not MergeTree table q( ch1, - "CREATE TABLE bar2 ( A Int64, D Date, S String ) ENGINE Log;" + "CREATE TABLE log ( A Int64, D Date, S String ) ENGINE Log;" ) -def check_tables_not_converted(): +def check_tables(converted): + engine_prefix = "" + if (converted): + engine_prefix = "Replicated" + # Check tables exists assert q( ch1, "SHOW TABLES", - ).strip() == "bar\nbar2\nfoo\nreplacing" + ).strip() == "log\nmt\nreplacing\nrmt" # Check engines assert q( ch1, - f"SELECT name, engine FROM system.tables WHERE database = '{database_name}' AND (name LIKE 'foo%' OR name LIKE 'replacing%')", - ).strip() == "foo\tMergeTree\nreplacing\tReplacingMergeTree" + f"SELECT name, engine FROM system.tables WHERE database = '{database_name}' AND (name != 'log' AND name != 'mt')", + ).strip() == f"replacing\t{engine_prefix}ReplacingMergeTree\nrmt\t{engine_prefix}MergeTree" assert q( ch1, - f"SELECT name, engine FROM system.tables WHERE database = '{database_name}' AND (name LIKE 'bar%')", - ).strip() == "bar\tMergeTree\nbar2\tLog" + f"SELECT name, engine FROM system.tables WHERE database = '{database_name}' AND (name = 'log' OR name = 'mt')", + ).strip() == "log\tLog\nmt\tMergeTree" # Check values - for table in ["foo", "replacing"]: + for table in ["rmt", "replacing"]: + assert q( ch1, f"SELECT count() FROM {table}", ).strip() == "1100000" -def check_tables_converted(): - # Check tables exists - assert q( - ch1, - "SHOW TABLES", - ).strip() == "bar\nbar2\nfoo\nfoo_temp\nreplacing\nreplacing_temp" - - # Check engines - assert q( - ch1, - f"SELECT name, engine FROM system.tables WHERE database = '{database_name}' AND (name LIKE 'foo%' OR name LIKE 'replacing%')", - ).strip() == "foo\tReplicatedMergeTree\nfoo_temp\tMergeTree\nreplacing\tReplicatedReplacingMergeTree\nreplacing_temp\tReplacingMergeTree" - assert q( - ch1, - f"SELECT name, engine FROM system.tables WHERE database = '{database_name}' AND (name LIKE 'bar%')", - ).strip() == "bar\tMergeTree\nbar2\tLog" - - # Check values - for table in ["foo", "replacing"]: - assert q( - ch1, - f"SELECT count() FROM {table}", - ).strip() == "1100000" - assert q( - ch1, - f"SELECT count() FROM {table}_temp", - ).strip() == "1100000" - def set_convert_flags(): # Set convert flag on actually convertable tables - for table in ["foo", "replacing"]: + for table in ["rmt", "replacing"]: ch1.exec_in_container( ["bash", "-c", f"mkdir /var/lib/clickhouse/data/{database_name}/{table}/flags"] ) @@ -147,10 +124,10 @@ def set_convert_flags(): # Set flag to not MergeTree table to check that nothing happens ch1.exec_in_container( - ["bash", "-c", f"mkdir /var/lib/clickhouse/data/{database_name}/bar2/flags"] + ["bash", "-c", f"mkdir /var/lib/clickhouse/data/{database_name}/log/flags"] ) ch1.exec_in_container( - ["bash", "-c", f"touch /var/lib/clickhouse/data/{database_name}/bar2/flags/convert_to_replicated"] + ["bash", "-c", f"touch /var/lib/clickhouse/data/{database_name}/log/flags/convert_to_replicated"] ) def check_replica_added(): @@ -158,41 +135,41 @@ def check_replica_added(): uuid = q( ch1, - f"SELECT uuid FROM system.tables WHERE table = 'foo' AND database = '{database_name}'" + f"SELECT uuid FROM system.tables WHERE table = 'rmt' AND database = '{database_name}'" ).strip() q( ch2, - f"CREATE TABLE foo ( A Int64, D Date, S String ) ENGINE ReplicatedMergeTree('/clickhouse/tables/{uuid}/{{shard}}', '{{replica}}') PARTITION BY toYYYYMM(D) ORDER BY A" + f"CREATE TABLE rmt ( A Int64, D Date, S String ) ENGINE ReplicatedMergeTree('/clickhouse/tables/{uuid}/{{shard}}', '{{replica}}') PARTITION BY toYYYYMM(D) ORDER BY A" ) - ch2.query(database=database_name, sql="SYSTEM SYNC REPLICA foo", timeout=20) + ch2.query(database=database_name, sql="SYSTEM SYNC REPLICA rmt", timeout=20) # Check values assert q( ch2, - f"SELECT count() FROM foo", + f"SELECT count() FROM rmt", ).strip() == "1100000" def test_modify_engine_on_restart(started_cluster): ch1.query("CREATE DATABASE " + database_name + " ON CLUSTER cluster") - assert q( - ch1, - "SHOW TABLES" - ).strip() == "" - + create_tables() - check_tables_not_converted() + check_tables(False) ch1.restart_clickhouse() - check_tables_not_converted() + check_tables(False) set_convert_flags() ch1.restart_clickhouse() - check_tables_converted() + check_tables(True) check_replica_added() + + ch1.restart_clickhouse() + + check_tables(True) \ No newline at end of file diff --git a/tests/integration/test_modify_engine_on_restart/test_mv.py b/tests/integration/test_modify_engine_on_restart/test_mv.py index cba815a34a1..c9c0c3d19c9 100644 --- a/tests/integration/test_modify_engine_on_restart/test_mv.py +++ b/tests/integration/test_modify_engine_on_restart/test_mv.py @@ -13,6 +13,8 @@ ch1 = cluster.add_instance( stay_alive=True, ) +database_name = "modify_engine_with_mv" + @pytest.fixture(scope="module") def started_cluster(): try: @@ -22,22 +24,20 @@ def started_cluster(): finally: cluster.shutdown() -def q(node, database, query): +def q(node, query): return node.query( - database=database, + database=database_name, sql=query ) -def create_tables(database_name): +def create_tables(): q( ch1, - database_name, "CREATE TABLE hourly_data(`domain_name` String, `event_time` DateTime, `count_views` UInt64) ENGINE = MergeTree ORDER BY (domain_name, event_time)" ) q( ch1, - database_name, "CREATE TABLE monthly_aggregated_data\ (`domain_name` String, `month` Date, `sumCountViews` AggregateFunction(sum, UInt64))\ ENGINE = AggregatingMergeTree ORDER BY (domain_name, month)" @@ -45,7 +45,6 @@ def create_tables(database_name): q( ch1, - database_name, "CREATE MATERIALIZED VIEW monthly_aggregated_data_mv\ TO monthly_aggregated_data\ AS\ @@ -61,7 +60,6 @@ def create_tables(database_name): q( ch1, - database_name, "INSERT INTO hourly_data (domain_name, event_time, count_views)\ VALUES ('clickhouse.com', '2019-01-01 10:00:00', 1),\ ('clickhouse.com', '2019-02-02 00:00:00', 2),\ @@ -69,77 +67,50 @@ def create_tables(database_name): ('clickhouse.com', '2020-01-01 00:00:00', 6)" ) -def check_tables_not_converted(database_name): +def check_tables(converted): + engine_prefix = "" + if (converted): + engine_prefix = "Replicated" + # Check engines assert q( ch1, - database_name, f"SELECT name, engine FROM system.tables WHERE database = '{database_name}'", - ).strip() == "hourly_data\tMergeTree\nmonthly_aggregated_data\tAggregatingMergeTree\nmonthly_aggregated_data_mv\tMaterializedView" + ).strip() == f"hourly_data\t{engine_prefix}MergeTree\nmonthly_aggregated_data\t{engine_prefix}AggregatingMergeTree\nmonthly_aggregated_data_mv\tMaterializedView" # Check values assert q( ch1, - database_name, "SELECT sumMerge(sumCountViews) as sumCountViews\ FROM monthly_aggregated_data_mv" ).strip() == "12" assert q( ch1, - database_name, "SELECT count() FROM hourly_data" ).strip() == "4" -def check_tables_converted(database_name): - # Check engines - assert q( - ch1, - database_name, - f"SELECT name, engine FROM system.tables WHERE database = '{database_name}' AND name NOT LIKE '%_temp'", - ).strip() == "hourly_data\tReplicatedMergeTree\nmonthly_aggregated_data\tReplicatedAggregatingMergeTree\nmonthly_aggregated_data_mv\tMaterializedView" - assert q( - ch1, - database_name, - f"SELECT name, engine FROM system.tables WHERE database = '{database_name}' AND name LIKE '%_temp'", - ).strip() == "hourly_data_temp\tMergeTree\nmonthly_aggregated_data_temp\tAggregatingMergeTree" + if (converted): + # Insert new values to check if new dependencies are set correctly + q( + ch1, + "INSERT INTO hourly_data (domain_name, event_time, count_views)\ + VALUES ('clickhouse.com', '2019-01-01 10:00:00', 1),\ + ('clickhouse.com', '2019-02-02 00:00:00', 2),\ + ('clickhouse.com', '2019-02-01 00:00:00', 3),\ + ('clickhouse.com', '2020-01-01 00:00:00', 6)" + ) - # Check values - assert q( - ch1, - database_name, - "SELECT sumMerge(sumCountViews) as sumCountViews\ - FROM monthly_aggregated_data_mv" - ).strip() == "12" - assert q( - ch1, - database_name, - "SELECT count() FROM hourly_data" - ).strip() == "4" + assert q( + ch1, + "SELECT sumMerge(sumCountViews) as sumCountViews\ + FROM monthly_aggregated_data_mv" + ).strip() == "24" + assert q( + ch1, + "SELECT count() FROM hourly_data" + ).strip() == "8" - # Insert new values to check if new dependencies are set correctly - q( - ch1, - database_name, - "INSERT INTO hourly_data (domain_name, event_time, count_views)\ - VALUES ('clickhouse.com', '2019-01-01 10:00:00', 1),\ - ('clickhouse.com', '2019-02-02 00:00:00', 2),\ - ('clickhouse.com', '2019-02-01 00:00:00', 3),\ - ('clickhouse.com', '2020-01-01 00:00:00', 6)" - ) - - assert q( - ch1, - database_name, - "SELECT sumMerge(sumCountViews) as sumCountViews\ - FROM monthly_aggregated_data_mv" - ).strip() == "24" - assert q( - ch1, - database_name, - "SELECT count() FROM hourly_data" - ).strip() == "8" - -def set_convert_flags(database_name): +def set_convert_flags(): for table in ["hourly_data", "monthly_aggregated_data"]: ch1.exec_in_container( @@ -150,25 +121,15 @@ def set_convert_flags(database_name): ) def test_modify_engine_on_restart_with_materialized_view(started_cluster): - database_name = "modify_engine_with_mv" - q( - ch1, - "default", - f"CREATE DATABASE {database_name}", - ) - assert q( - ch1, - database_name, - "SHOW TABLES" - ).strip() == "" + ch1.query(f"CREATE DATABASE {database_name}") - create_tables(database_name) + create_tables() - check_tables_not_converted(database_name) + check_tables(False) - set_convert_flags(database_name) + set_convert_flags() ch1.restart_clickhouse() - check_tables_converted(database_name) + check_tables(True) \ No newline at end of file From 8623a3044802ec6d701a7b82008d14ff46b7594c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9A=D0=B8=D1=80=D0=B8=D0=BB=D0=BB=20=D0=93=D0=B0=D1=80?= =?UTF-8?q?=D0=B1=D0=B0=D1=80?= Date: Thu, 14 Dec 2023 23:19:39 +0300 Subject: [PATCH 019/530] Load MT table as RMT instead of creating new one --- programs/server/Server.cpp | 2 - src/Databases/DatabaseOrdinary.cpp | 87 ++++++++++++++++++++++ src/Interpreters/loadMetadata.cpp | 116 ----------------------------- src/Interpreters/loadMetadata.h | 4 - 4 files changed, 87 insertions(+), 122 deletions(-) diff --git a/programs/server/Server.cpp b/programs/server/Server.cpp index cd7e14d1596..8076d108083 100644 --- a/programs/server/Server.cpp +++ b/programs/server/Server.cpp @@ -1719,8 +1719,6 @@ try load_metadata_tasks = loadMetadata(global_context, default_database, server_settings.async_load_databases); /// If we need to convert database engines, disable async tables loading convertDatabasesEnginesIfNeed(load_metadata_tasks, global_context); - /// Convert MergeTree tables to replicated if flag is set - convertMergeTreeToReplicatedIfNeed(global_context); database_catalog.startupBackgroundTasks(); /// After loading validate that default database exists database_catalog.assertDatabaseExists(default_database); diff --git a/src/Databases/DatabaseOrdinary.cpp b/src/Databases/DatabaseOrdinary.cpp index 1f344551c5e..f9c9da9c206 100644 --- a/src/Databases/DatabaseOrdinary.cpp +++ b/src/Databases/DatabaseOrdinary.cpp @@ -28,6 +28,10 @@ #include #include #include +#include "Core/Defines.h" +#include + +#include namespace fs = std::filesystem; @@ -107,6 +111,59 @@ void DatabaseOrdinary::loadTablesMetadata(ContextPtr local_context, ParsedTables QualifiedTableName qualified_name{TSA_SUPPRESS_WARNING_FOR_READ(database_name), create_query->getTable()}; + if (create_query->storage && create_query->storage->engine->name.ends_with("MergeTree") && !create_query->storage->engine->name.starts_with("Replicated")) + { + auto convert_to_replicated_flag_path = fs::path(getContext()->getPath()) / "data" / qualified_name.database / qualified_name.table / "flags" / "convert_to_replicated"; + + LOG_INFO(log, "Searching for convert_to_replicated flag at {}.", backQuote(convert_to_replicated_flag_path.string())); + + if (fs::exists(convert_to_replicated_flag_path)) + { + LOG_INFO(log, "Found convert_to_replicated flag for table {}. Will try to load it as replicated table.", backQuote(qualified_name.getFullName())); + + /// Get storage definition + /// Set uuid explicitly, because it is forbidden to use the 'uuid' macro without ON CLUSTER + auto * storage = create_query->storage->as(); + String storage_definition = queryToString(*storage); + + String replica_path = getContext()->getConfigRef().getString("default_replica_path", "/clickhouse/tables/{uuid}/{shard}"); + replica_path = boost::algorithm::replace_all_copy(replica_path, "{uuid}", fmt::format("{}", create_query->uuid)); + String replica_name = getContext()->getConfigRef().getString("default_replica_name", "{replica}"); + String replicated_args = fmt::format("('{}', '{}')", replica_path, replica_name); + String replicated_engine = "Replicated" + storage->engine->name + replicated_args; + + String create_query_string = queryToString(*ast); + + create_query_string = boost::algorithm::replace_first_copy(create_query_string, storage->engine->name, replicated_engine); + + ParserCreateQuery parser_create_query; + auto new_ast = parseQuery(parser_create_query, create_query_string, 0, DBMS_DEFAULT_MAX_PARSER_DEPTH); + + ast = new_ast; + + /// Write changes to metadata + String table_metadata_path = full_path; + String table_metadata_tmp_path = table_metadata_path + ".tmp"; + String statement = getObjectDefinitionFromCreateQuery(ast); + { + WriteBufferFromFile out(table_metadata_tmp_path, statement.size(), O_WRONLY | O_CREAT | O_EXCL); + writeString(statement, out); + out.next(); + if (getContext()->getSettingsRef().fsync_metadata) + out.sync(); + out.close(); + } + fs::rename(table_metadata_tmp_path, table_metadata_path); + + LOG_INFO + ( + log, + "Table {} is loaded as replicated. Not removing convert_to_replicated flag until metadata in zookeeper is restored.", + backQuote(qualified_name.getFullName()) + ); + } + } + std::lock_guard lock{metadata.mutex}; metadata.parsed_tables[qualified_name] = ParsedTableMetadata{full_path.string(), ast}; metadata.total_dictionaries += create_query->is_dictionary; @@ -208,6 +265,36 @@ LoadTaskPtr DatabaseOrdinary::startupTableAsync( /// until startup finished. auto table_lock_holder = table->lockForShare(RWLockImpl::NO_QUERY, getContext()->getSettingsRef().lock_acquire_timeout); table->startup(); + + /// If table is ReplicatedMergeTree after conversion from MergeTree, + /// it is in readonly mode due to metadata in zookeeper missing. + if (auto * rmt = table->as()) + { + auto convert_to_replicated_flag_path = fs::path(getContext()->getPath()) / "data" / name.database / name.table / "flags" / "convert_to_replicated"; + if (fs::exists(convert_to_replicated_flag_path)) + { + if (rmt->isTableReadOnly()) + { + rmt->restoreMetadataInZooKeeper(); + LOG_INFO + ( + log, + "Metadata in zookeeper for {} is restored. Removing convert_to_replicated flag.", + backQuote(name.getFullName()) + ); + } + else + { + LOG_INFO + ( + log, + "Table {} is not in readonly mode but convert_to_replicated flag is set. Removing flag.", + backQuote(name.getFullName()) + ); + } + fs::remove(convert_to_replicated_flag_path); + } + } logAboutProgress(log, ++tables_started, total_tables_to_startup, startup_watch); } else diff --git a/src/Interpreters/loadMetadata.cpp b/src/Interpreters/loadMetadata.cpp index 1ef8694dcd0..541f9c6ee89 100644 --- a/src/Interpreters/loadMetadata.cpp +++ b/src/Interpreters/loadMetadata.cpp @@ -4,9 +4,7 @@ #include #include #include -#include -#include #include #include #include @@ -16,7 +14,6 @@ #include #include #include -#include #include #include @@ -26,9 +23,6 @@ #include #include -#include -#include - #include #define ORDINARY_TO_ATOMIC_PREFIX ".tmp_convert." @@ -501,116 +495,6 @@ void convertDatabasesEnginesIfNeed(const LoadTaskPtrs & load_metadata, ContextMu fs::remove(convert_flag_path); } -static void convertMergeTreeToReplicated(Poco::Logger * log, ContextMutablePtr context, const DatabasePtr & database, StorageID id) -{ - ASTPtr as_create_ptr = database->getCreateTableQuery(id.table_name, context); - const auto & as_create = as_create_ptr->as(); - const auto & storage = as_create.storage->as(); - String storage_definition = queryToString(storage); - - /// Get storage definition - /// Set uuid explicitly, because it is forbidden to use the 'uuid' macro without ON CLUSTER - auto uuid = UUIDHelpers::generateV4(); - String replica_path = context->getConfigRef().getString("default_replica_path", "/clickhouse/tables/{uuid}/{shard}"); - replica_path = boost::algorithm::replace_all_copy(replica_path, "{uuid}", fmt::format("{}", uuid)); - String replica_name = context->getConfigRef().getString("default_replica_name", "{replica}"); - String replicated_args = fmt::format("('{}', '{}')", replica_path, replica_name); - String replicated_engine = "Replicated" + storage.engine->name + replicated_args; - - storage_definition = boost::algorithm::replace_first_copy(storage_definition, storage.engine->name, replicated_engine); - - /// Get names - String table_name = id.getTableName(); - String database_name = id.getDatabaseName(); - String qualified_quoted_name = id.getFullTableName(); - id.table_name = id.table_name + "_temp"; - String tmp_qualified_quoted_name = id.getFullTableName(); - - try - { - String create_table_query = fmt::format("CREATE TABLE {} UUID '{}' AS {} {}", tmp_qualified_quoted_name, uuid, qualified_quoted_name, storage_definition); - auto res = executeQuery(create_table_query, context, QueryFlags{ .internal = true }).second; - executeTrivialBlockIO(res, context); - - String exchange_tables_query = fmt::format("EXCHANGE TABLES {} AND {}", qualified_quoted_name, tmp_qualified_quoted_name); - res = executeQuery(exchange_tables_query, context, QueryFlags{ .internal = true }).second; - executeTrivialBlockIO(res, context); - - /// Get partition ids - String get_attach_queries_query = fmt::format("SELECT DISTINCT partition_id FROM system.parts WHERE table = '{}' AND database = '{}' AND active;", id.table_name, database_name); - WriteBufferFromOwnString buffer2; - ReadBufferFromOwnString buffer3 {std::move(get_attach_queries_query)}; - auto select_query_context = Context::createCopy(context); - select_query_context->makeQueryContext(); - select_query_context->setCurrentQueryId(""); - - executeQuery(buffer3, buffer2, false, select_query_context, {}, {.internal=true}); - - std::stringstream partition_ids_string{buffer2.str()}; - std::string line; - - /// Attach partitions - while (std::getline(partition_ids_string, line, '\n')) - { - String query3 = fmt::format("ALTER TABLE {} ATTACH PARTITION ID '{}' FROM {};", qualified_quoted_name, line, tmp_qualified_quoted_name); - executeQuery(query3, select_query_context, {.internal=true}); - } - - LOG_INFO(log, "Table {} is converted from MergeTree to replicated", qualified_quoted_name); - } - catch (Exception & e) - { - e.addMessage( - "Exception while trying to convert table {} from MergeTree to replicated. Tables may be in some intermediate state." - , qualified_quoted_name - ); - throw; - } -} - -static void findAndConvertMergeTreeTablesToReplicated(ContextMutablePtr context, const String & database_name) -{ - Poco::Logger * log = &Poco::Logger::get("loadMetadata"); - - auto database = DatabaseCatalog::instance().getDatabase(database_name); - if (!database) - { - LOG_WARNING(log, "Database {} not found (while trying to convert it's tables from MergeTree to ReplicatedMergeTree)", database_name); - return; - } - - auto local_context = Context::createCopy(context); - - for (auto iterator = database->getTablesIterator(context); iterator->isValid(); iterator->next()) - { - if (const auto * merge_tree = dynamic_cast(iterator->table().get())) - { - auto id = merge_tree->getStorageID(); - - /// Check if convert flag is set - auto convert_flag_path = fs::path(context->getPath()) / "data" / database_name / id.getTableName() / "flags" / "convert_to_replicated"; - if (fs::exists(convert_flag_path)) - { - LOG_INFO(log, "Will convert table {} from MergeTree to replicated", id.getFullTableName()); - convertMergeTreeToReplicated(log, local_context, database, id); - LOG_INFO(log, "Removing convert_to_replicated flag after convertation"); - convert_flag_path = fs::path(local_context->getPath()) / "data" / database_name / (id.getTableName() + "_temp") / "flags" / "convert_to_replicated"; - fs::remove(convert_flag_path); - } - } - } -} - -void convertMergeTreeToReplicatedIfNeed(ContextMutablePtr context) -{ - LOG_INFO(&Poco::Logger::get("loadMetadata"), "Start searching for MergeTree tables with convert_to_replicated flag"); - - for (const auto & [name, _] : DatabaseCatalog::instance().getDatabases()) - findAndConvertMergeTreeTablesToReplicated(context, name); - - LOG_INFO(&Poco::Logger::get("loadMetadata"), "All MergTree tables with convert_to_replicated flag are converted"); -} - LoadTaskPtrs loadMetadataSystem(ContextMutablePtr context) { loadSystemDatabaseImpl(context, DatabaseCatalog::SYSTEM_DATABASE, "Atomic"); diff --git a/src/Interpreters/loadMetadata.h b/src/Interpreters/loadMetadata.h index 80d960cfa60..b0d97d53de3 100644 --- a/src/Interpreters/loadMetadata.h +++ b/src/Interpreters/loadMetadata.h @@ -26,8 +26,4 @@ void maybeConvertSystemDatabase(ContextMutablePtr context, LoadTaskPtrs & system /// Waits for `load_metadata` task before conversions void convertDatabasesEnginesIfNeed(const LoadTaskPtrs & load_metadata, ContextMutablePtr context); -/// Converts MergeTree tables to replicated if convert_to_replicated flag exists -/// Flag must be set at /clickhouse/data/database_name/table_name/flags/ -void convertMergeTreeToReplicatedIfNeed(ContextMutablePtr context); - } From e000e0b561b9645de218f6678f05359bb6a15d93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9A=D0=B8=D1=80=D0=B8=D0=BB=D0=BB=20=D0=93=D0=B0=D1=80?= =?UTF-8?q?=D0=B1=D0=B0=D1=80?= Date: Thu, 14 Dec 2023 23:21:03 +0300 Subject: [PATCH 020/530] Quotes in include --- src/Databases/DatabaseOrdinary.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Databases/DatabaseOrdinary.cpp b/src/Databases/DatabaseOrdinary.cpp index f9c9da9c206..03c0febcfc4 100644 --- a/src/Databases/DatabaseOrdinary.cpp +++ b/src/Databases/DatabaseOrdinary.cpp @@ -28,7 +28,7 @@ #include #include #include -#include "Core/Defines.h" +#include #include #include From 0f60259fc75430243dba12021f1693e22de87484 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9A=D0=B8=D1=80=D0=B8=D0=BB=D0=BB=20=D0=93=D0=B0=D1=80?= =?UTF-8?q?=D0=B1=D0=B0=D1=80?= Date: Fri, 15 Dec 2023 15:57:25 +0300 Subject: [PATCH 021/530] Docs for MT convertation --- .../engines/table-engines/mergetree-family/replication.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/en/engines/table-engines/mergetree-family/replication.md b/docs/en/engines/table-engines/mergetree-family/replication.md index 01782ac25bd..6ebaf4db51d 100644 --- a/docs/en/engines/table-engines/mergetree-family/replication.md +++ b/docs/en/engines/table-engines/mergetree-family/replication.md @@ -304,6 +304,12 @@ We use the term `MergeTree` to refer to all table engines in the `MergeTree fami If you had a `MergeTree` table that was manually replicated, you can convert it to a replicated table. You might need to do this if you have already collected a large amount of data in a `MergeTree` table and now you want to enable replication. +`MergeTree` table can be automatically converted on server restart if `convert_to_replicated` flag is set at the table's flags directory (`/var/lib/clickhouse/data/db_name/table_name/flags/`). +This directory is not created by default, so you need to create it first. +Then simply create empty `convert_to_replicated` file and the table will be loaded as replicated on next server restart. + +There is also a manual way to do this without server restart. + If the data differs on various replicas, first sync it, or delete this data on all the replicas except one. Rename the existing MergeTree table, then create a `ReplicatedMergeTree` table with the old name. From f5bb5697293de3771f7e2ba75af3c2a31c0a38e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9A=D0=B8=D1=80=D0=B8=D0=BB=D0=BB=20=D0=93=D0=B0=D1=80?= =?UTF-8?q?=D0=B1=D0=B0=D1=80?= Date: Sat, 16 Dec 2023 13:31:33 +0300 Subject: [PATCH 022/530] Trailing whitespaces --- src/Databases/DatabaseOrdinary.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/Databases/DatabaseOrdinary.cpp b/src/Databases/DatabaseOrdinary.cpp index 03c0febcfc4..6e29babcbb0 100644 --- a/src/Databases/DatabaseOrdinary.cpp +++ b/src/Databases/DatabaseOrdinary.cpp @@ -114,9 +114,9 @@ void DatabaseOrdinary::loadTablesMetadata(ContextPtr local_context, ParsedTables if (create_query->storage && create_query->storage->engine->name.ends_with("MergeTree") && !create_query->storage->engine->name.starts_with("Replicated")) { auto convert_to_replicated_flag_path = fs::path(getContext()->getPath()) / "data" / qualified_name.database / qualified_name.table / "flags" / "convert_to_replicated"; - + LOG_INFO(log, "Searching for convert_to_replicated flag at {}.", backQuote(convert_to_replicated_flag_path.string())); - + if (fs::exists(convert_to_replicated_flag_path)) { LOG_INFO(log, "Found convert_to_replicated flag for table {}. Will try to load it as replicated table.", backQuote(qualified_name.getFullName())); @@ -157,8 +157,8 @@ void DatabaseOrdinary::loadTablesMetadata(ContextPtr local_context, ParsedTables LOG_INFO ( - log, - "Table {} is loaded as replicated. Not removing convert_to_replicated flag until metadata in zookeeper is restored.", + log, + "Table {} is loaded as replicated. Not removing convert_to_replicated flag until metadata in zookeeper is restored.", backQuote(qualified_name.getFullName()) ); } @@ -278,23 +278,23 @@ LoadTaskPtr DatabaseOrdinary::startupTableAsync( rmt->restoreMetadataInZooKeeper(); LOG_INFO ( - log, - "Metadata in zookeeper for {} is restored. Removing convert_to_replicated flag.", + log, + "Metadata in zookeeper for {} is restored. Removing convert_to_replicated flag.", backQuote(name.getFullName()) ); } - else + else { LOG_INFO ( - log, - "Table {} is not in readonly mode but convert_to_replicated flag is set. Removing flag.", + log, + "Table {} is not in readonly mode but convert_to_replicated flag is set. Removing flag.", backQuote(name.getFullName()) ); } fs::remove(convert_to_replicated_flag_path); } - } + } logAboutProgress(log, ++tables_started, total_tables_to_startup, startup_watch); } else From 1ca059594f32765e37b215b09563ef02af15d4d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9A=D0=B8=D1=80=D0=B8=D0=BB=D0=BB=20=D0=93=D0=B0=D1=80?= =?UTF-8?q?=D0=B1=D0=B0=D1=80?= Date: Sat, 16 Dec 2023 13:48:25 +0300 Subject: [PATCH 023/530] Format tests --- .../test_modify_engine_on_restart/test.py | 128 ++++++++++-------- .../test_modify_engine_on_restart/test_mv.py | 86 +++++++----- 2 files changed, 120 insertions(+), 94 deletions(-) diff --git a/tests/integration/test_modify_engine_on_restart/test.py b/tests/integration/test_modify_engine_on_restart/test.py index 6c82cb93a4d..4f0e9c9da69 100644 --- a/tests/integration/test_modify_engine_on_restart/test.py +++ b/tests/integration/test_modify_engine_on_restart/test.py @@ -24,6 +24,7 @@ ch2 = cluster.add_instance( database_name = "modify_engine" + @pytest.fixture(scope="module") def started_cluster(): try: @@ -33,93 +34,97 @@ def started_cluster(): finally: cluster.shutdown() + def q(node, query): - return node.query( - database=database_name, - sql=query - ) + return node.query(database=database_name, sql=query) + def create_tables(): # MergeTree table that will be converted q( ch1, - "CREATE TABLE rmt ( A Int64, D Date, S String ) ENGINE MergeTree() PARTITION BY toYYYYMM(D) ORDER BY A;" + "CREATE TABLE rmt ( A Int64, D Date, S String ) ENGINE MergeTree() PARTITION BY toYYYYMM(D) ORDER BY A;", ) - q( - ch1, - "INSERT INTO rmt SELECT number, today(), '' FROM numbers(1e6);" - ) - q( - ch1, - "INSERT INTO rmt SELECT number, today()-60, '' FROM numbers(1e5);" - ) + q(ch1, "INSERT INTO rmt SELECT number, today(), '' FROM numbers(1e6);") + q(ch1, "INSERT INTO rmt SELECT number, today()-60, '' FROM numbers(1e5);") # ReplacingMergeTree table that will be converted to check unusual engine kinds q( ch1, - "CREATE TABLE replacing ( A Int64, D Date, S String ) ENGINE ReplacingMergeTree() PARTITION BY toYYYYMM(D) ORDER BY A;" + "CREATE TABLE replacing ( A Int64, D Date, S String ) ENGINE ReplacingMergeTree() PARTITION BY toYYYYMM(D) ORDER BY A;", ) - q( - ch1, - "INSERT INTO replacing SELECT number, today(), '' FROM numbers(1e6);" - ) - q( - ch1, - "INSERT INTO replacing SELECT number, today()-60, '' FROM numbers(1e5);" - ) + q(ch1, "INSERT INTO replacing SELECT number, today(), '' FROM numbers(1e6);") + q(ch1, "INSERT INTO replacing SELECT number, today()-60, '' FROM numbers(1e5);") # MergeTree table that will not be converted q( ch1, - "CREATE TABLE mt ( A Int64, D Date, S String ) ENGINE MergeTree() PARTITION BY toYYYYMM(D) ORDER BY A;" + "CREATE TABLE mt ( A Int64, D Date, S String ) ENGINE MergeTree() PARTITION BY toYYYYMM(D) ORDER BY A;", ) # Not MergeTree table - q( - ch1, - "CREATE TABLE log ( A Int64, D Date, S String ) ENGINE Log;" - ) + q(ch1, "CREATE TABLE log ( A Int64, D Date, S String ) ENGINE Log;") + def check_tables(converted): engine_prefix = "" - if (converted): + if converted: engine_prefix = "Replicated" # Check tables exists - assert q( - ch1, - "SHOW TABLES", - ).strip() == "log\nmt\nreplacing\nrmt" + assert ( + q( + ch1, + "SHOW TABLES", + ).strip() + == "log\nmt\nreplacing\nrmt" + ) # Check engines - assert q( - ch1, - f"SELECT name, engine FROM system.tables WHERE database = '{database_name}' AND (name != 'log' AND name != 'mt')", - ).strip() == f"replacing\t{engine_prefix}ReplacingMergeTree\nrmt\t{engine_prefix}MergeTree" - assert q( - ch1, - f"SELECT name, engine FROM system.tables WHERE database = '{database_name}' AND (name = 'log' OR name = 'mt')", - ).strip() == "log\tLog\nmt\tMergeTree" + assert ( + q( + ch1, + f"SELECT name, engine FROM system.tables WHERE database = '{database_name}' AND (name != 'log' AND name != 'mt')", + ).strip() + == f"replacing\t{engine_prefix}ReplacingMergeTree\nrmt\t{engine_prefix}MergeTree" + ) + assert ( + q( + ch1, + f"SELECT name, engine FROM system.tables WHERE database = '{database_name}' AND (name = 'log' OR name = 'mt')", + ).strip() + == "log\tLog\nmt\tMergeTree" + ) # Check values for table in ["rmt", "replacing"]: + assert ( + q( + ch1, + f"SELECT count() FROM {table}", + ).strip() + == "1100000" + ) - assert q( - ch1, - f"SELECT count() FROM {table}", - ).strip() == "1100000" def set_convert_flags(): - # Set convert flag on actually convertable tables for table in ["rmt", "replacing"]: ch1.exec_in_container( - ["bash", "-c", f"mkdir /var/lib/clickhouse/data/{database_name}/{table}/flags"] + [ + "bash", + "-c", + f"mkdir /var/lib/clickhouse/data/{database_name}/{table}/flags", + ] ) ch1.exec_in_container( - ["bash", "-c", f"touch /var/lib/clickhouse/data/{database_name}/{table}/flags/convert_to_replicated"] + [ + "bash", + "-c", + f"touch /var/lib/clickhouse/data/{database_name}/{table}/flags/convert_to_replicated", + ] ) # Set flag to not MergeTree table to check that nothing happens @@ -127,33 +132,42 @@ def set_convert_flags(): ["bash", "-c", f"mkdir /var/lib/clickhouse/data/{database_name}/log/flags"] ) ch1.exec_in_container( - ["bash", "-c", f"touch /var/lib/clickhouse/data/{database_name}/log/flags/convert_to_replicated"] + [ + "bash", + "-c", + f"touch /var/lib/clickhouse/data/{database_name}/log/flags/convert_to_replicated", + ] ) + def check_replica_added(): # Add replica to check if zookeeper path is correct and consistent with table uuid uuid = q( ch1, - f"SELECT uuid FROM system.tables WHERE table = 'rmt' AND database = '{database_name}'" + f"SELECT uuid FROM system.tables WHERE table = 'rmt' AND database = '{database_name}'", ).strip() q( ch2, - f"CREATE TABLE rmt ( A Int64, D Date, S String ) ENGINE ReplicatedMergeTree('/clickhouse/tables/{uuid}/{{shard}}', '{{replica}}') PARTITION BY toYYYYMM(D) ORDER BY A" + f"CREATE TABLE rmt ( A Int64, D Date, S String ) ENGINE ReplicatedMergeTree('/clickhouse/tables/{uuid}/{{shard}}', '{{replica}}') PARTITION BY toYYYYMM(D) ORDER BY A", ) - + ch2.query(database=database_name, sql="SYSTEM SYNC REPLICA rmt", timeout=20) # Check values - assert q( - ch2, - f"SELECT count() FROM rmt", - ).strip() == "1100000" + assert ( + q( + ch2, + f"SELECT count() FROM rmt", + ).strip() + == "1100000" + ) + def test_modify_engine_on_restart(started_cluster): ch1.query("CREATE DATABASE " + database_name + " ON CLUSTER cluster") - + create_tables() check_tables(False) @@ -172,4 +186,4 @@ def test_modify_engine_on_restart(started_cluster): ch1.restart_clickhouse() - check_tables(True) \ No newline at end of file + check_tables(True) diff --git a/tests/integration/test_modify_engine_on_restart/test_mv.py b/tests/integration/test_modify_engine_on_restart/test_mv.py index c9c0c3d19c9..b624eeb8c32 100644 --- a/tests/integration/test_modify_engine_on_restart/test_mv.py +++ b/tests/integration/test_modify_engine_on_restart/test_mv.py @@ -15,6 +15,7 @@ ch1 = cluster.add_instance( database_name = "modify_engine_with_mv" + @pytest.fixture(scope="module") def started_cluster(): try: @@ -24,23 +25,22 @@ def started_cluster(): finally: cluster.shutdown() + def q(node, query): - return node.query( - database=database_name, - sql=query - ) + return node.query(database=database_name, sql=query) + def create_tables(): q( ch1, - "CREATE TABLE hourly_data(`domain_name` String, `event_time` DateTime, `count_views` UInt64) ENGINE = MergeTree ORDER BY (domain_name, event_time)" + "CREATE TABLE hourly_data(`domain_name` String, `event_time` DateTime, `count_views` UInt64) ENGINE = MergeTree ORDER BY (domain_name, event_time)", ) q( ch1, "CREATE TABLE monthly_aggregated_data\ (`domain_name` String, `month` Date, `sumCountViews` AggregateFunction(sum, UInt64))\ - ENGINE = AggregatingMergeTree ORDER BY (domain_name, month)" + ENGINE = AggregatingMergeTree ORDER BY (domain_name, month)", ) q( @@ -55,7 +55,7 @@ def create_tables(): FROM hourly_data\ GROUP BY\ domain_name,\ - month" + month", ) q( @@ -64,32 +64,36 @@ def create_tables(): VALUES ('clickhouse.com', '2019-01-01 10:00:00', 1),\ ('clickhouse.com', '2019-02-02 00:00:00', 2),\ ('clickhouse.com', '2019-02-01 00:00:00', 3),\ - ('clickhouse.com', '2020-01-01 00:00:00', 6)" + ('clickhouse.com', '2020-01-01 00:00:00', 6)", ) + def check_tables(converted): engine_prefix = "" - if (converted): + if converted: engine_prefix = "Replicated" # Check engines - assert q( - ch1, - f"SELECT name, engine FROM system.tables WHERE database = '{database_name}'", - ).strip() == f"hourly_data\t{engine_prefix}MergeTree\nmonthly_aggregated_data\t{engine_prefix}AggregatingMergeTree\nmonthly_aggregated_data_mv\tMaterializedView" + assert ( + q( + ch1, + f"SELECT name, engine FROM system.tables WHERE database = '{database_name}'", + ).strip() + == f"hourly_data\t{engine_prefix}MergeTree\nmonthly_aggregated_data\t{engine_prefix}AggregatingMergeTree\nmonthly_aggregated_data_mv\tMaterializedView" + ) # Check values - assert q( - ch1, - "SELECT sumMerge(sumCountViews) as sumCountViews\ - FROM monthly_aggregated_data_mv" - ).strip() == "12" - assert q( - ch1, - "SELECT count() FROM hourly_data" - ).strip() == "4" + assert ( + q( + ch1, + "SELECT sumMerge(sumCountViews) as sumCountViews\ + FROM monthly_aggregated_data_mv", + ).strip() + == "12" + ) + assert q(ch1, "SELECT count() FROM hourly_data").strip() == "4" - if (converted): + if converted: # Insert new values to check if new dependencies are set correctly q( ch1, @@ -97,29 +101,38 @@ def check_tables(converted): VALUES ('clickhouse.com', '2019-01-01 10:00:00', 1),\ ('clickhouse.com', '2019-02-02 00:00:00', 2),\ ('clickhouse.com', '2019-02-01 00:00:00', 3),\ - ('clickhouse.com', '2020-01-01 00:00:00', 6)" + ('clickhouse.com', '2020-01-01 00:00:00', 6)", ) - assert q( - ch1, - "SELECT sumMerge(sumCountViews) as sumCountViews\ - FROM monthly_aggregated_data_mv" - ).strip() == "24" - assert q( - ch1, - "SELECT count() FROM hourly_data" - ).strip() == "8" + assert ( + q( + ch1, + "SELECT sumMerge(sumCountViews) as sumCountViews\ + FROM monthly_aggregated_data_mv", + ).strip() + == "24" + ) + assert q(ch1, "SELECT count() FROM hourly_data").strip() == "8" + def set_convert_flags(): - for table in ["hourly_data", "monthly_aggregated_data"]: ch1.exec_in_container( - ["bash", "-c", f"mkdir /var/lib/clickhouse/data/{database_name}/{table}/flags"] + [ + "bash", + "-c", + f"mkdir /var/lib/clickhouse/data/{database_name}/{table}/flags", + ] ) ch1.exec_in_container( - ["bash", "-c", f"touch /var/lib/clickhouse/data/{database_name}/{table}/flags/convert_to_replicated"] + [ + "bash", + "-c", + f"touch /var/lib/clickhouse/data/{database_name}/{table}/flags/convert_to_replicated", + ] ) + def test_modify_engine_on_restart_with_materialized_view(started_cluster): ch1.query(f"CREATE DATABASE {database_name}") @@ -132,4 +145,3 @@ def test_modify_engine_on_restart_with_materialized_view(started_cluster): ch1.restart_clickhouse() check_tables(True) - \ No newline at end of file From f81b1375634cd208ea6e1fe9aaf3a0e534511d05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9A=D0=B8=D1=80=D0=B8=D0=BB=D0=BB=20=D0=93=D0=B0=D1=80?= =?UTF-8?q?=D0=B1=D0=B0=D1=80?= Date: Mon, 18 Dec 2023 21:35:26 +0300 Subject: [PATCH 024/530] Fix heap use after free --- src/Databases/DatabaseOrdinary.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Databases/DatabaseOrdinary.cpp b/src/Databases/DatabaseOrdinary.cpp index ef9e65a60e8..2fca9afc304 100644 --- a/src/Databases/DatabaseOrdinary.cpp +++ b/src/Databases/DatabaseOrdinary.cpp @@ -140,6 +140,7 @@ void DatabaseOrdinary::loadTablesMetadata(ContextPtr local_context, ParsedTables auto new_ast = parseQuery(parser_create_query, create_query_string, 0, DBMS_DEFAULT_MAX_PARSER_DEPTH); ast = new_ast; + create_query = ast->as(); /// Write changes to metadata String table_metadata_path = full_path; From 64073f09b66faea4b2e17f61ac35e0ca0c66ec17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9A=D0=B8=D1=80=D0=B8=D0=BB=D0=BB=20=D0=93=D0=B0=D1=80?= =?UTF-8?q?=D0=B1=D0=B0=D1=80?= Date: Tue, 19 Dec 2023 20:57:25 +0300 Subject: [PATCH 025/530] Support converting tables with explicit engine arguments --- src/Databases/DatabaseOrdinary.cpp | 23 ++-- .../test_args.py | 117 ++++++++++++++++++ 2 files changed, 130 insertions(+), 10 deletions(-) create mode 100644 tests/integration/test_modify_engine_on_restart/test_args.py diff --git a/src/Databases/DatabaseOrdinary.cpp b/src/Databases/DatabaseOrdinary.cpp index 2fca9afc304..dedbc85e6da 100644 --- a/src/Databases/DatabaseOrdinary.cpp +++ b/src/Databases/DatabaseOrdinary.cpp @@ -123,24 +123,27 @@ void DatabaseOrdinary::loadTablesMetadata(ContextPtr local_context, ParsedTables /// Get storage definition /// Set uuid explicitly, because it is forbidden to use the 'uuid' macro without ON CLUSTER - auto * storage = create_query->storage->as(); - String storage_definition = queryToString(*storage); + auto * storage = create_query->storage; String replica_path = getContext()->getConfigRef().getString("default_replica_path", "/clickhouse/tables/{uuid}/{shard}"); replica_path = boost::algorithm::replace_all_copy(replica_path, "{uuid}", fmt::format("{}", create_query->uuid)); String replica_name = getContext()->getConfigRef().getString("default_replica_name", "{replica}"); String replicated_args = fmt::format("('{}', '{}')", replica_path, replica_name); - String replicated_engine = "Replicated" + storage->engine->name + replicated_args; + String replicated_engine = "ENGINE = Replicated" + storage->engine->name + replicated_args; - String create_query_string = queryToString(*ast); + ParserStorage parser_storage{ParserStorage::TABLE_ENGINE}; + auto replicated_storage_ast = parseQuery(parser_storage, replicated_engine, 0, DBMS_DEFAULT_MAX_PARSER_DEPTH); + auto * replicated_storage = replicated_storage_ast->as(); - create_query_string = boost::algorithm::replace_first_copy(create_query_string, storage->engine->name, replicated_engine); + /// Add old engine's arguments + if (storage->engine->arguments) + { + for (size_t i = 0; i < storage->engine->arguments->children.size(); ++i) + replicated_storage->engine->arguments->children.push_back(storage->engine->arguments->children[i]->clone()); + } - ParserCreateQuery parser_create_query; - auto new_ast = parseQuery(parser_create_query, create_query_string, 0, DBMS_DEFAULT_MAX_PARSER_DEPTH); - - ast = new_ast; - create_query = ast->as(); + /// Set new engine for the old query + create_query->storage->set(create_query->storage->engine, replicated_storage->engine->clone()); /// Write changes to metadata String table_metadata_path = full_path; diff --git a/tests/integration/test_modify_engine_on_restart/test_args.py b/tests/integration/test_modify_engine_on_restart/test_args.py new file mode 100644 index 00000000000..e3d917b0079 --- /dev/null +++ b/tests/integration/test_modify_engine_on_restart/test_args.py @@ -0,0 +1,117 @@ +import pytest +from helpers.cluster import ClickHouseCluster + +cluster = ClickHouseCluster(__file__) +ch1 = cluster.add_instance( + "ch1", + main_configs=[ + "configs/config.d/clusters.xml", + "configs/config.d/distributed_ddl.xml", + ], + with_zookeeper=True, + macros={"replica": "node1"}, + stay_alive=True, +) + +database_name = "modify_engine_args" + + +@pytest.fixture(scope="module") +def started_cluster(): + try: + cluster.start() + yield cluster + + finally: + cluster.shutdown() + + +def q(node, query): + return node.query(database=database_name, sql=query) + + +def create_tables(): + # Check one argument + q( + ch1, + "CREATE TABLE replacing_ver ( A Int64, D Date, S String ) ENGINE = ReplacingMergeTree(D) PARTITION BY toYYYYMM(D) ORDER BY A", + ) + + # Check more than one argument + q( + ch1, + "CREATE TABLE collapsing_ver ( ID UInt64, Sign Int8, Version UInt8 ) ENGINE = VersionedCollapsingMergeTree(Sign, Version) ORDER BY ID", + ) + + +def check_tables(): + # Check tables exists + assert ( + q( + ch1, + "SHOW TABLES", + ).strip() + == "collapsing_ver\nreplacing_ver" + ) + + replacing_uuid = q( + ch1, + f"SELECT uuid FROM system.tables WHERE database = '{database_name}' and name = 'replacing_ver'", + ).strip() + collapsing_uuid = q( + ch1, + f"SELECT uuid FROM system.tables WHERE database = '{database_name}' and name = 'collapsing_ver'", + ).strip() + + # Check engines + assert ( + q( + ch1, + f"SELECT engine_full FROM system.tables WHERE database = '{database_name}' and name = 'replacing_ver'", + ) + .strip() + .startswith( + f"ReplicatedReplacingMergeTree(\\'/clickhouse/tables/{replacing_uuid}/{{shard}}\\', \\'{{replica}}\\', D)" + ) + ) + assert ( + q( + ch1, + f"SELECT engine_full FROM system.tables WHERE database = '{database_name}' and name = 'collapsing_ver'", + ) + .strip() + .startswith( + f"ReplicatedVersionedCollapsingMergeTree(\\'/clickhouse/tables/{collapsing_uuid}/{{shard}}\\', \\'{{replica}}\\', Sign, Version)" + ) + ) + + +def set_convert_flags(): + # Set convert flag on actually convertable tables + for table in ["replacing_ver", "collapsing_ver"]: + ch1.exec_in_container( + [ + "bash", + "-c", + f"mkdir /var/lib/clickhouse/data/{database_name}/{table}/flags", + ] + ) + ch1.exec_in_container( + [ + "bash", + "-c", + f"touch /var/lib/clickhouse/data/{database_name}/{table}/flags/convert_to_replicated", + ] + ) + + +def test_modify_engine_on_restart(started_cluster): + ch1.query("CREATE DATABASE " + database_name) + + create_tables() + + set_convert_flags() + + ch1.restart_clickhouse() + + check_tables() From 002bce2b27c934cab64cc76d508829aa0212de74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9A=D0=B8=D1=80=D0=B8=D0=BB=D0=BB=20=D0=93=D0=B0=D1=80?= =?UTF-8?q?=D0=B1=D0=B0=D1=80?= Date: Wed, 27 Dec 2023 23:00:26 +0300 Subject: [PATCH 026/530] Move code to method, default values, early exit --- src/Databases/DatabaseOrdinary.cpp | 118 +++++++++++++++-------------- 1 file changed, 62 insertions(+), 56 deletions(-) diff --git a/src/Databases/DatabaseOrdinary.cpp b/src/Databases/DatabaseOrdinary.cpp index dedbc85e6da..2ac577db976 100644 --- a/src/Databases/DatabaseOrdinary.cpp +++ b/src/Databases/DatabaseOrdinary.cpp @@ -62,6 +62,67 @@ void DatabaseOrdinary::loadStoredObjects(ContextMutablePtr, LoadingStrictnessLev throw Exception(ErrorCodes::LOGICAL_ERROR, "Not implemented"); } +static void convertMergeTreeToReplicatedIfNeeded(ASTPtr ast, QualifiedTableName qualified_name, ContextPtr context, Poco::Logger * log, fs::path full_path) +{ + auto * create_query = ast->as(); + + if (!create_query->storage || !create_query->storage->engine->name.ends_with("MergeTree") || create_query->storage->engine->name.starts_with("Replicated")) + return; + + auto convert_to_replicated_flag_path = fs::path(context->getPath()) / "data" / qualified_name.database / qualified_name.table / "flags" / "convert_to_replicated"; + + LOG_INFO(log, "Searching for convert_to_replicated flag at {}.", backQuote(convert_to_replicated_flag_path.string())); + + if (!fs::exists(convert_to_replicated_flag_path)) + return; + + LOG_INFO(log, "Found convert_to_replicated flag for table {}. Will try to change it's engine in metadata to replicated table.", backQuote(qualified_name.getFullName())); + + /// Get storage definition + /// Set uuid explicitly, because it is forbidden to use the 'uuid' macro without ON CLUSTER + auto * storage = create_query->storage; + + const auto & config = context->getConfigRef(); + String replica_path = StorageReplicatedMergeTree::getDefaultZooKeeperPath(config); + replica_path = boost::algorithm::replace_all_copy(replica_path, "{uuid}", fmt::format("{}", create_query->uuid)); + String replica_name = StorageReplicatedMergeTree::getDefaultReplicaName(config); + String replicated_args = fmt::format("('{}', '{}')", replica_path, replica_name); + String replicated_engine = "ENGINE = Replicated" + storage->engine->name + replicated_args; + + ParserStorage parser_storage{ParserStorage::TABLE_ENGINE}; + auto replicated_storage_ast = parseQuery(parser_storage, replicated_engine, 0, DBMS_DEFAULT_MAX_PARSER_DEPTH); + auto * replicated_storage = replicated_storage_ast->as(); + + /// Add old engine's arguments + if (storage->engine->arguments) + { + for (size_t i = 0; i < storage->engine->arguments->children.size(); ++i) + replicated_storage->engine->arguments->children.push_back(storage->engine->arguments->children[i]->clone()); + } + + /// Set new engine for the old query + create_query->storage->set(create_query->storage->engine, replicated_storage->engine->clone()); + + /// Write changes to metadata + String table_metadata_path = full_path; + String table_metadata_tmp_path = table_metadata_path + ".tmp"; + String statement = getObjectDefinitionFromCreateQuery(ast); + { + WriteBufferFromFile out(table_metadata_tmp_path, statement.size(), O_WRONLY | O_CREAT | O_EXCL); + writeString(statement, out); + out.next(); + if (context->getSettingsRef().fsync_metadata) + out.sync(); + out.close(); + } + fs::rename(table_metadata_tmp_path, table_metadata_path); + + LOG_INFO( + log, + "Table {} is loaded as replicated. Not removing convert_to_replicated flag until metadata in zookeeper is restored.", + backQuote(qualified_name.getFullName())); +} + void DatabaseOrdinary::loadTablesMetadata(ContextPtr local_context, ParsedTablesMetadata & metadata, bool is_startup) { size_t prev_tables_count = metadata.parsed_tables.size(); @@ -111,62 +172,7 @@ void DatabaseOrdinary::loadTablesMetadata(ContextPtr local_context, ParsedTables QualifiedTableName qualified_name{TSA_SUPPRESS_WARNING_FOR_READ(database_name), create_query->getTable()}; - if (create_query->storage && create_query->storage->engine->name.ends_with("MergeTree") && !create_query->storage->engine->name.starts_with("Replicated")) - { - auto convert_to_replicated_flag_path = fs::path(getContext()->getPath()) / "data" / qualified_name.database / qualified_name.table / "flags" / "convert_to_replicated"; - - LOG_INFO(log, "Searching for convert_to_replicated flag at {}.", backQuote(convert_to_replicated_flag_path.string())); - - if (fs::exists(convert_to_replicated_flag_path)) - { - LOG_INFO(log, "Found convert_to_replicated flag for table {}. Will try to load it as replicated table.", backQuote(qualified_name.getFullName())); - - /// Get storage definition - /// Set uuid explicitly, because it is forbidden to use the 'uuid' macro without ON CLUSTER - auto * storage = create_query->storage; - - String replica_path = getContext()->getConfigRef().getString("default_replica_path", "/clickhouse/tables/{uuid}/{shard}"); - replica_path = boost::algorithm::replace_all_copy(replica_path, "{uuid}", fmt::format("{}", create_query->uuid)); - String replica_name = getContext()->getConfigRef().getString("default_replica_name", "{replica}"); - String replicated_args = fmt::format("('{}', '{}')", replica_path, replica_name); - String replicated_engine = "ENGINE = Replicated" + storage->engine->name + replicated_args; - - ParserStorage parser_storage{ParserStorage::TABLE_ENGINE}; - auto replicated_storage_ast = parseQuery(parser_storage, replicated_engine, 0, DBMS_DEFAULT_MAX_PARSER_DEPTH); - auto * replicated_storage = replicated_storage_ast->as(); - - /// Add old engine's arguments - if (storage->engine->arguments) - { - for (size_t i = 0; i < storage->engine->arguments->children.size(); ++i) - replicated_storage->engine->arguments->children.push_back(storage->engine->arguments->children[i]->clone()); - } - - /// Set new engine for the old query - create_query->storage->set(create_query->storage->engine, replicated_storage->engine->clone()); - - /// Write changes to metadata - String table_metadata_path = full_path; - String table_metadata_tmp_path = table_metadata_path + ".tmp"; - String statement = getObjectDefinitionFromCreateQuery(ast); - { - WriteBufferFromFile out(table_metadata_tmp_path, statement.size(), O_WRONLY | O_CREAT | O_EXCL); - writeString(statement, out); - out.next(); - if (getContext()->getSettingsRef().fsync_metadata) - out.sync(); - out.close(); - } - fs::rename(table_metadata_tmp_path, table_metadata_path); - - LOG_INFO - ( - log, - "Table {} is loaded as replicated. Not removing convert_to_replicated flag until metadata in zookeeper is restored.", - backQuote(qualified_name.getFullName()) - ); - } - } + convertMergeTreeToReplicatedIfNeeded(ast, qualified_name, getContext(), log, full_path); std::lock_guard lock{metadata.mutex}; metadata.parsed_tables[qualified_name] = ParsedTableMetadata{full_path.string(), ast}; From 8fa23a76d0e6e92047804ddd5bd6b388359cbf25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9A=D0=B8=D1=80=D0=B8=D0=BB=D0=BB=20=D0=93=D0=B0=D1=80?= =?UTF-8?q?=D0=B1=D0=B0=D1=80?= Date: Wed, 27 Dec 2023 23:03:24 +0300 Subject: [PATCH 027/530] Better log messages --- src/Databases/DatabaseOrdinary.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Databases/DatabaseOrdinary.cpp b/src/Databases/DatabaseOrdinary.cpp index 2ac577db976..cf8f73e6de7 100644 --- a/src/Databases/DatabaseOrdinary.cpp +++ b/src/Databases/DatabaseOrdinary.cpp @@ -71,7 +71,7 @@ static void convertMergeTreeToReplicatedIfNeeded(ASTPtr ast, QualifiedTableName auto convert_to_replicated_flag_path = fs::path(context->getPath()) / "data" / qualified_name.database / qualified_name.table / "flags" / "convert_to_replicated"; - LOG_INFO(log, "Searching for convert_to_replicated flag at {}.", backQuote(convert_to_replicated_flag_path.string())); + LOG_DEBUG(log, "Searching for convert_to_replicated flag at {}.", backQuote(convert_to_replicated_flag_path.string())); if (!fs::exists(convert_to_replicated_flag_path)) return; @@ -119,8 +119,9 @@ static void convertMergeTreeToReplicatedIfNeeded(ASTPtr ast, QualifiedTableName LOG_INFO( log, - "Table {} is loaded as replicated. Not removing convert_to_replicated flag until metadata in zookeeper is restored.", - backQuote(qualified_name.getFullName())); + "Engine of table {} is set to replicated in metadata. Not removing convert_to_replicated flag until table is loaded and metadata in zookeeper is restored.", + backQuote(qualified_name.getFullName()) + ); } void DatabaseOrdinary::loadTablesMetadata(ContextPtr local_context, ParsedTablesMetadata & metadata, bool is_startup) From 68cf2a6b60a753d69685008fec94d6b8e923ff08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9A=D0=B8=D1=80=D0=B8=D0=BB=D0=BB=20=D0=93=D0=B0=D1=80?= =?UTF-8?q?=D0=B1=D0=B0=D1=80?= Date: Wed, 27 Dec 2023 23:33:54 +0300 Subject: [PATCH 028/530] Convert method as private member --- src/Databases/DatabaseOrdinary.cpp | 14 +++++++++----- src/Databases/DatabaseOrdinary.h | 3 +++ 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/Databases/DatabaseOrdinary.cpp b/src/Databases/DatabaseOrdinary.cpp index cf8f73e6de7..8caefdca2a3 100644 --- a/src/Databases/DatabaseOrdinary.cpp +++ b/src/Databases/DatabaseOrdinary.cpp @@ -62,14 +62,18 @@ void DatabaseOrdinary::loadStoredObjects(ContextMutablePtr, LoadingStrictnessLev throw Exception(ErrorCodes::LOGICAL_ERROR, "Not implemented"); } -static void convertMergeTreeToReplicatedIfNeeded(ASTPtr ast, QualifiedTableName qualified_name, ContextPtr context, Poco::Logger * log, fs::path full_path) +void DatabaseOrdinary::convertMergeTreeToReplicatedIfNeeded(ASTPtr ast, const QualifiedTableName & qualified_name, const String & file_name) { + fs::path path(getMetadataPath()); + fs::path file_path(file_name); + fs::path full_path = path / file_path; + auto * create_query = ast->as(); if (!create_query->storage || !create_query->storage->engine->name.ends_with("MergeTree") || create_query->storage->engine->name.starts_with("Replicated")) return; - auto convert_to_replicated_flag_path = fs::path(context->getPath()) / "data" / qualified_name.database / qualified_name.table / "flags" / "convert_to_replicated"; + auto convert_to_replicated_flag_path = fs::path(getContext()->getPath()) / "data" / qualified_name.database / qualified_name.table / "flags" / "convert_to_replicated"; LOG_DEBUG(log, "Searching for convert_to_replicated flag at {}.", backQuote(convert_to_replicated_flag_path.string())); @@ -82,7 +86,7 @@ static void convertMergeTreeToReplicatedIfNeeded(ASTPtr ast, QualifiedTableName /// Set uuid explicitly, because it is forbidden to use the 'uuid' macro without ON CLUSTER auto * storage = create_query->storage; - const auto & config = context->getConfigRef(); + const auto & config = getContext()->getConfigRef(); String replica_path = StorageReplicatedMergeTree::getDefaultZooKeeperPath(config); replica_path = boost::algorithm::replace_all_copy(replica_path, "{uuid}", fmt::format("{}", create_query->uuid)); String replica_name = StorageReplicatedMergeTree::getDefaultReplicaName(config); @@ -111,7 +115,7 @@ static void convertMergeTreeToReplicatedIfNeeded(ASTPtr ast, QualifiedTableName WriteBufferFromFile out(table_metadata_tmp_path, statement.size(), O_WRONLY | O_CREAT | O_EXCL); writeString(statement, out); out.next(); - if (context->getSettingsRef().fsync_metadata) + if (getContext()->getSettingsRef().fsync_metadata) out.sync(); out.close(); } @@ -173,7 +177,7 @@ void DatabaseOrdinary::loadTablesMetadata(ContextPtr local_context, ParsedTables QualifiedTableName qualified_name{TSA_SUPPRESS_WARNING_FOR_READ(database_name), create_query->getTable()}; - convertMergeTreeToReplicatedIfNeeded(ast, qualified_name, getContext(), log, full_path); + convertMergeTreeToReplicatedIfNeeded(ast, qualified_name, file_name); std::lock_guard lock{metadata.mutex}; metadata.parsed_tables[qualified_name] = ParsedTableMetadata{full_path.string(), ast}; diff --git a/src/Databases/DatabaseOrdinary.h b/src/Databases/DatabaseOrdinary.h index d1d7dfd83fa..6c0829f0da8 100644 --- a/src/Databases/DatabaseOrdinary.h +++ b/src/Databases/DatabaseOrdinary.h @@ -80,6 +80,9 @@ protected: std::atomic total_tables_to_startup{0}; std::atomic tables_started{0}; AtomicStopwatch startup_watch; + +private: + void convertMergeTreeToReplicatedIfNeeded(ASTPtr ast, const QualifiedTableName & qualified_name, const String & file_name); }; } From 6f79f987191c8196f6b8f26fc395b15448fd0202 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9A=D0=B8=D1=80=D0=B8=D0=BB=D0=BB=20=D0=93=D0=B0=D1=80?= =?UTF-8?q?=D0=B1=D0=B0=D1=80?= Date: Thu, 28 Dec 2023 00:54:10 +0300 Subject: [PATCH 029/530] Throw on ordinary db engine --- src/Databases/DatabaseOrdinary.cpp | 4 + .../test_ordinary.py | 106 ++++++++++++++++++ 2 files changed, 110 insertions(+) create mode 100644 tests/integration/test_modify_engine_on_restart/test_ordinary.py diff --git a/src/Databases/DatabaseOrdinary.cpp b/src/Databases/DatabaseOrdinary.cpp index 8caefdca2a3..e5ab7589b1c 100644 --- a/src/Databases/DatabaseOrdinary.cpp +++ b/src/Databases/DatabaseOrdinary.cpp @@ -80,6 +80,10 @@ void DatabaseOrdinary::convertMergeTreeToReplicatedIfNeeded(ASTPtr ast, const Qu if (!fs::exists(convert_to_replicated_flag_path)) return; + if (getUUID() == UUIDHelpers::Nil) + throw Exception(ErrorCodes::NOT_IMPLEMENTED, + "Table engine conversion to replicated is supported only for Atomic databases. Convert your database engine to Atomic first."); + LOG_INFO(log, "Found convert_to_replicated flag for table {}. Will try to change it's engine in metadata to replicated table.", backQuote(qualified_name.getFullName())); /// Get storage definition diff --git a/tests/integration/test_modify_engine_on_restart/test_ordinary.py b/tests/integration/test_modify_engine_on_restart/test_ordinary.py new file mode 100644 index 00000000000..7e9830c5166 --- /dev/null +++ b/tests/integration/test_modify_engine_on_restart/test_ordinary.py @@ -0,0 +1,106 @@ +import pytest +from helpers.cluster import ClickHouseCluster + +cluster = ClickHouseCluster(__file__) +ch1 = cluster.add_instance( + "ch1", + main_configs=[ + "configs/config.d/clusters.xml", + "configs/config.d/distributed_ddl.xml", + ], + with_zookeeper=True, + macros={"replica": "node1"}, + stay_alive=True, +) + +database_name = "modify_engine_on_ordinary" + + +@pytest.fixture(scope="module") +def started_cluster(): + try: + cluster.start() + yield cluster + + finally: + cluster.shutdown() + + +def q(node, query): + return node.query(database=database_name, sql=query) + +def create_tables(): + q( + ch1, + "CREATE TABLE mt ( A Int64, D Date, S String ) ENGINE MergeTree() PARTITION BY toYYYYMM(D) ORDER BY A;", + ) + +def check_tables(): + # Check tables exists + assert ( + q( + ch1, + "SHOW TABLES", + ).strip() + == "mt" + ) + + # Check engines + assert ( + q( + ch1, + f"SELECT name, engine FROM system.tables WHERE database = '{database_name}'", + ).strip() + == f"mt\tMergeTree" + ) + + +def set_convert_flags(): + ch1.exec_in_container( + [ + "bash", + "-c", + f"mkdir /var/lib/clickhouse/data/{database_name}/mt/flags", + ] + ) + ch1.exec_in_container( + [ + "bash", + "-c", + f"touch /var/lib/clickhouse/data/{database_name}/mt/flags/convert_to_replicated", + ] + ) + +def remove_convert_flags(): + ch1.exec_in_container( + [ + "bash", + "-c", + f"rm -rf /var/lib/clickhouse/data/{database_name}/mt/flags", + ] + ) + +def test_modify_engine_on_restart_ordinary_database(started_cluster): + ch1.query( + sql=f"CREATE DATABASE {database_name} ENGINE = Ordinary", + settings={"allow_deprecated_database_ordinary": 1}, + ) + + create_tables() + + check_tables() + + set_convert_flags() + + cannot_start = False + try: + ch1.restart_clickhouse() + except: + cannot_start = True + assert cannot_start + + remove_convert_flags() + + ch1.restart_clickhouse() + + check_tables() From f0b12c4debfe22b9df72309ef1a2e49d182fb6ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9A=D0=B8=D1=80=D0=B8=D0=BB=D0=BB=20=D0=93=D0=B0=D1=80?= =?UTF-8?q?=D0=B1=D0=B0=D1=80?= Date: Thu, 28 Dec 2023 01:49:39 +0300 Subject: [PATCH 030/530] Better replicated engine creation --- src/Databases/DatabaseOrdinary.cpp | 57 +++++++++++++++++------------- 1 file changed, 32 insertions(+), 25 deletions(-) diff --git a/src/Databases/DatabaseOrdinary.cpp b/src/Databases/DatabaseOrdinary.cpp index e5ab7589b1c..770c223d931 100644 --- a/src/Databases/DatabaseOrdinary.cpp +++ b/src/Databases/DatabaseOrdinary.cpp @@ -62,6 +62,36 @@ void DatabaseOrdinary::loadStoredObjects(ContextMutablePtr, LoadingStrictnessLev throw Exception(ErrorCodes::LOGICAL_ERROR, "Not implemented"); } +static void setReplicatedEngine(ASTCreateQuery * create_query, ContextPtr context) +{ + auto * storage = create_query->storage; + + /// Get replicated engine + /// Set uuid explicitly, because it is forbidden to use the 'uuid' macro without ON CLUSTER + const auto & config = context->getConfigRef(); + String replica_path = StorageReplicatedMergeTree::getDefaultZooKeeperPath(config); + replica_path = boost::algorithm::replace_all_copy(replica_path, "{uuid}", fmt::format("{}", create_query->uuid)); + String replica_name = StorageReplicatedMergeTree::getDefaultReplicaName(config); + + auto args = std::make_shared(); + args->children.push_back(std::make_shared(replica_path)); + args->children.push_back(std::make_shared(replica_name)); + + /// Add old engine's arguments + if (storage->engine->arguments) + { + for (size_t i = 0; i < storage->engine->arguments->children.size(); ++i) + args->children.push_back(storage->engine->arguments->children[i]->clone()); + } + + auto engine = std::make_shared(); + engine->name = "Replicated" + storage->engine->name; + engine->arguments = args; + + /// Set new engine for the old query + create_query->storage->set(create_query->storage->engine, engine->clone()); +} + void DatabaseOrdinary::convertMergeTreeToReplicatedIfNeeded(ASTPtr ast, const QualifiedTableName & qualified_name, const String & file_name) { fs::path path(getMetadataPath()); @@ -84,32 +114,9 @@ void DatabaseOrdinary::convertMergeTreeToReplicatedIfNeeded(ASTPtr ast, const Qu throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Table engine conversion to replicated is supported only for Atomic databases. Convert your database engine to Atomic first."); - LOG_INFO(log, "Found convert_to_replicated flag for table {}. Will try to change it's engine in metadata to replicated table.", backQuote(qualified_name.getFullName())); + LOG_INFO(log, "Found convert_to_replicated flag for table {}. Will try to change it's engine in metadata to replicated.", backQuote(qualified_name.getFullName())); - /// Get storage definition - /// Set uuid explicitly, because it is forbidden to use the 'uuid' macro without ON CLUSTER - auto * storage = create_query->storage; - - const auto & config = getContext()->getConfigRef(); - String replica_path = StorageReplicatedMergeTree::getDefaultZooKeeperPath(config); - replica_path = boost::algorithm::replace_all_copy(replica_path, "{uuid}", fmt::format("{}", create_query->uuid)); - String replica_name = StorageReplicatedMergeTree::getDefaultReplicaName(config); - String replicated_args = fmt::format("('{}', '{}')", replica_path, replica_name); - String replicated_engine = "ENGINE = Replicated" + storage->engine->name + replicated_args; - - ParserStorage parser_storage{ParserStorage::TABLE_ENGINE}; - auto replicated_storage_ast = parseQuery(parser_storage, replicated_engine, 0, DBMS_DEFAULT_MAX_PARSER_DEPTH); - auto * replicated_storage = replicated_storage_ast->as(); - - /// Add old engine's arguments - if (storage->engine->arguments) - { - for (size_t i = 0; i < storage->engine->arguments->children.size(); ++i) - replicated_storage->engine->arguments->children.push_back(storage->engine->arguments->children[i]->clone()); - } - - /// Set new engine for the old query - create_query->storage->set(create_query->storage->engine, replicated_storage->engine->clone()); + setReplicatedEngine(create_query, getContext()); /// Write changes to metadata String table_metadata_path = full_path; From 697b4d5fa177fefe01791ad4235b8938b2948b0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9A=D0=B8=D1=80=D0=B8=D0=BB=D0=BB=20=D0=93=D0=B0=D1=80?= =?UTF-8?q?=D0=B1=D0=B0=D1=80?= Date: Thu, 28 Dec 2023 02:35:13 +0300 Subject: [PATCH 031/530] No need to expand uuid macro --- src/Databases/DatabaseOrdinary.cpp | 2 -- .../test_modify_engine_on_restart/test_args.py | 13 ++----------- 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/src/Databases/DatabaseOrdinary.cpp b/src/Databases/DatabaseOrdinary.cpp index 770c223d931..33805f41153 100644 --- a/src/Databases/DatabaseOrdinary.cpp +++ b/src/Databases/DatabaseOrdinary.cpp @@ -67,10 +67,8 @@ static void setReplicatedEngine(ASTCreateQuery * create_query, ContextPtr contex auto * storage = create_query->storage; /// Get replicated engine - /// Set uuid explicitly, because it is forbidden to use the 'uuid' macro without ON CLUSTER const auto & config = context->getConfigRef(); String replica_path = StorageReplicatedMergeTree::getDefaultZooKeeperPath(config); - replica_path = boost::algorithm::replace_all_copy(replica_path, "{uuid}", fmt::format("{}", create_query->uuid)); String replica_name = StorageReplicatedMergeTree::getDefaultReplicaName(config); auto args = std::make_shared(); diff --git a/tests/integration/test_modify_engine_on_restart/test_args.py b/tests/integration/test_modify_engine_on_restart/test_args.py index e3d917b0079..e509fb3e287 100644 --- a/tests/integration/test_modify_engine_on_restart/test_args.py +++ b/tests/integration/test_modify_engine_on_restart/test_args.py @@ -54,15 +54,6 @@ def check_tables(): == "collapsing_ver\nreplacing_ver" ) - replacing_uuid = q( - ch1, - f"SELECT uuid FROM system.tables WHERE database = '{database_name}' and name = 'replacing_ver'", - ).strip() - collapsing_uuid = q( - ch1, - f"SELECT uuid FROM system.tables WHERE database = '{database_name}' and name = 'collapsing_ver'", - ).strip() - # Check engines assert ( q( @@ -71,7 +62,7 @@ def check_tables(): ) .strip() .startswith( - f"ReplicatedReplacingMergeTree(\\'/clickhouse/tables/{replacing_uuid}/{{shard}}\\', \\'{{replica}}\\', D)" + "ReplicatedReplacingMergeTree(\\'/clickhouse/tables/{uuid}/{shard}\\', \\'{replica}\\', D)" ) ) assert ( @@ -81,7 +72,7 @@ def check_tables(): ) .strip() .startswith( - f"ReplicatedVersionedCollapsingMergeTree(\\'/clickhouse/tables/{collapsing_uuid}/{{shard}}\\', \\'{{replica}}\\', Sign, Version)" + "ReplicatedVersionedCollapsingMergeTree(\\'/clickhouse/tables/{uuid}/{shard}\\', \\'{replica}\\', Sign, Version)" ) ) From 764791809b1a63158b55c98db65c835dbc34b092 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9A=D0=B8=D1=80=D0=B8=D0=BB=D0=BB=20=D0=93=D0=B0=D1=80?= =?UTF-8?q?=D0=B1=D0=B0=D1=80?= Date: Thu, 28 Dec 2023 03:33:56 +0300 Subject: [PATCH 032/530] Test for unusual zk path --- .../configs/config.d/clusters_unusual.xml | 20 ++++ .../test_args.py | 2 +- .../test_ordinary.py | 4 + .../test_unusual_path.py | 107 ++++++++++++++++++ 4 files changed, 132 insertions(+), 1 deletion(-) create mode 100644 tests/integration/test_modify_engine_on_restart/configs/config.d/clusters_unusual.xml create mode 100644 tests/integration/test_modify_engine_on_restart/test_unusual_path.py diff --git a/tests/integration/test_modify_engine_on_restart/configs/config.d/clusters_unusual.xml b/tests/integration/test_modify_engine_on_restart/configs/config.d/clusters_unusual.xml new file mode 100644 index 00000000000..812291335b8 --- /dev/null +++ b/tests/integration/test_modify_engine_on_restart/configs/config.d/clusters_unusual.xml @@ -0,0 +1,20 @@ + + + + + true + + ch1 + 9000 + + + + + + + 01 + + +/lol/kek/'/{uuid} + + diff --git a/tests/integration/test_modify_engine_on_restart/test_args.py b/tests/integration/test_modify_engine_on_restart/test_args.py index e509fb3e287..43a980c5d59 100644 --- a/tests/integration/test_modify_engine_on_restart/test_args.py +++ b/tests/integration/test_modify_engine_on_restart/test_args.py @@ -96,7 +96,7 @@ def set_convert_flags(): ) -def test_modify_engine_on_restart(started_cluster): +def test_modify_engine_on_restart_with_arguments(started_cluster): ch1.query("CREATE DATABASE " + database_name) create_tables() diff --git a/tests/integration/test_modify_engine_on_restart/test_ordinary.py b/tests/integration/test_modify_engine_on_restart/test_ordinary.py index 7e9830c5166..dbe93869ede 100644 --- a/tests/integration/test_modify_engine_on_restart/test_ordinary.py +++ b/tests/integration/test_modify_engine_on_restart/test_ordinary.py @@ -29,12 +29,14 @@ def started_cluster(): def q(node, query): return node.query(database=database_name, sql=query) + def create_tables(): q( ch1, "CREATE TABLE mt ( A Int64, D Date, S String ) ENGINE MergeTree() PARTITION BY toYYYYMM(D) ORDER BY A;", ) + def check_tables(): # Check tables exists assert ( @@ -71,6 +73,7 @@ def set_convert_flags(): ] ) + def remove_convert_flags(): ch1.exec_in_container( [ @@ -80,6 +83,7 @@ def remove_convert_flags(): ] ) + def test_modify_engine_on_restart_ordinary_database(started_cluster): ch1.query( sql=f"CREATE DATABASE {database_name} ENGINE = Ordinary", diff --git a/tests/integration/test_modify_engine_on_restart/test_unusual_path.py b/tests/integration/test_modify_engine_on_restart/test_unusual_path.py new file mode 100644 index 00000000000..971afa3d142 --- /dev/null +++ b/tests/integration/test_modify_engine_on_restart/test_unusual_path.py @@ -0,0 +1,107 @@ +import pytest +from helpers.cluster import ClickHouseCluster + +cluster = ClickHouseCluster(__file__) +ch1 = cluster.add_instance( + "ch1", + main_configs=[ + "configs/config.d/clusters_unusual.xml", + "configs/config.d/distributed_ddl.xml", + ], + with_zookeeper=True, + macros={"replica": "node1"}, + stay_alive=True, +) + +database_name = "modify_engine_unusual_path" + + +@pytest.fixture(scope="module") +def started_cluster(): + try: + cluster.start() + yield cluster + + finally: + cluster.shutdown() + + +def q(node, query): + return node.query(database=database_name, sql=query) + + +def create_tables(): + # Check one argument + q( + ch1, + "CREATE TABLE replacing_ver ( A Int64, D Date, S String ) ENGINE = ReplacingMergeTree(D) PARTITION BY toYYYYMM(D) ORDER BY A", + ) + + # Check more than one argument + q( + ch1, + "CREATE TABLE collapsing_ver ( ID UInt64, Sign Int8, Version UInt8 ) ENGINE = VersionedCollapsingMergeTree(Sign, Version) ORDER BY ID", + ) + + +def check_tables(): + # Check tables exists + assert ( + q( + ch1, + "SHOW TABLES", + ).strip() + == "collapsing_ver\nreplacing_ver" + ) + + # Check engines + assert ( + q( + ch1, + f"SELECT engine_full FROM system.tables WHERE database = '{database_name}' and name = 'replacing_ver'", + ) + .strip() + .startswith( + "ReplicatedReplacingMergeTree(\\'/lol/kek/\\\\\\'/{uuid}\\', \\'{replica}\\', D)" + ) + ) + assert ( + q( + ch1, + f"SELECT engine_full FROM system.tables WHERE database = '{database_name}' and name = 'collapsing_ver'", + ) + .strip() + .startswith( + "ReplicatedVersionedCollapsingMergeTree(\\'/lol/kek/\\\\\\'/{uuid}\\', \\'{replica}\\', Sign, Version)" + ) + ) + + +def set_convert_flags(): + for table in ["replacing_ver", "collapsing_ver"]: + ch1.exec_in_container( + [ + "bash", + "-c", + f"mkdir /var/lib/clickhouse/data/{database_name}/{table}/flags", + ] + ) + ch1.exec_in_container( + [ + "bash", + "-c", + f"touch /var/lib/clickhouse/data/{database_name}/{table}/flags/convert_to_replicated", + ] + ) + + +def test_modify_engine_on_restart_with_unusual_path(started_cluster): + ch1.query("CREATE DATABASE " + database_name) + + create_tables() + + set_convert_flags() + + ch1.restart_clickhouse() + + check_tables() From 780cc724b2cb9b8fe50c602db8411540d774c480 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9A=D0=B8=D1=80=D0=B8=D0=BB=D0=BB=20=D0=93=D0=B0=D1=80?= =?UTF-8?q?=D0=B1=D0=B0=D1=80?= Date: Mon, 8 Jan 2024 07:08:26 +0300 Subject: [PATCH 033/530] Check table metadata, move restore to a separate method --- src/Databases/DatabaseOrdinary.cpp | 97 +++++++++++++++-------- src/Databases/DatabaseOrdinary.h | 1 + src/Storages/StorageReplicatedMergeTree.h | 2 + 3 files changed, 68 insertions(+), 32 deletions(-) diff --git a/src/Databases/DatabaseOrdinary.cpp b/src/Databases/DatabaseOrdinary.cpp index 33805f41153..28797b51727 100644 --- a/src/Databases/DatabaseOrdinary.cpp +++ b/src/Databases/DatabaseOrdinary.cpp @@ -45,6 +45,8 @@ namespace ErrorCodes static constexpr size_t METADATA_FILE_BUFFER_SIZE = 32768; +static constexpr const char * const CONVERT_TO_REPLICATED_FLAG_NAME = "convert_to_replicated"; + DatabaseOrdinary::DatabaseOrdinary(const String & name_, const String & metadata_path_, ContextPtr context_) : DatabaseOrdinary(name_, metadata_path_, "data/" + escapeForFileName(name_) + "/", "DatabaseOrdinary (" + name_ + ")", context_) { @@ -90,6 +92,11 @@ static void setReplicatedEngine(ASTCreateQuery * create_query, ContextPtr contex create_query->storage->set(create_query->storage->engine, engine->clone()); } +static fs::path getConvertToReplicatedFlagPath(ContextPtr context, const QualifiedTableName & name) +{ + return fs::path(context->getPath()) / "data" / name.database / name.table / "flags" / CONVERT_TO_REPLICATED_FLAG_NAME; +} + void DatabaseOrdinary::convertMergeTreeToReplicatedIfNeeded(ASTPtr ast, const QualifiedTableName & qualified_name, const String & file_name) { fs::path path(getMetadataPath()); @@ -101,9 +108,9 @@ void DatabaseOrdinary::convertMergeTreeToReplicatedIfNeeded(ASTPtr ast, const Qu if (!create_query->storage || !create_query->storage->engine->name.ends_with("MergeTree") || create_query->storage->engine->name.starts_with("Replicated")) return; - auto convert_to_replicated_flag_path = fs::path(getContext()->getPath()) / "data" / qualified_name.database / qualified_name.table / "flags" / "convert_to_replicated"; + auto convert_to_replicated_flag_path = getConvertToReplicatedFlagPath(getContext(), qualified_name); - LOG_DEBUG(log, "Searching for convert_to_replicated flag at {}.", backQuote(convert_to_replicated_flag_path.string())); + LOG_DEBUG(log, "Searching for {} flag at {}.", CONVERT_TO_REPLICATED_FLAG_NAME, backQuote(convert_to_replicated_flag_path.string())); if (!fs::exists(convert_to_replicated_flag_path)) return; @@ -112,7 +119,7 @@ void DatabaseOrdinary::convertMergeTreeToReplicatedIfNeeded(ASTPtr ast, const Qu throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Table engine conversion to replicated is supported only for Atomic databases. Convert your database engine to Atomic first."); - LOG_INFO(log, "Found convert_to_replicated flag for table {}. Will try to change it's engine in metadata to replicated.", backQuote(qualified_name.getFullName())); + LOG_INFO(log, "Found {} flag for table {}. Will try to change it's engine in metadata to replicated.", CONVERT_TO_REPLICATED_FLAG_NAME, backQuote(qualified_name.getFullName())); setReplicatedEngine(create_query, getContext()); @@ -132,8 +139,9 @@ void DatabaseOrdinary::convertMergeTreeToReplicatedIfNeeded(ASTPtr ast, const Qu LOG_INFO( log, - "Engine of table {} is set to replicated in metadata. Not removing convert_to_replicated flag until table is loaded and metadata in zookeeper is restored.", - backQuote(qualified_name.getFullName()) + "Engine of table {} is set to replicated in metadata. Not removing {} flag until table is loaded and metadata in zookeeper is restored.", + backQuote(qualified_name.getFullName()), + CONVERT_TO_REPLICATED_FLAG_NAME ); } @@ -264,6 +272,56 @@ LoadTaskPtr DatabaseOrdinary::loadTableFromMetadataAsync( return load_table[name.table] = makeLoadTask(async_loader, {job}); } +void DatabaseOrdinary::restoreMetadataAfterConvertingToReplicated(StoragePtr table, const QualifiedTableName & name) +{ + if (auto * rmt = table->as()) + { + auto convert_to_replicated_flag_path = getConvertToReplicatedFlagPath(getContext(), name); + if (!fs::exists(convert_to_replicated_flag_path)) + return; + + auto has_metadata = rmt->hasMetadataInZooKeeper(); + if (has_metadata.has_value()) + { + if (*has_metadata) + { + LOG_INFO + ( + log, + "Table {} already has metatada in ZooKeeper.", + backQuote(name.getFullName()) + ); + } + else + { + rmt->restoreMetadataInZooKeeper(); + LOG_INFO + ( + log, + "Metadata in ZooKeeper for {} is restored.", + backQuote(name.getFullName()) + ); + } + } + else + { + LOG_INFO + ( + log, + "No connection to ZooKeeper, can't restore metadata for {} in ZooKeeper after conversion. Run SYSTEM RESTORE REPLICA while connected to ZooKeeper.", + backQuote(name.getFullName()) + ); + } + fs::remove(convert_to_replicated_flag_path); + LOG_INFO + ( + log, + "Removing convert to replicated flag for {}.", + backQuote(name.getFullName()) + ); + } +} + LoadTaskPtr DatabaseOrdinary::startupTableAsync( AsyncLoader & async_loader, LoadJobSet startup_after, @@ -294,33 +352,8 @@ LoadTaskPtr DatabaseOrdinary::startupTableAsync( /// If table is ReplicatedMergeTree after conversion from MergeTree, /// it is in readonly mode due to metadata in zookeeper missing. - if (auto * rmt = table->as()) - { - auto convert_to_replicated_flag_path = fs::path(getContext()->getPath()) / "data" / name.database / name.table / "flags" / "convert_to_replicated"; - if (fs::exists(convert_to_replicated_flag_path)) - { - if (rmt->isTableReadOnly()) - { - rmt->restoreMetadataInZooKeeper(); - LOG_INFO - ( - log, - "Metadata in zookeeper for {} is restored. Removing convert_to_replicated flag.", - backQuote(name.getFullName()) - ); - } - else - { - LOG_INFO - ( - log, - "Table {} is not in readonly mode but convert_to_replicated flag is set. Removing flag.", - backQuote(name.getFullName()) - ); - } - fs::remove(convert_to_replicated_flag_path); - } - } + restoreMetadataAfterConvertingToReplicated(table, name); + logAboutProgress(log, ++tables_started, total_tables_to_startup, startup_watch); } else diff --git a/src/Databases/DatabaseOrdinary.h b/src/Databases/DatabaseOrdinary.h index 6c0829f0da8..25020e4abbd 100644 --- a/src/Databases/DatabaseOrdinary.h +++ b/src/Databases/DatabaseOrdinary.h @@ -83,6 +83,7 @@ protected: private: void convertMergeTreeToReplicatedIfNeeded(ASTPtr ast, const QualifiedTableName & qualified_name, const String & file_name); + void restoreMetadataAfterConvertingToReplicated(StoragePtr table, const QualifiedTableName & name); }; } diff --git a/src/Storages/StorageReplicatedMergeTree.h b/src/Storages/StorageReplicatedMergeTree.h index f68a7561b93..e398a7b6b05 100644 --- a/src/Storages/StorageReplicatedMergeTree.h +++ b/src/Storages/StorageReplicatedMergeTree.h @@ -364,6 +364,8 @@ public: bool isTableReadOnly () { return is_readonly; } + std::optional hasMetadataInZooKeeper () { return has_metadata_in_zookeeper; } + /// Get a sequential consistent view of current parts. ReplicatedMergeTreeQuorumAddedParts::PartitionIdToMaxBlock getMaxAddedBlocks() const; From c7b14d190e6ab32775bd8cf5a5eefbf0e36bb58d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9A=D0=B8=D1=80=D0=B8=D0=BB=D0=BB=20=D0=93=D0=B0=D1=80?= =?UTF-8?q?=D0=B1=D0=B0=D1=80?= Date: Mon, 8 Jan 2024 07:09:38 +0300 Subject: [PATCH 034/530] Remove flags directory --- .../table-engines/mergetree-family/replication.md | 5 ++--- src/Databases/DatabaseOrdinary.cpp | 2 +- .../test_modify_engine_on_restart/test.py | 14 ++------------ .../test_modify_engine_on_restart/test_args.py | 9 +-------- .../test_modify_engine_on_restart/test_mv.py | 9 +-------- .../test_modify_engine_on_restart/test_ordinary.py | 11 ++--------- .../test_unusual_path.py | 9 +-------- 7 files changed, 10 insertions(+), 49 deletions(-) diff --git a/docs/en/engines/table-engines/mergetree-family/replication.md b/docs/en/engines/table-engines/mergetree-family/replication.md index 6ebaf4db51d..2bdfd81f34e 100644 --- a/docs/en/engines/table-engines/mergetree-family/replication.md +++ b/docs/en/engines/table-engines/mergetree-family/replication.md @@ -304,9 +304,8 @@ We use the term `MergeTree` to refer to all table engines in the `MergeTree fami If you had a `MergeTree` table that was manually replicated, you can convert it to a replicated table. You might need to do this if you have already collected a large amount of data in a `MergeTree` table and now you want to enable replication. -`MergeTree` table can be automatically converted on server restart if `convert_to_replicated` flag is set at the table's flags directory (`/var/lib/clickhouse/data/db_name/table_name/flags/`). -This directory is not created by default, so you need to create it first. -Then simply create empty `convert_to_replicated` file and the table will be loaded as replicated on next server restart. +`MergeTree` table can be automatically converted on server restart if `convert_to_replicated` flag is set at the table's directory (`/var/lib/clickhouse/data/db_name/table_name/`). +Create empty `convert_to_replicated` file and the table will be loaded as replicated on next server restart. There is also a manual way to do this without server restart. diff --git a/src/Databases/DatabaseOrdinary.cpp b/src/Databases/DatabaseOrdinary.cpp index 28797b51727..c8e70954095 100644 --- a/src/Databases/DatabaseOrdinary.cpp +++ b/src/Databases/DatabaseOrdinary.cpp @@ -94,7 +94,7 @@ static void setReplicatedEngine(ASTCreateQuery * create_query, ContextPtr contex static fs::path getConvertToReplicatedFlagPath(ContextPtr context, const QualifiedTableName & name) { - return fs::path(context->getPath()) / "data" / name.database / name.table / "flags" / CONVERT_TO_REPLICATED_FLAG_NAME; + return fs::path(context->getPath()) / "data" / name.database / name.table / CONVERT_TO_REPLICATED_FLAG_NAME; } void DatabaseOrdinary::convertMergeTreeToReplicatedIfNeeded(ASTPtr ast, const QualifiedTableName & qualified_name, const String & file_name) diff --git a/tests/integration/test_modify_engine_on_restart/test.py b/tests/integration/test_modify_engine_on_restart/test.py index 4f0e9c9da69..0153a615064 100644 --- a/tests/integration/test_modify_engine_on_restart/test.py +++ b/tests/integration/test_modify_engine_on_restart/test.py @@ -116,26 +116,16 @@ def set_convert_flags(): [ "bash", "-c", - f"mkdir /var/lib/clickhouse/data/{database_name}/{table}/flags", - ] - ) - ch1.exec_in_container( - [ - "bash", - "-c", - f"touch /var/lib/clickhouse/data/{database_name}/{table}/flags/convert_to_replicated", + f"touch /var/lib/clickhouse/data/{database_name}/{table}/convert_to_replicated", ] ) # Set flag to not MergeTree table to check that nothing happens - ch1.exec_in_container( - ["bash", "-c", f"mkdir /var/lib/clickhouse/data/{database_name}/log/flags"] - ) ch1.exec_in_container( [ "bash", "-c", - f"touch /var/lib/clickhouse/data/{database_name}/log/flags/convert_to_replicated", + f"touch /var/lib/clickhouse/data/{database_name}/log/convert_to_replicated", ] ) diff --git a/tests/integration/test_modify_engine_on_restart/test_args.py b/tests/integration/test_modify_engine_on_restart/test_args.py index 43a980c5d59..94b541cfb60 100644 --- a/tests/integration/test_modify_engine_on_restart/test_args.py +++ b/tests/integration/test_modify_engine_on_restart/test_args.py @@ -84,14 +84,7 @@ def set_convert_flags(): [ "bash", "-c", - f"mkdir /var/lib/clickhouse/data/{database_name}/{table}/flags", - ] - ) - ch1.exec_in_container( - [ - "bash", - "-c", - f"touch /var/lib/clickhouse/data/{database_name}/{table}/flags/convert_to_replicated", + f"touch /var/lib/clickhouse/data/{database_name}/{table}/convert_to_replicated", ] ) diff --git a/tests/integration/test_modify_engine_on_restart/test_mv.py b/tests/integration/test_modify_engine_on_restart/test_mv.py index b624eeb8c32..cdb76fb0742 100644 --- a/tests/integration/test_modify_engine_on_restart/test_mv.py +++ b/tests/integration/test_modify_engine_on_restart/test_mv.py @@ -121,14 +121,7 @@ def set_convert_flags(): [ "bash", "-c", - f"mkdir /var/lib/clickhouse/data/{database_name}/{table}/flags", - ] - ) - ch1.exec_in_container( - [ - "bash", - "-c", - f"touch /var/lib/clickhouse/data/{database_name}/{table}/flags/convert_to_replicated", + f"touch /var/lib/clickhouse/data/{database_name}/{table}/convert_to_replicated", ] ) diff --git a/tests/integration/test_modify_engine_on_restart/test_ordinary.py b/tests/integration/test_modify_engine_on_restart/test_ordinary.py index dbe93869ede..ce2d17d578e 100644 --- a/tests/integration/test_modify_engine_on_restart/test_ordinary.py +++ b/tests/integration/test_modify_engine_on_restart/test_ordinary.py @@ -62,14 +62,7 @@ def set_convert_flags(): [ "bash", "-c", - f"mkdir /var/lib/clickhouse/data/{database_name}/mt/flags", - ] - ) - ch1.exec_in_container( - [ - "bash", - "-c", - f"touch /var/lib/clickhouse/data/{database_name}/mt/flags/convert_to_replicated", + f"touch /var/lib/clickhouse/data/{database_name}/mt/convert_to_replicated", ] ) @@ -79,7 +72,7 @@ def remove_convert_flags(): [ "bash", "-c", - f"rm -rf /var/lib/clickhouse/data/{database_name}/mt/flags", + f"rm /var/lib/clickhouse/data/{database_name}/mt/convert_to_replicated", ] ) diff --git a/tests/integration/test_modify_engine_on_restart/test_unusual_path.py b/tests/integration/test_modify_engine_on_restart/test_unusual_path.py index 971afa3d142..e7be8f11c7b 100644 --- a/tests/integration/test_modify_engine_on_restart/test_unusual_path.py +++ b/tests/integration/test_modify_engine_on_restart/test_unusual_path.py @@ -83,14 +83,7 @@ def set_convert_flags(): [ "bash", "-c", - f"mkdir /var/lib/clickhouse/data/{database_name}/{table}/flags", - ] - ) - ch1.exec_in_container( - [ - "bash", - "-c", - f"touch /var/lib/clickhouse/data/{database_name}/{table}/flags/convert_to_replicated", + f"touch /var/lib/clickhouse/data/{database_name}/{table}/convert_to_replicated", ] ) From bebb2c9dca9371cf2478e79c5939fe6569423585 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9A=D0=B8=D1=80=D0=B8=D0=BB=D0=BB=20=D0=93=D0=B0=D1=80?= =?UTF-8?q?=D0=B1=D0=B0=D1=80?= Date: Tue, 9 Jan 2024 04:22:40 +0300 Subject: [PATCH 035/530] Common test functions, check flags are removed --- .../test_modify_engine_on_restart/common.py | 23 ++++++++++++++++ .../test_modify_engine_on_restart/test.py | 26 +++---------------- .../test_args.py | 17 +++--------- .../test_modify_engine_on_restart/test_mv.py | 16 +++--------- .../test_ordinary.py | 13 ++-------- .../test_unusual_path.py | 16 +++--------- 6 files changed, 41 insertions(+), 70 deletions(-) create mode 100644 tests/integration/test_modify_engine_on_restart/common.py diff --git a/tests/integration/test_modify_engine_on_restart/common.py b/tests/integration/test_modify_engine_on_restart/common.py new file mode 100644 index 00000000000..f0fdc371a70 --- /dev/null +++ b/tests/integration/test_modify_engine_on_restart/common.py @@ -0,0 +1,23 @@ +from helpers.cluster import ClickHouseCluster + + +def check_flags_deleted(node, database_name, tables): + for table in tables: + assert "convert_to_replicated" not in node.exec_in_container( + [ + "bash", + "-c", + f"ls /var/lib/clickhouse/data/{database_name}/{table}/", + ] + ) + + +def set_convert_flags(node, database_name, tables): + for table in tables: + node.exec_in_container( + [ + "bash", + "-c", + f"touch /var/lib/clickhouse/data/{database_name}/{table}/convert_to_replicated", + ] + ) diff --git a/tests/integration/test_modify_engine_on_restart/test.py b/tests/integration/test_modify_engine_on_restart/test.py index 0153a615064..289b25dd89e 100644 --- a/tests/integration/test_modify_engine_on_restart/test.py +++ b/tests/integration/test_modify_engine_on_restart/test.py @@ -1,4 +1,5 @@ import pytest +from test_modify_engine_on_restart.common import check_flags_deleted, set_convert_flags from helpers.cluster import ClickHouseCluster cluster = ClickHouseCluster(__file__) @@ -109,27 +110,6 @@ def check_tables(converted): ) -def set_convert_flags(): - # Set convert flag on actually convertable tables - for table in ["rmt", "replacing"]: - ch1.exec_in_container( - [ - "bash", - "-c", - f"touch /var/lib/clickhouse/data/{database_name}/{table}/convert_to_replicated", - ] - ) - - # Set flag to not MergeTree table to check that nothing happens - ch1.exec_in_container( - [ - "bash", - "-c", - f"touch /var/lib/clickhouse/data/{database_name}/log/convert_to_replicated", - ] - ) - - def check_replica_added(): # Add replica to check if zookeeper path is correct and consistent with table uuid @@ -166,10 +146,12 @@ def test_modify_engine_on_restart(started_cluster): check_tables(False) - set_convert_flags() + set_convert_flags(ch1, database_name, ["rmt", "replacing", "log"]) ch1.restart_clickhouse() + check_flags_deleted(ch1, database_name, ["rmt", "replacing"]) + check_tables(True) check_replica_added() diff --git a/tests/integration/test_modify_engine_on_restart/test_args.py b/tests/integration/test_modify_engine_on_restart/test_args.py index 94b541cfb60..f83d540bfb9 100644 --- a/tests/integration/test_modify_engine_on_restart/test_args.py +++ b/tests/integration/test_modify_engine_on_restart/test_args.py @@ -1,4 +1,5 @@ import pytest +from test_modify_engine_on_restart.common import check_flags_deleted, set_convert_flags from helpers.cluster import ClickHouseCluster cluster = ClickHouseCluster(__file__) @@ -77,25 +78,15 @@ def check_tables(): ) -def set_convert_flags(): - # Set convert flag on actually convertable tables - for table in ["replacing_ver", "collapsing_ver"]: - ch1.exec_in_container( - [ - "bash", - "-c", - f"touch /var/lib/clickhouse/data/{database_name}/{table}/convert_to_replicated", - ] - ) - - def test_modify_engine_on_restart_with_arguments(started_cluster): ch1.query("CREATE DATABASE " + database_name) create_tables() - set_convert_flags() + set_convert_flags(ch1, database_name, ["replacing_ver", "collapsing_ver"]) ch1.restart_clickhouse() + check_flags_deleted(ch1, database_name, ["replacing_ver", "collapsing_ver"]) + check_tables() diff --git a/tests/integration/test_modify_engine_on_restart/test_mv.py b/tests/integration/test_modify_engine_on_restart/test_mv.py index cdb76fb0742..30cb2ddc5e5 100644 --- a/tests/integration/test_modify_engine_on_restart/test_mv.py +++ b/tests/integration/test_modify_engine_on_restart/test_mv.py @@ -1,4 +1,5 @@ import pytest +from test_modify_engine_on_restart.common import check_flags_deleted, set_convert_flags from helpers.cluster import ClickHouseCluster cluster = ClickHouseCluster(__file__) @@ -115,17 +116,6 @@ def check_tables(converted): assert q(ch1, "SELECT count() FROM hourly_data").strip() == "8" -def set_convert_flags(): - for table in ["hourly_data", "monthly_aggregated_data"]: - ch1.exec_in_container( - [ - "bash", - "-c", - f"touch /var/lib/clickhouse/data/{database_name}/{table}/convert_to_replicated", - ] - ) - - def test_modify_engine_on_restart_with_materialized_view(started_cluster): ch1.query(f"CREATE DATABASE {database_name}") @@ -133,8 +123,10 @@ def test_modify_engine_on_restart_with_materialized_view(started_cluster): check_tables(False) - set_convert_flags() + set_convert_flags(ch1, database_name, ["hourly_data", "monthly_aggregated_data"]) ch1.restart_clickhouse() + check_flags_deleted(ch1, database_name, ["hourly_data", "monthly_aggregated_data"]) + check_tables(True) diff --git a/tests/integration/test_modify_engine_on_restart/test_ordinary.py b/tests/integration/test_modify_engine_on_restart/test_ordinary.py index ce2d17d578e..fd86417a216 100644 --- a/tests/integration/test_modify_engine_on_restart/test_ordinary.py +++ b/tests/integration/test_modify_engine_on_restart/test_ordinary.py @@ -1,4 +1,5 @@ import pytest +from test_modify_engine_on_restart.common import check_flags_deleted, set_convert_flags from helpers.cluster import ClickHouseCluster cluster = ClickHouseCluster(__file__) @@ -57,16 +58,6 @@ def check_tables(): ) -def set_convert_flags(): - ch1.exec_in_container( - [ - "bash", - "-c", - f"touch /var/lib/clickhouse/data/{database_name}/mt/convert_to_replicated", - ] - ) - - def remove_convert_flags(): ch1.exec_in_container( [ @@ -87,7 +78,7 @@ def test_modify_engine_on_restart_ordinary_database(started_cluster): check_tables() - set_convert_flags() + set_convert_flags(ch1, database_name, ["mt"]) cannot_start = False try: diff --git a/tests/integration/test_modify_engine_on_restart/test_unusual_path.py b/tests/integration/test_modify_engine_on_restart/test_unusual_path.py index e7be8f11c7b..e82f48e8b34 100644 --- a/tests/integration/test_modify_engine_on_restart/test_unusual_path.py +++ b/tests/integration/test_modify_engine_on_restart/test_unusual_path.py @@ -1,4 +1,5 @@ import pytest +from test_modify_engine_on_restart.common import check_flags_deleted, set_convert_flags from helpers.cluster import ClickHouseCluster cluster = ClickHouseCluster(__file__) @@ -77,24 +78,15 @@ def check_tables(): ) -def set_convert_flags(): - for table in ["replacing_ver", "collapsing_ver"]: - ch1.exec_in_container( - [ - "bash", - "-c", - f"touch /var/lib/clickhouse/data/{database_name}/{table}/convert_to_replicated", - ] - ) - - def test_modify_engine_on_restart_with_unusual_path(started_cluster): ch1.query("CREATE DATABASE " + database_name) create_tables() - set_convert_flags() + set_convert_flags(ch1, database_name, ["replacing_ver", "collapsing_ver"]) ch1.restart_clickhouse() + check_flags_deleted(ch1, database_name, ["replacing_ver", "collapsing_ver"]) + check_tables() From b8744c3510542bfd4b3a0b1e724d9ca6620bd6ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9A=D0=B8=D1=80=D0=B8=D0=BB=D0=BB=20=D0=93=D0=B0=D1=80?= =?UTF-8?q?=D0=B1=D0=B0=D1=80?= Date: Tue, 9 Jan 2024 05:56:42 +0300 Subject: [PATCH 036/530] Remove flag on restore metadata exception --- src/Databases/DatabaseOrdinary.cpp | 44 ++++++++++++++++++------------ 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/src/Databases/DatabaseOrdinary.cpp b/src/Databases/DatabaseOrdinary.cpp index 1c23cc4b5e6..dcc76e9e8b8 100644 --- a/src/Databases/DatabaseOrdinary.cpp +++ b/src/Databases/DatabaseOrdinary.cpp @@ -283,7 +283,16 @@ void DatabaseOrdinary::restoreMetadataAfterConvertingToReplicated(StoragePtr tab return; auto has_metadata = rmt->hasMetadataInZooKeeper(); - if (has_metadata.has_value()) + if (!has_metadata.has_value()) + { + LOG_INFO + ( + log, + "No connection to ZooKeeper, can't restore metadata for {} in ZooKeeper after conversion. Run SYSTEM RESTORE REPLICA while connected to ZooKeeper.", + backQuote(name.getFullName()) + ); + } + else { if (*has_metadata) { @@ -296,24 +305,25 @@ void DatabaseOrdinary::restoreMetadataAfterConvertingToReplicated(StoragePtr tab } else { - rmt->restoreMetadataInZooKeeper(); - LOG_INFO - ( - log, - "Metadata in ZooKeeper for {} is restored.", - backQuote(name.getFullName()) - ); + try + { + rmt->restoreMetadataInZooKeeper(); + LOG_INFO + ( + log, + "Metadata in ZooKeeper for {} is restored.", + backQuote(name.getFullName()) + ); + } + catch (...) + { + /// Something unpredicted happened + /// Remove flag to allow server to restart + fs::remove(convert_to_replicated_flag_path); + throw; + } } } - else - { - LOG_INFO - ( - log, - "No connection to ZooKeeper, can't restore metadata for {} in ZooKeeper after conversion. Run SYSTEM RESTORE REPLICA while connected to ZooKeeper.", - backQuote(name.getFullName()) - ); - } fs::remove(convert_to_replicated_flag_path); LOG_INFO ( From 1ada60cf234b100176a0bdbc87d25e2acbb43e0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9A=D0=B8=D1=80=D0=B8=D0=BB=D0=BB=20=D0=93=D0=B0=D1=80?= =?UTF-8?q?=D0=B1=D0=B0=D1=80?= Date: Tue, 9 Jan 2024 06:09:55 +0300 Subject: [PATCH 037/530] Style fix --- src/Databases/DatabaseOrdinary.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Databases/DatabaseOrdinary.cpp b/src/Databases/DatabaseOrdinary.cpp index dcc76e9e8b8..d06691c2c45 100644 --- a/src/Databases/DatabaseOrdinary.cpp +++ b/src/Databases/DatabaseOrdinary.cpp @@ -43,6 +43,7 @@ namespace ErrorCodes { extern const int LOGICAL_ERROR; extern const int UNKNOWN_DATABASE_ENGINE; + extern const int NOT_IMPLEMENTED; } static constexpr size_t METADATA_FILE_BUFFER_SIZE = 32768; From b3cd4069b701bee4c3107de09edf54561f6284e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Mar=C3=ADn?= Date: Mon, 8 Jan 2024 22:20:36 +0100 Subject: [PATCH 038/530] Reimplement AggregateFunctionMin --- .../AggregateFunctionAny.cpp | 486 +++--- .../AggregateFunctionMax.cpp | 448 ++--- .../AggregateFunctionMin.cpp | 420 +++-- .../AggregateFunctionMinMaxAny.h | 1449 ----------------- .../AggregateFunctionSingleValueOrNull.cpp | 24 +- .../AggregateFunctionArgMinMax.cpp | 150 +- src/AggregateFunctions/HelpersMinMaxAny.h | 160 +- src/AggregateFunctions/SingleValueData.cpp | 476 ++++++ src/AggregateFunctions/SingleValueData.h | 723 ++++++++ src/Common/findExtreme.cpp | 119 +- src/Common/findExtreme.h | 14 +- 11 files changed, 2137 insertions(+), 2332 deletions(-) delete mode 100644 src/AggregateFunctions/AggregateFunctionMinMaxAny.h create mode 100644 src/AggregateFunctions/SingleValueData.cpp create mode 100644 src/AggregateFunctions/SingleValueData.h diff --git a/src/AggregateFunctions/AggregateFunctionAny.cpp b/src/AggregateFunctions/AggregateFunctionAny.cpp index a6010ff07c3..7a81cf53bfc 100644 --- a/src/AggregateFunctions/AggregateFunctionAny.cpp +++ b/src/AggregateFunctions/AggregateFunctionAny.cpp @@ -8,250 +8,250 @@ namespace DB { struct Settings; - -namespace ErrorCodes +// +//namespace ErrorCodes +//{ +// extern const int INCORRECT_DATA; +// extern const int LOGICAL_ERROR; +//} +// +//namespace +//{ +//struct AggregateFunctionAnyRespectNullsData +//{ +// enum Status : UInt8 +// { +// NotSet = 1, +// SetNull = 2, +// SetOther = 3 +// }; +// +// Status status = Status::NotSet; +// Field value; +// +// bool isSet() const { return status != Status::NotSet; } +// void setNull() { status = Status::SetNull; } +// void setOther() { status = Status::SetOther; } +//}; +// +//template +//class AggregateFunctionAnyRespectNulls final +// : public IAggregateFunctionDataHelper> +//{ +//public: +// using Data = AggregateFunctionAnyRespectNullsData; +// +// SerializationPtr serialization; +// const bool returns_nullable_type = false; +// +// explicit AggregateFunctionAnyRespectNulls(const DataTypePtr & type) +// : IAggregateFunctionDataHelper>({type}, {}, type) +// , serialization(type->getDefaultSerialization()) +// , returns_nullable_type(type->isNullable()) +// { +// } +// +// String getName() const override +// { +// if constexpr (First) +// return "any_respect_nulls"; +// else +// return "anyLast_respect_nulls"; +// } +// +// bool allocatesMemoryInArena() const override { return false; } +// +// void addNull(AggregateDataPtr __restrict place) const +// { +// chassert(returns_nullable_type); +// auto & d = this->data(place); +// if (First && d.isSet()) +// return; +// d.setNull(); +// } +// +// void add(AggregateDataPtr __restrict place, const IColumn ** columns, size_t row_num, Arena *) const override +// { +// if (columns[0]->isNullable()) +// { +// if (columns[0]->isNullAt(row_num)) +// return addNull(place); +// } +// auto & d = this->data(place); +// if (First && d.isSet()) +// return; +// d.setOther(); +// columns[0]->get(row_num, d.value); +// } +// +// void addManyDefaults(AggregateDataPtr __restrict place, const IColumn ** columns, size_t, Arena * arena) const override +// { +// if (columns[0]->isNullable()) +// addNull(place); +// else +// add(place, columns, 0, arena); +// } +// +// void addBatchSinglePlace( +// size_t row_begin, size_t row_end, AggregateDataPtr place, const IColumn ** columns, Arena * arena, ssize_t if_argument_pos) +// const override +// { +// if (if_argument_pos >= 0) +// { +// const auto & flags = assert_cast(*columns[if_argument_pos]).getData(); +// size_t size = row_end - row_begin; +// for (size_t i = 0; i < size; ++i) +// { +// size_t pos = First ? row_begin + i : row_end - 1 - i; +// if (flags[pos]) +// { +// add(place, columns, pos, arena); +// break; +// } +// } +// } +// else if (row_begin < row_end) +// { +// size_t pos = First ? row_begin : row_end - 1; +// add(place, columns, pos, arena); +// } +// } +// +// void addBatchSinglePlaceNotNull( +// size_t, size_t, AggregateDataPtr __restrict, const IColumn **, const UInt8 *, Arena *, ssize_t) const override +// { +// /// This should not happen since it means somebody else has preprocessed the data (NULLs or IFs) and might +// /// have discarded values that we need (NULLs) +// throw DB::Exception(ErrorCodes::LOGICAL_ERROR, "AggregateFunctionAnyRespectNulls::addBatchSinglePlaceNotNull called"); +// } +// +// void merge(AggregateDataPtr __restrict place, ConstAggregateDataPtr rhs, Arena *) const override +// { +// auto & d = this->data(place); +// if (First && d.isSet()) +// return; +// +// auto & other = this->data(rhs); +// if (other.isSet()) +// { +// d.status = other.status; +// d.value = other.value; +// } +// } +// +// void serialize(ConstAggregateDataPtr __restrict place, WriteBuffer & buf, std::optional /* version */) const override +// { +// auto & d = this->data(place); +// UInt8 k = d.status; +// +// writeBinaryLittleEndian(k, buf); +// if (k == Data::Status::SetOther) +// serialization->serializeBinary(d.value, buf, {}); +// } +// +// void deserialize(AggregateDataPtr place, ReadBuffer & buf, std::optional /* version */, Arena *) const override +// { +// auto & d = this->data(place); +// UInt8 k = Data::Status::NotSet; +// readBinaryLittleEndian(k, buf); +// d.status = static_cast(k); +// if (d.status == Data::Status::NotSet) +// return; +// else if (d.status == Data::Status::SetNull) +// { +// if (!returns_nullable_type) +// throw Exception(ErrorCodes::INCORRECT_DATA, "Incorrect type (NULL) in non-nullable {}State", getName()); +// return; +// } +// else if (d.status == Data::Status::SetOther) +// serialization->deserializeBinary(d.value, buf, {}); +// else +// throw Exception(ErrorCodes::INCORRECT_DATA, "Incorrect type ({}) in {}State", static_cast(k), getName()); +// } +// +// void insertResultInto(AggregateDataPtr __restrict place, IColumn & to, Arena *) const override +// { +// auto & d = this->data(place); +// if (d.status == Data::Status::SetOther) +// to.insert(d.value); +// else +// to.insertDefault(); +// } +// +// AggregateFunctionPtr getOwnNullAdapter( +// const AggregateFunctionPtr & original_function, +// const DataTypes & /*arguments*/, +// const Array & /*params*/, +// const AggregateFunctionProperties & /*properties*/) const override +// { +// return original_function; +// } +//}; +// +// +//template +//IAggregateFunction * createAggregateFunctionSingleValueRespectNulls( +// const String & name, const DataTypes & argument_types, const Array & parameters, const Settings *) +//{ +// assertNoParameters(name, parameters); +// assertUnary(name, argument_types); +// +// return new AggregateFunctionAnyRespectNulls(argument_types[0]); +//} +// +//AggregateFunctionPtr createAggregateFunctionAny(const std::string & name, const DataTypes & argument_types, const Array & parameters, const Settings * settings) +//{ +// return AggregateFunctionPtr(createAggregateFunctionSingleValue(name, argument_types, parameters, settings)); +//} +// +//AggregateFunctionPtr createAggregateFunctionAnyRespectNulls( +// const std::string & name, const DataTypes & argument_types, const Array & parameters, const Settings * settings) +//{ +// return AggregateFunctionPtr(createAggregateFunctionSingleValueRespectNulls(name, argument_types, parameters, settings)); +//} +// +//AggregateFunctionPtr createAggregateFunctionAnyLast(const std::string & name, const DataTypes & argument_types, const Array & parameters, const Settings * settings) +//{ +// return AggregateFunctionPtr(createAggregateFunctionSingleValue(name, argument_types, parameters, settings)); +//} +// +//AggregateFunctionPtr createAggregateFunctionAnyLastRespectNulls( +// const std::string & name, const DataTypes & argument_types, const Array & parameters, const Settings * settings) +//{ +// return AggregateFunctionPtr(createAggregateFunctionSingleValueRespectNulls(name, argument_types, parameters, settings)); +//} +// +//AggregateFunctionPtr createAggregateFunctionAnyHeavy(const std::string & name, const DataTypes & argument_types, const Array & parameters, const Settings * settings) +//{ +// return AggregateFunctionPtr(createAggregateFunctionSingleValue(name, argument_types, parameters, settings)); +//} +// +//} +// +void registerAggregateFunctionsAny(AggregateFunctionFactory &) { - extern const int INCORRECT_DATA; - extern const int LOGICAL_ERROR; -} - -namespace -{ -struct AggregateFunctionAnyRespectNullsData -{ - enum Status : UInt8 - { - NotSet = 1, - SetNull = 2, - SetOther = 3 - }; - - Status status = Status::NotSet; - Field value; - - bool isSet() const { return status != Status::NotSet; } - void setNull() { status = Status::SetNull; } - void setOther() { status = Status::SetOther; } -}; - -template -class AggregateFunctionAnyRespectNulls final - : public IAggregateFunctionDataHelper> -{ -public: - using Data = AggregateFunctionAnyRespectNullsData; - - SerializationPtr serialization; - const bool returns_nullable_type = false; - - explicit AggregateFunctionAnyRespectNulls(const DataTypePtr & type) - : IAggregateFunctionDataHelper>({type}, {}, type) - , serialization(type->getDefaultSerialization()) - , returns_nullable_type(type->isNullable()) - { - } - - String getName() const override - { - if constexpr (First) - return "any_respect_nulls"; - else - return "anyLast_respect_nulls"; - } - - bool allocatesMemoryInArena() const override { return false; } - - void addNull(AggregateDataPtr __restrict place) const - { - chassert(returns_nullable_type); - auto & d = this->data(place); - if (First && d.isSet()) - return; - d.setNull(); - } - - void add(AggregateDataPtr __restrict place, const IColumn ** columns, size_t row_num, Arena *) const override - { - if (columns[0]->isNullable()) - { - if (columns[0]->isNullAt(row_num)) - return addNull(place); - } - auto & d = this->data(place); - if (First && d.isSet()) - return; - d.setOther(); - columns[0]->get(row_num, d.value); - } - - void addManyDefaults(AggregateDataPtr __restrict place, const IColumn ** columns, size_t, Arena * arena) const override - { - if (columns[0]->isNullable()) - addNull(place); - else - add(place, columns, 0, arena); - } - - void addBatchSinglePlace( - size_t row_begin, size_t row_end, AggregateDataPtr place, const IColumn ** columns, Arena * arena, ssize_t if_argument_pos) - const override - { - if (if_argument_pos >= 0) - { - const auto & flags = assert_cast(*columns[if_argument_pos]).getData(); - size_t size = row_end - row_begin; - for (size_t i = 0; i < size; ++i) - { - size_t pos = First ? row_begin + i : row_end - 1 - i; - if (flags[pos]) - { - add(place, columns, pos, arena); - break; - } - } - } - else if (row_begin < row_end) - { - size_t pos = First ? row_begin : row_end - 1; - add(place, columns, pos, arena); - } - } - - void addBatchSinglePlaceNotNull( - size_t, size_t, AggregateDataPtr __restrict, const IColumn **, const UInt8 *, Arena *, ssize_t) const override - { - /// This should not happen since it means somebody else has preprocessed the data (NULLs or IFs) and might - /// have discarded values that we need (NULLs) - throw DB::Exception(ErrorCodes::LOGICAL_ERROR, "AggregateFunctionAnyRespectNulls::addBatchSinglePlaceNotNull called"); - } - - void merge(AggregateDataPtr __restrict place, ConstAggregateDataPtr rhs, Arena *) const override - { - auto & d = this->data(place); - if (First && d.isSet()) - return; - - auto & other = this->data(rhs); - if (other.isSet()) - { - d.status = other.status; - d.value = other.value; - } - } - - void serialize(ConstAggregateDataPtr __restrict place, WriteBuffer & buf, std::optional /* version */) const override - { - auto & d = this->data(place); - UInt8 k = d.status; - - writeBinaryLittleEndian(k, buf); - if (k == Data::Status::SetOther) - serialization->serializeBinary(d.value, buf, {}); - } - - void deserialize(AggregateDataPtr place, ReadBuffer & buf, std::optional /* version */, Arena *) const override - { - auto & d = this->data(place); - UInt8 k = Data::Status::NotSet; - readBinaryLittleEndian(k, buf); - d.status = static_cast(k); - if (d.status == Data::Status::NotSet) - return; - else if (d.status == Data::Status::SetNull) - { - if (!returns_nullable_type) - throw Exception(ErrorCodes::INCORRECT_DATA, "Incorrect type (NULL) in non-nullable {}State", getName()); - return; - } - else if (d.status == Data::Status::SetOther) - serialization->deserializeBinary(d.value, buf, {}); - else - throw Exception(ErrorCodes::INCORRECT_DATA, "Incorrect type ({}) in {}State", static_cast(k), getName()); - } - - void insertResultInto(AggregateDataPtr __restrict place, IColumn & to, Arena *) const override - { - auto & d = this->data(place); - if (d.status == Data::Status::SetOther) - to.insert(d.value); - else - to.insertDefault(); - } - - AggregateFunctionPtr getOwnNullAdapter( - const AggregateFunctionPtr & original_function, - const DataTypes & /*arguments*/, - const Array & /*params*/, - const AggregateFunctionProperties & /*properties*/) const override - { - return original_function; - } -}; - - -template -IAggregateFunction * createAggregateFunctionSingleValueRespectNulls( - const String & name, const DataTypes & argument_types, const Array & parameters, const Settings *) -{ - assertNoParameters(name, parameters); - assertUnary(name, argument_types); - - return new AggregateFunctionAnyRespectNulls(argument_types[0]); -} - -AggregateFunctionPtr createAggregateFunctionAny(const std::string & name, const DataTypes & argument_types, const Array & parameters, const Settings * settings) -{ - return AggregateFunctionPtr(createAggregateFunctionSingleValue(name, argument_types, parameters, settings)); -} - -AggregateFunctionPtr createAggregateFunctionAnyRespectNulls( - const std::string & name, const DataTypes & argument_types, const Array & parameters, const Settings * settings) -{ - return AggregateFunctionPtr(createAggregateFunctionSingleValueRespectNulls(name, argument_types, parameters, settings)); -} - -AggregateFunctionPtr createAggregateFunctionAnyLast(const std::string & name, const DataTypes & argument_types, const Array & parameters, const Settings * settings) -{ - return AggregateFunctionPtr(createAggregateFunctionSingleValue(name, argument_types, parameters, settings)); -} - -AggregateFunctionPtr createAggregateFunctionAnyLastRespectNulls( - const std::string & name, const DataTypes & argument_types, const Array & parameters, const Settings * settings) -{ - return AggregateFunctionPtr(createAggregateFunctionSingleValueRespectNulls(name, argument_types, parameters, settings)); -} - -AggregateFunctionPtr createAggregateFunctionAnyHeavy(const std::string & name, const DataTypes & argument_types, const Array & parameters, const Settings * settings) -{ - return AggregateFunctionPtr(createAggregateFunctionSingleValue(name, argument_types, parameters, settings)); -} - -} - -void registerAggregateFunctionsAny(AggregateFunctionFactory & factory) -{ - AggregateFunctionProperties default_properties = {.returns_default_when_only_null = false, .is_order_dependent = true}; - AggregateFunctionProperties default_properties_for_respect_nulls - = {.returns_default_when_only_null = false, .is_order_dependent = true, .is_window_function = true}; - - factory.registerFunction("any", {createAggregateFunctionAny, default_properties}); - factory.registerAlias("any_value", "any", AggregateFunctionFactory::CaseInsensitive); - factory.registerAlias("first_value", "any", AggregateFunctionFactory::CaseInsensitive); - - factory.registerFunction("any_respect_nulls", {createAggregateFunctionAnyRespectNulls, default_properties_for_respect_nulls}); - factory.registerAlias("any_value_respect_nulls", "any_respect_nulls", AggregateFunctionFactory::CaseInsensitive); - factory.registerAlias("first_value_respect_nulls", "any_respect_nulls", AggregateFunctionFactory::CaseInsensitive); - - factory.registerFunction("anyLast", {createAggregateFunctionAnyLast, default_properties}); - factory.registerAlias("last_value", "anyLast", AggregateFunctionFactory::CaseInsensitive); - - factory.registerFunction("anyLast_respect_nulls", {createAggregateFunctionAnyLastRespectNulls, default_properties_for_respect_nulls}); - factory.registerAlias("last_value_respect_nulls", "anyLast_respect_nulls", AggregateFunctionFactory::CaseInsensitive); - - factory.registerFunction("anyHeavy", {createAggregateFunctionAnyHeavy, default_properties}); - - factory.registerNullsActionTransformation("any", "any_respect_nulls"); - factory.registerNullsActionTransformation("anyLast", "anyLast_respect_nulls"); + // AggregateFunctionProperties default_properties = {.returns_default_when_only_null = false, .is_order_dependent = true}; + // AggregateFunctionProperties default_properties_for_respect_nulls + // = {.returns_default_when_only_null = false, .is_order_dependent = true, .is_window_function = true}; + // + // factory.registerFunction("any", {createAggregateFunctionAny, default_properties}); + // factory.registerAlias("any_value", "any", AggregateFunctionFactory::CaseInsensitive); + // factory.registerAlias("first_value", "any", AggregateFunctionFactory::CaseInsensitive); + // + // factory.registerFunction("any_respect_nulls", {createAggregateFunctionAnyRespectNulls, default_properties_for_respect_nulls}); + // factory.registerAlias("any_value_respect_nulls", "any_respect_nulls", AggregateFunctionFactory::CaseInsensitive); + // factory.registerAlias("first_value_respect_nulls", "any_respect_nulls", AggregateFunctionFactory::CaseInsensitive); + // + // factory.registerFunction("anyLast", {createAggregateFunctionAnyLast, default_properties}); + // factory.registerAlias("last_value", "anyLast", AggregateFunctionFactory::CaseInsensitive); + // + // factory.registerFunction("anyLast_respect_nulls", {createAggregateFunctionAnyLastRespectNulls, default_properties_for_respect_nulls}); + // factory.registerAlias("last_value_respect_nulls", "anyLast_respect_nulls", AggregateFunctionFactory::CaseInsensitive); + // + // factory.registerFunction("anyHeavy", {createAggregateFunctionAnyHeavy, default_properties}); + // + // factory.registerNullsActionTransformation("any", "any_respect_nulls"); + // factory.registerNullsActionTransformation("anyLast", "anyLast_respect_nulls"); } } diff --git a/src/AggregateFunctions/AggregateFunctionMax.cpp b/src/AggregateFunctions/AggregateFunctionMax.cpp index e9cd651b8db..692426ded0e 100644 --- a/src/AggregateFunctions/AggregateFunctionMax.cpp +++ b/src/AggregateFunctions/AggregateFunctionMax.cpp @@ -8,231 +8,231 @@ namespace DB { struct Settings; -namespace +//namespace +//{ +// +//template +//class AggregateFunctionsSingleValueMax final : public AggregateFunctionsSingleValue +//{ +// using Parent = AggregateFunctionsSingleValue; +// +//public: +// explicit AggregateFunctionsSingleValueMax(const DataTypePtr & type) : Parent(type) { } +// +// /// Specializations for native numeric types +// void addBatchSinglePlace( +// size_t row_begin, +// size_t row_end, +// AggregateDataPtr __restrict place, +// const IColumn ** __restrict columns, +// Arena * arena, +// ssize_t if_argument_pos) const override; +// +// void addBatchSinglePlaceNotNull( +// size_t row_begin, +// size_t row_end, +// AggregateDataPtr __restrict place, +// const IColumn ** __restrict columns, +// const UInt8 * __restrict null_map, +// Arena * arena, +// ssize_t if_argument_pos) const override; +//}; +// +//// NOLINTBEGIN(bugprone-macro-parentheses) +//#define SPECIALIZE(TYPE) \ +//template <> \ +//void AggregateFunctionsSingleValueMax>>::addBatchSinglePlace( \ +// size_t row_begin, \ +// size_t row_end, \ +// AggregateDataPtr __restrict place, \ +// const IColumn ** __restrict columns, \ +// Arena *, \ +// ssize_t if_argument_pos) const \ +//{ \ +// const auto & column = assert_cast>::ColVecType &>(*columns[0]); \ +// std::optional opt; \ +// if (if_argument_pos >= 0) \ +// { \ +// const auto & flags = assert_cast(*columns[if_argument_pos]).getData(); \ +// opt = findExtremeMaxIf(column.getData().data(), flags.data(), row_begin, row_end); \ +// } \ +// else \ +// opt = findExtremeMax(column.getData().data(), row_begin, row_end); \ +// if (opt.has_value()) \ +// this->data(place).changeIfGreater(opt.value()); \ +//} +//// NOLINTEND(bugprone-macro-parentheses) +// +//FOR_BASIC_NUMERIC_TYPES(SPECIALIZE) +//#undef SPECIALIZE +// +//template +//void AggregateFunctionsSingleValueMax::addBatchSinglePlace( +// size_t row_begin, +// size_t row_end, +// AggregateDataPtr __restrict place, +// const IColumn ** __restrict columns, +// Arena * arena, +// ssize_t if_argument_pos) const +//{ +// if constexpr (!is_any_of) +// { +// /// Leave other numeric types (large integers, decimals, etc) to keep doing the comparison as it's +// /// faster than doing a permutation +// return Parent::addBatchSinglePlace(row_begin, row_end, place, columns, arena, if_argument_pos); +// } +// +// constexpr int nan_direction_hint = 1; +// auto const & column = *columns[0]; +// if (if_argument_pos >= 0) +// { +// size_t index = row_begin; +// const auto & if_flags = assert_cast(*columns[if_argument_pos]).getData(); +// while (if_flags[index] == 0 && index < row_end) +// index++; +// if (index >= row_end) +// return; +// +// for (size_t i = index + 1; i < row_end; i++) +// { +// if ((if_flags[i] != 0) && (column.compareAt(i, index, column, nan_direction_hint) > 0)) +// index = i; +// } +// this->data(place).changeIfGreater(column, index, arena); +// } +// else +// { +// if (row_begin >= row_end) +// return; +// +// /// TODO: Introduce row_begin and row_end to getPermutation +// if (row_begin != 0 || row_end != column.size()) +// { +// size_t index = row_begin; +// for (size_t i = index + 1; i < row_end; i++) +// { +// if (column.compareAt(i, index, column, nan_direction_hint) > 0) +// index = i; +// } +// this->data(place).changeIfGreater(column, index, arena); +// } +// else +// { +// constexpr IColumn::PermutationSortDirection direction = IColumn::PermutationSortDirection::Descending; +// constexpr IColumn::PermutationSortStability stability = IColumn::PermutationSortStability::Unstable; +// IColumn::Permutation permutation; +// constexpr UInt64 limit = 1; +// column.getPermutation(direction, stability, limit, nan_direction_hint, permutation); +// this->data(place).changeIfGreater(column, permutation[0], arena); +// } +// } +//} +// +//// NOLINTBEGIN(bugprone-macro-parentheses) +//#define SPECIALIZE(TYPE) \ +//template <> \ +//void AggregateFunctionsSingleValueMax>>::addBatchSinglePlaceNotNull( \ +// size_t row_begin, \ +// size_t row_end, \ +// AggregateDataPtr __restrict place, \ +// const IColumn ** __restrict columns, \ +// const UInt8 * __restrict null_map, \ +// Arena *, \ +// ssize_t if_argument_pos) const \ +//{ \ +// const auto & column = assert_cast>::ColVecType &>(*columns[0]); \ +// std::optional opt; \ +// if (if_argument_pos >= 0) \ +// { \ +// const auto * if_flags = assert_cast(*columns[if_argument_pos]).getData().data(); \ +// auto final_flags = std::make_unique(row_end); \ +// for (size_t i = row_begin; i < row_end; ++i) \ +// final_flags[i] = (!null_map[i]) & !!if_flags[i]; \ +// opt = findExtremeMaxIf(column.getData().data(), final_flags.get(), row_begin, row_end); \ +// } \ +// else \ +// opt = findExtremeMaxNotNull(column.getData().data(), null_map, row_begin, row_end); \ +// if (opt.has_value()) \ +// this->data(place).changeIfGreater(opt.value()); \ +//} +//// NOLINTEND(bugprone-macro-parentheses) +// +//FOR_BASIC_NUMERIC_TYPES(SPECIALIZE) +//#undef SPECIALIZE +// +//template +//void AggregateFunctionsSingleValueMax::addBatchSinglePlaceNotNull( +// size_t row_begin, +// size_t row_end, +// AggregateDataPtr __restrict place, +// const IColumn ** __restrict columns, +// const UInt8 * __restrict null_map, +// Arena * arena, +// ssize_t if_argument_pos) const +//{ +// if constexpr (!is_any_of) +// { +// /// Leave other numeric types (large integers, decimals, etc) to keep doing the comparison as it's +// /// faster than doing a permutation +// return Parent::addBatchSinglePlaceNotNull(row_begin, row_end, place, columns, null_map, arena, if_argument_pos); +// } +// +// constexpr int nan_direction_hint = 1; +// auto const & column = *columns[0]; +// if (if_argument_pos >= 0) +// { +// size_t index = row_begin; +// const auto & if_flags = assert_cast(*columns[if_argument_pos]).getData(); +// while ((if_flags[index] == 0 || null_map[index] != 0) && (index < row_end)) +// index++; +// if (index >= row_end) +// return; +// +// for (size_t i = index + 1; i < row_end; i++) +// { +// if ((if_flags[i] != 0) && (null_map[i] == 0) && (column.compareAt(i, index, column, nan_direction_hint) > 0)) +// index = i; +// } +// this->data(place).changeIfGreater(column, index, arena); +// } +// else +// { +// size_t index = row_begin; +// while ((null_map[index] != 0) && (index < row_end)) +// index++; +// if (index >= row_end) +// return; +// +// for (size_t i = index + 1; i < row_end; i++) +// { +// if ((null_map[i] == 0) && (column.compareAt(i, index, column, nan_direction_hint) > 0)) +// index = i; +// } +// this->data(place).changeIfGreater(column, index, arena); +// } +//} +// +//AggregateFunctionPtr createAggregateFunctionMax( +// const std::string & name, const DataTypes & argument_types, const Array & parameters, const Settings * settings) +//{ +// return AggregateFunctionPtr(createAggregateFunctionSingleValue(name, argument_types, parameters, settings)); +//} +// +//AggregateFunctionPtr createAggregateFunctionArgMax( +// const std::string & name, const DataTypes & argument_types, const Array & parameters, const Settings * settings) +//{ +// return AggregateFunctionPtr(createAggregateFunctionArgMinMax(name, argument_types, parameters, settings)); +//} +// +//} +// +void registerAggregateFunctionsMax(AggregateFunctionFactory &) { - -template -class AggregateFunctionsSingleValueMax final : public AggregateFunctionsSingleValue -{ - using Parent = AggregateFunctionsSingleValue; - -public: - explicit AggregateFunctionsSingleValueMax(const DataTypePtr & type) : Parent(type) { } - - /// Specializations for native numeric types - void addBatchSinglePlace( - size_t row_begin, - size_t row_end, - AggregateDataPtr __restrict place, - const IColumn ** __restrict columns, - Arena * arena, - ssize_t if_argument_pos) const override; - - void addBatchSinglePlaceNotNull( - size_t row_begin, - size_t row_end, - AggregateDataPtr __restrict place, - const IColumn ** __restrict columns, - const UInt8 * __restrict null_map, - Arena * arena, - ssize_t if_argument_pos) const override; -}; - -// NOLINTBEGIN(bugprone-macro-parentheses) -#define SPECIALIZE(TYPE) \ -template <> \ -void AggregateFunctionsSingleValueMax>>::addBatchSinglePlace( \ - size_t row_begin, \ - size_t row_end, \ - AggregateDataPtr __restrict place, \ - const IColumn ** __restrict columns, \ - Arena *, \ - ssize_t if_argument_pos) const \ -{ \ - const auto & column = assert_cast>::ColVecType &>(*columns[0]); \ - std::optional opt; \ - if (if_argument_pos >= 0) \ - { \ - const auto & flags = assert_cast(*columns[if_argument_pos]).getData(); \ - opt = findExtremeMaxIf(column.getData().data(), flags.data(), row_begin, row_end); \ - } \ - else \ - opt = findExtremeMax(column.getData().data(), row_begin, row_end); \ - if (opt.has_value()) \ - this->data(place).changeIfGreater(opt.value()); \ -} -// NOLINTEND(bugprone-macro-parentheses) - -FOR_BASIC_NUMERIC_TYPES(SPECIALIZE) -#undef SPECIALIZE - -template -void AggregateFunctionsSingleValueMax::addBatchSinglePlace( - size_t row_begin, - size_t row_end, - AggregateDataPtr __restrict place, - const IColumn ** __restrict columns, - Arena * arena, - ssize_t if_argument_pos) const -{ - if constexpr (!is_any_of) - { - /// Leave other numeric types (large integers, decimals, etc) to keep doing the comparison as it's - /// faster than doing a permutation - return Parent::addBatchSinglePlace(row_begin, row_end, place, columns, arena, if_argument_pos); - } - - constexpr int nan_direction_hint = 1; - auto const & column = *columns[0]; - if (if_argument_pos >= 0) - { - size_t index = row_begin; - const auto & if_flags = assert_cast(*columns[if_argument_pos]).getData(); - while (if_flags[index] == 0 && index < row_end) - index++; - if (index >= row_end) - return; - - for (size_t i = index + 1; i < row_end; i++) - { - if ((if_flags[i] != 0) && (column.compareAt(i, index, column, nan_direction_hint) > 0)) - index = i; - } - this->data(place).changeIfGreater(column, index, arena); - } - else - { - if (row_begin >= row_end) - return; - - /// TODO: Introduce row_begin and row_end to getPermutation - if (row_begin != 0 || row_end != column.size()) - { - size_t index = row_begin; - for (size_t i = index + 1; i < row_end; i++) - { - if (column.compareAt(i, index, column, nan_direction_hint) > 0) - index = i; - } - this->data(place).changeIfGreater(column, index, arena); - } - else - { - constexpr IColumn::PermutationSortDirection direction = IColumn::PermutationSortDirection::Descending; - constexpr IColumn::PermutationSortStability stability = IColumn::PermutationSortStability::Unstable; - IColumn::Permutation permutation; - constexpr UInt64 limit = 1; - column.getPermutation(direction, stability, limit, nan_direction_hint, permutation); - this->data(place).changeIfGreater(column, permutation[0], arena); - } - } -} - -// NOLINTBEGIN(bugprone-macro-parentheses) -#define SPECIALIZE(TYPE) \ -template <> \ -void AggregateFunctionsSingleValueMax>>::addBatchSinglePlaceNotNull( \ - size_t row_begin, \ - size_t row_end, \ - AggregateDataPtr __restrict place, \ - const IColumn ** __restrict columns, \ - const UInt8 * __restrict null_map, \ - Arena *, \ - ssize_t if_argument_pos) const \ -{ \ - const auto & column = assert_cast>::ColVecType &>(*columns[0]); \ - std::optional opt; \ - if (if_argument_pos >= 0) \ - { \ - const auto * if_flags = assert_cast(*columns[if_argument_pos]).getData().data(); \ - auto final_flags = std::make_unique(row_end); \ - for (size_t i = row_begin; i < row_end; ++i) \ - final_flags[i] = (!null_map[i]) & !!if_flags[i]; \ - opt = findExtremeMaxIf(column.getData().data(), final_flags.get(), row_begin, row_end); \ - } \ - else \ - opt = findExtremeMaxNotNull(column.getData().data(), null_map, row_begin, row_end); \ - if (opt.has_value()) \ - this->data(place).changeIfGreater(opt.value()); \ -} -// NOLINTEND(bugprone-macro-parentheses) - -FOR_BASIC_NUMERIC_TYPES(SPECIALIZE) -#undef SPECIALIZE - -template -void AggregateFunctionsSingleValueMax::addBatchSinglePlaceNotNull( - size_t row_begin, - size_t row_end, - AggregateDataPtr __restrict place, - const IColumn ** __restrict columns, - const UInt8 * __restrict null_map, - Arena * arena, - ssize_t if_argument_pos) const -{ - if constexpr (!is_any_of) - { - /// Leave other numeric types (large integers, decimals, etc) to keep doing the comparison as it's - /// faster than doing a permutation - return Parent::addBatchSinglePlaceNotNull(row_begin, row_end, place, columns, null_map, arena, if_argument_pos); - } - - constexpr int nan_direction_hint = 1; - auto const & column = *columns[0]; - if (if_argument_pos >= 0) - { - size_t index = row_begin; - const auto & if_flags = assert_cast(*columns[if_argument_pos]).getData(); - while ((if_flags[index] == 0 || null_map[index] != 0) && (index < row_end)) - index++; - if (index >= row_end) - return; - - for (size_t i = index + 1; i < row_end; i++) - { - if ((if_flags[i] != 0) && (null_map[i] == 0) && (column.compareAt(i, index, column, nan_direction_hint) > 0)) - index = i; - } - this->data(place).changeIfGreater(column, index, arena); - } - else - { - size_t index = row_begin; - while ((null_map[index] != 0) && (index < row_end)) - index++; - if (index >= row_end) - return; - - for (size_t i = index + 1; i < row_end; i++) - { - if ((null_map[i] == 0) && (column.compareAt(i, index, column, nan_direction_hint) > 0)) - index = i; - } - this->data(place).changeIfGreater(column, index, arena); - } -} - -AggregateFunctionPtr createAggregateFunctionMax( - const std::string & name, const DataTypes & argument_types, const Array & parameters, const Settings * settings) -{ - return AggregateFunctionPtr(createAggregateFunctionSingleValue(name, argument_types, parameters, settings)); -} - -AggregateFunctionPtr createAggregateFunctionArgMax( - const std::string & name, const DataTypes & argument_types, const Array & parameters, const Settings * settings) -{ - return AggregateFunctionPtr(createAggregateFunctionArgMinMax(name, argument_types, parameters, settings)); -} - -} - -void registerAggregateFunctionsMax(AggregateFunctionFactory & factory) -{ - factory.registerFunction("max", createAggregateFunctionMax, AggregateFunctionFactory::CaseInsensitive); - - /// The functions below depend on the order of data. - AggregateFunctionProperties properties = { .returns_default_when_only_null = false, .is_order_dependent = true }; - factory.registerFunction("argMax", { createAggregateFunctionArgMax, properties }); + // factory.registerFunction("max", createAggregateFunctionMax, AggregateFunctionFactory::CaseInsensitive); + // + // /// The functions below depend on the order of data. + // AggregateFunctionProperties properties = { .returns_default_when_only_null = false, .is_order_dependent = true }; + // factory.registerFunction("argMax", { createAggregateFunctionArgMax, properties }); } } diff --git a/src/AggregateFunctions/AggregateFunctionMin.cpp b/src/AggregateFunctions/AggregateFunctionMin.cpp index d767bd5c563..a4f05378079 100644 --- a/src/AggregateFunctions/AggregateFunctionMin.cpp +++ b/src/AggregateFunctions/AggregateFunctionMin.cpp @@ -13,12 +13,34 @@ namespace { template -class AggregateFunctionsSingleValueMin final : public AggregateFunctionsSingleValue +class AggregateFunctionMin final : public IAggregateFunctionDataHelper> { - using Parent = AggregateFunctionsSingleValue; +private: + SerializationPtr serialization; public: - explicit AggregateFunctionsSingleValueMin(const DataTypePtr & type) : Parent(type) { } + explicit AggregateFunctionMin(const DataTypePtr & type) + : IAggregateFunctionDataHelper>({type}, {}, type), serialization(type->getDefaultSerialization()) + { + if (!type->isComparable()) + throw Exception( + ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT, + "Illegal type {} of argument of aggregate function {} because the values of that data type are not comparable", + type->getName(), + getName()); + } + + String getName() const override { return "min"; } + + void add(AggregateDataPtr __restrict place, const IColumn ** columns, size_t row_num, Arena * arena) const override + { + this->data(place).setIfSmaller(*columns[0], row_num, arena); + } + + void addManyDefaults(AggregateDataPtr __restrict place, const IColumn ** columns, size_t, Arena * arena) const override + { + this->data(place).setIfSmaller(*columns[0], 0, arena); + } /// Specializations for native numeric types void addBatchSinglePlace( @@ -27,7 +49,18 @@ public: AggregateDataPtr __restrict place, const IColumn ** __restrict columns, Arena * arena, - ssize_t if_argument_pos) const override; + ssize_t if_argument_pos) const override + { + if (if_argument_pos >= 0) + { + const auto & if_map = assert_cast(*columns[if_argument_pos]).getData(); + this->data(place).setSmallestNotNullIf(*columns[0], nullptr, if_map.data(), row_begin, row_end, arena); + } + else + { + this->data(place).setSmallest(*columns[0], row_begin, row_end, arena); + } + } void addBatchSinglePlaceNotNull( size_t row_begin, @@ -36,205 +69,236 @@ public: const IColumn ** __restrict columns, const UInt8 * __restrict null_map, Arena * arena, - ssize_t if_argument_pos) const override; -}; - -// NOLINTBEGIN(bugprone-macro-parentheses) -#define SPECIALIZE(TYPE) \ - template <> \ - void AggregateFunctionsSingleValueMin>>::addBatchSinglePlace( \ - size_t row_begin, \ - size_t row_end, \ - AggregateDataPtr __restrict place, \ - const IColumn ** __restrict columns, \ - Arena *, \ - ssize_t if_argument_pos) const \ - { \ - const auto & column = assert_cast>::ColVecType &>(*columns[0]); \ - std::optional opt; \ - if (if_argument_pos >= 0) \ - { \ - const auto & flags = assert_cast(*columns[if_argument_pos]).getData(); \ - opt = findExtremeMinIf(column.getData().data(), flags.data(), row_begin, row_end); \ - } \ - else \ - opt = findExtremeMin(column.getData().data(), row_begin, row_end); \ - if (opt.has_value()) \ - this->data(place).changeIfLess(opt.value()); \ - } -// NOLINTEND(bugprone-macro-parentheses) - -FOR_BASIC_NUMERIC_TYPES(SPECIALIZE) -#undef SPECIALIZE - -template -void AggregateFunctionsSingleValueMin::addBatchSinglePlace( - size_t row_begin, - size_t row_end, - AggregateDataPtr __restrict place, - const IColumn ** __restrict columns, - Arena * arena, - ssize_t if_argument_pos) const -{ - if constexpr (!is_any_of) + ssize_t if_argument_pos) const override { - /// Leave other numeric types (large integers, decimals, etc) to keep doing the comparison as it's - /// faster than doing a permutation - return Parent::addBatchSinglePlace(row_begin, row_end, place, columns, arena, if_argument_pos); - } - - constexpr int nan_direction_hint = 1; - auto const & column = *columns[0]; - if (if_argument_pos >= 0) - { - size_t index = row_begin; - const auto & if_flags = assert_cast(*columns[if_argument_pos]).getData(); - while (if_flags[index] == 0 && index < row_end) - index++; - if (index >= row_end) - return; - - for (size_t i = index + 1; i < row_end; i++) + if (if_argument_pos >= 0) { - if ((if_flags[i] != 0) && (column.compareAt(i, index, column, nan_direction_hint) < 0)) - index = i; - } - this->data(place).changeIfLess(column, index, arena); - } - else - { - if (row_begin >= row_end) - return; - - /// TODO: Introduce row_begin and row_end to getPermutation - if (row_begin != 0 || row_end != column.size()) - { - size_t index = row_begin; - for (size_t i = index + 1; i < row_end; i++) - { - if (column.compareAt(i, index, column, nan_direction_hint) < 0) - index = i; - } - this->data(place).changeIfLess(column, index, arena); + const auto & if_map = assert_cast(*columns[if_argument_pos]).getData(); + this->data(place).setSmallestNotNullIf(*columns[0], null_map, if_map.data(), row_begin, row_end, arena); } else { - constexpr IColumn::PermutationSortDirection direction = IColumn::PermutationSortDirection::Ascending; - constexpr IColumn::PermutationSortStability stability = IColumn::PermutationSortStability::Unstable; - IColumn::Permutation permutation; - constexpr UInt64 limit = 1; - column.getPermutation(direction, stability, limit, nan_direction_hint, permutation); - this->data(place).changeIfLess(column, permutation[0], arena); + this->data(place).setSmallestNotNullIf(*columns[0], null_map, nullptr, row_begin, row_end, arena); } } -} -// NOLINTBEGIN(bugprone-macro-parentheses) -#define SPECIALIZE(TYPE) \ - template <> \ - void AggregateFunctionsSingleValueMin>>::addBatchSinglePlaceNotNull( \ - size_t row_begin, \ - size_t row_end, \ - AggregateDataPtr __restrict place, \ - const IColumn ** __restrict columns, \ - const UInt8 * __restrict null_map, \ - Arena *, \ - ssize_t if_argument_pos) const \ - { \ - const auto & column = assert_cast>::ColVecType &>(*columns[0]); \ - std::optional opt; \ - if (if_argument_pos >= 0) \ - { \ - const auto * if_flags = assert_cast(*columns[if_argument_pos]).getData().data(); \ - auto final_flags = std::make_unique(row_end); \ - for (size_t i = row_begin; i < row_end; ++i) \ - final_flags[i] = (!null_map[i]) & !!if_flags[i]; \ - opt = findExtremeMinIf(column.getData().data(), final_flags.get(), row_begin, row_end); \ - } \ - else \ - opt = findExtremeMinNotNull(column.getData().data(), null_map, row_begin, row_end); \ - if (opt.has_value()) \ - this->data(place).changeIfLess(opt.value()); \ - } -// NOLINTEND(bugprone-macro-parentheses) - -FOR_BASIC_NUMERIC_TYPES(SPECIALIZE) -#undef SPECIALIZE - -template -void AggregateFunctionsSingleValueMin::addBatchSinglePlaceNotNull( - size_t row_begin, - size_t row_end, - AggregateDataPtr __restrict place, - const IColumn ** __restrict columns, - const UInt8 * __restrict null_map, - Arena * arena, - ssize_t if_argument_pos) const -{ - if constexpr (!is_any_of) + void merge(AggregateDataPtr __restrict place, ConstAggregateDataPtr rhs, Arena * arena) const override { - /// Leave other numeric types (large integers, decimals, etc) to keep doing the comparison as it's - /// faster than doing a permutation - return Parent::addBatchSinglePlaceNotNull(row_begin, row_end, place, columns, null_map, arena, if_argument_pos); + this->data(place).setIfSmaller(this->data(rhs), arena); } - constexpr int nan_direction_hint = 1; - auto const & column = *columns[0]; - if (if_argument_pos >= 0) + void serialize(ConstAggregateDataPtr __restrict place, WriteBuffer & buf, std::optional /* version */) const override { - size_t index = row_begin; - const auto & if_flags = assert_cast(*columns[if_argument_pos]).getData(); - while ((if_flags[index] == 0 || null_map[index] != 0) && (index < row_end)) - index++; - if (index >= row_end) - return; - - for (size_t i = index + 1; i < row_end; i++) - { - if ((if_flags[i] != 0) && (null_map[index] == 0) && (column.compareAt(i, index, column, nan_direction_hint) < 0)) - index = i; - } - this->data(place).changeIfLess(column, index, arena); + this->data(place).write(buf, *serialization); } - else + + void deserialize(AggregateDataPtr place, ReadBuffer & buf, std::optional /* version */, Arena * arena) const override { - size_t index = row_begin; - while ((null_map[index] != 0) && (index < row_end)) - index++; - if (index >= row_end) - return; - - for (size_t i = index + 1; i < row_end; i++) - { - if ((null_map[i] == 0) && (column.compareAt(i, index, column, nan_direction_hint) < 0)) - index = i; - } - this->data(place).changeIfLess(column, index, arena); + this->data(place).read(buf, *serialization, arena); } -} -AggregateFunctionPtr createAggregateFunctionMin( - const std::string & name, const DataTypes & argument_types, const Array & parameters, const Settings * settings) + bool allocatesMemoryInArena() const override { return Data::allocatesMemoryInArena(); } + + void insertResultInto(AggregateDataPtr __restrict place, IColumn & to, Arena *) const override + { + this->data(place).insertResultInto(to); + } +}; + +//// NOLINTBEGIN(bugprone-macro-parentheses) +//#define SPECIALIZE(TYPE) \ +// template <> \ +// void AggregateFunctionsSingleValueMin>::addBatchSinglePlace( \ +// size_t row_begin, \ +// size_t row_end, \ +// AggregateDataPtr __restrict place, \ +// const IColumn ** __restrict columns, \ +// Arena *, \ +// ssize_t if_argument_pos) const \ +// { \ +// const auto & column = assert_cast>::ColVecType &>(*columns[0]); \ +// std::optional opt; \ +// if (if_argument_pos >= 0) \ +// { \ +// const auto & flags = assert_cast(*columns[if_argument_pos]).getData(); \ +// opt = findExtremeMinIf(column.getData().data(), flags.data(), row_begin, row_end); \ +// } \ +// else \ +// opt = findExtremeMin(column.getData().data(), row_begin, row_end); \ +// if (opt.has_value()) \ +// this->data(place).setIfSmaller(opt.value()); \ +// } +//// NOLINTEND(bugprone-macro-parentheses) +// +//FOR_BASIC_NUMERIC_TYPES(SPECIALIZE) +//#undef SPECIALIZE +// +//template +//void AggregateFunctionsSingleValueMin::addBatchSinglePlace( +// size_t row_begin, +// size_t row_end, +// AggregateDataPtr __restrict place, +// const IColumn ** __restrict columns, +// Arena * arena, +// ssize_t if_argument_pos) const +//{ +//// if constexpr (!is_any_of) +//// { +//// /// Leave other numeric types (large integers, decimals, etc) to keep doing the comparison as it's +//// /// faster than doing a permutation +//// return Parent::addBatchSinglePlace(row_begin, row_end, place, columns, arena, if_argument_pos); +//// } +// +// constexpr int nan_direction_hint = 1; +// auto const & column = *columns[0]; +// if (if_argument_pos >= 0) +// { +// size_t index = row_begin; +// const auto & if_flags = assert_cast(*columns[if_argument_pos]).getData(); +// while (if_flags[index] == 0 && index < row_end) +// index++; +// if (index >= row_end) +// return; +// +// for (size_t i = index + 1; i < row_end; i++) +// { +// if ((if_flags[i] != 0) && (column.compareAt(i, index, column, nan_direction_hint) < 0)) +// index = i; +// } +// this->data(place).setIfSmaller(column, index, arena); +// } +// else +// { +// if (row_begin >= row_end) +// return; +// +// /// TODO: Introduce row_begin and row_end to getPermutation +// if (row_begin != 0 || row_end != column.size()) +// { +// size_t index = row_begin; +// for (size_t i = index + 1; i < row_end; i++) +// { +// if (column.compareAt(i, index, column, nan_direction_hint) < 0) +// index = i; +// } +// this->data(place).setIfSmaller(column, index, arena); +// } +// else +// { +// constexpr IColumn::PermutationSortDirection direction = IColumn::PermutationSortDirection::Ascending; +// constexpr IColumn::PermutationSortStability stability = IColumn::PermutationSortStability::Unstable; +// IColumn::Permutation permutation; +// constexpr UInt64 limit = 1; +// column.getPermutation(direction, stability, limit, nan_direction_hint, permutation); +// this->data(place).setIfSmaller(column, permutation[0], arena); +// } +// } +//} +// +//// NOLINTBEGIN(bugprone-macro-parentheses) +//#define SPECIALIZE(TYPE) \ +// template <> \ +// void AggregateFunctionsSingleValueMin>>::addBatchSinglePlaceNotNull( \ +// size_t row_begin, \ +// size_t row_end, \ +// AggregateDataPtr __restrict place, \ +// const IColumn ** __restrict columns, \ +// const UInt8 * __restrict null_map, \ +// Arena *, \ +// ssize_t if_argument_pos) const \ +// { \ +// const auto & column = assert_cast>::ColVecType &>(*columns[0]); \ +// std::optional opt; \ +// if (if_argument_pos >= 0) \ +// { \ +// const auto * if_flags = assert_cast(*columns[if_argument_pos]).getData().data(); \ +// auto final_flags = std::make_unique(row_end); \ +// for (size_t i = row_begin; i < row_end; ++i) \ +// final_flags[i] = (!null_map[i]) & !!if_flags[i]; \ +// opt = findExtremeMinIf(column.getData().data(), final_flags.get(), row_begin, row_end); \ +// } \ +// else \ +// opt = findExtremeMinNotNull(column.getData().data(), null_map, row_begin, row_end); \ +// if (opt.has_value()) \ +// this->data(place).setIfSmaller(opt.value()); \ +// } +//// NOLINTEND(bugprone-macro-parentheses) +// +//FOR_BASIC_NUMERIC_TYPES(SPECIALIZE) +//#undef SPECIALIZE +// +//template +//void AggregateFunctionsSingleValueMin::addBatchSinglePlaceNotNull( +// size_t row_begin, +// size_t row_end, +// AggregateDataPtr __restrict place, +// const IColumn ** __restrict columns, +// const UInt8 * __restrict null_map, +// Arena * arena, +// ssize_t if_argument_pos) const +//{ +//// if constexpr (!is_any_of) +//// { +//// /// Leave other numeric types (large integers, decimals, etc) to keep doing the comparison as it's +//// /// faster than doing a permutation +//// return Parent::addBatchSinglePlaceNotNull(row_begin, row_end, place, columns, null_map, arena, if_argument_pos); +//// } +// +// constexpr int nan_direction_hint = 1; +// auto const & column = *columns[0]; +// if (if_argument_pos >= 0) +// { +// size_t index = row_begin; +// const auto & if_flags = assert_cast(*columns[if_argument_pos]).getData(); +// while ((if_flags[index] == 0 || null_map[index] != 0) && (index < row_end)) +// index++; +// if (index >= row_end) +// return; +// +// for (size_t i = index + 1; i < row_end; i++) +// { +// if ((if_flags[i] != 0) && (null_map[index] == 0) && (column.compareAt(i, index, column, nan_direction_hint) < 0)) +// index = i; +// } +// this->data(place).setIfSmaller(column, index, arena); +// } +// else +// { +// size_t index = row_begin; +// while ((null_map[index] != 0) && (index < row_end)) +// index++; +// if (index >= row_end) +// return; +// +// for (size_t i = index + 1; i < row_end; i++) +// { +// if ((null_map[i] == 0) && (column.compareAt(i, index, column, nan_direction_hint) < 0)) +// index = i; +// } +// this->data(place).setIfSmaller(column, index, arena); +// } +//} + +AggregateFunctionPtr +createAggregateFunctionMin(const std::string & name, const DataTypes & argument_types, const Array & parameters, const Settings * settings) { - return AggregateFunctionPtr(createAggregateFunctionSingleValue( - name, argument_types, parameters, settings)); -} - -AggregateFunctionPtr createAggregateFunctionArgMin( - const std::string & name, const DataTypes & argument_types, const Array & parameters, const Settings * settings) -{ - return AggregateFunctionPtr(createAggregateFunctionArgMinMax(name, argument_types, parameters, settings)); + return AggregateFunctionPtr(createAggregateFunctionSingleValue(name, argument_types, parameters, settings)); } +//AggregateFunctionPtr createAggregateFunctionArgMin( +// const std::string & name, const DataTypes & argument_types, const Array & parameters, const Settings * settings) +//{ +// return AggregateFunctionPtr(createAggregateFunctionArgMinMax(name, argument_types, parameters, settings)); +//} } void registerAggregateFunctionsMin(AggregateFunctionFactory & factory) { factory.registerFunction("min", createAggregateFunctionMin, AggregateFunctionFactory::CaseInsensitive); - /// The functions below depend on the order of data. - AggregateFunctionProperties properties = { .returns_default_when_only_null = false, .is_order_dependent = true }; - factory.registerFunction("argMin", { createAggregateFunctionArgMin, properties }); + // /// The functions below depend on the order of data. + // AggregateFunctionProperties properties = { .returns_default_when_only_null = false, .is_order_dependent = true }; + // factory.registerFunction("argMin", { createAggregateFunctionArgMin, properties }); } } diff --git a/src/AggregateFunctions/AggregateFunctionMinMaxAny.h b/src/AggregateFunctions/AggregateFunctionMinMaxAny.h deleted file mode 100644 index dec70861543..00000000000 --- a/src/AggregateFunctions/AggregateFunctionMinMaxAny.h +++ /dev/null @@ -1,1449 +0,0 @@ -#pragma once - -#include -#include - -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#include "config.h" - -#if USE_EMBEDDED_COMPILER -# include -# include -#endif - -namespace DB -{ -struct Settings; - -namespace ErrorCodes -{ - extern const int ILLEGAL_TYPE_OF_ARGUMENT; - extern const int NOT_IMPLEMENTED; - extern const int TOO_LARGE_STRING_SIZE; - extern const int LOGICAL_ERROR; -} - -/** Aggregate functions that store one of passed values. - * For example: min, max, any, anyLast. - */ - - -/// For numeric values. -template -struct SingleValueDataFixed -{ - using Self = SingleValueDataFixed; - using ColVecType = ColumnVectorOrDecimal; - - bool has_value = false; /// We need to remember if at least one value has been passed. This is necessary for AggregateFunctionIf. - T value = T{}; - - static constexpr bool result_is_nullable = false; - static constexpr bool should_skip_null_arguments = true; - static constexpr bool is_any = false; - - bool has() const - { - return has_value; - } - - void insertResultInto(IColumn & to) const - { - if (has()) - assert_cast(to).getData().push_back(value); - else - assert_cast(to).insertDefault(); - } - - void write(WriteBuffer & buf, const ISerialization & /*serialization*/) const - { - writeBinary(has(), buf); - if (has()) - writeBinaryLittleEndian(value, buf); - } - - void read(ReadBuffer & buf, const ISerialization & /*serialization*/, Arena *) - { - readBinary(has_value, buf); - if (has()) - readBinaryLittleEndian(value, buf); - } - - - void change(const IColumn & column, size_t row_num, Arena *) - { - has_value = true; - value = assert_cast(column).getData()[row_num]; - } - - /// Assuming to.has() - void change(const Self & to, Arena *) - { - has_value = true; - value = to.value; - } - - bool changeFirstTime(const IColumn & column, size_t row_num, Arena * arena) - { - if (!has()) - { - change(column, row_num, arena); - return true; - } - else - return false; - } - - bool changeFirstTime(const Self & to, Arena * arena) - { - if (!has() && to.has()) - { - change(to, arena); - return true; - } - else - return false; - } - - bool changeEveryTime(const IColumn & column, size_t row_num, Arena * arena) - { - change(column, row_num, arena); - return true; - } - - bool changeEveryTime(const Self & to, Arena * arena) - { - if (to.has()) - { - change(to, arena); - return true; - } - else - return false; - } - - bool changeIfLess(const IColumn & column, size_t row_num, Arena * arena) - { - if (!has() || assert_cast(column).getData()[row_num] < value) - { - change(column, row_num, arena); - return true; - } - else - return false; - } - - bool changeIfLess(const Self & to, Arena * arena) - { - if (to.has() && (!has() || to.value < value)) - { - change(to, arena); - return true; - } - else - return false; - } - - void changeIfLess(T from) - { - if (!has() || from < value) - { - has_value = true; - value = from; - } - } - - bool changeIfGreater(const IColumn & column, size_t row_num, Arena * arena) - { - if (!has() || assert_cast(column).getData()[row_num] > value) - { - change(column, row_num, arena); - return true; - } - else - return false; - } - - bool changeIfGreater(const Self & to, Arena * arena) - { - if (to.has() && (!has() || to.value > value)) - { - change(to, arena); - return true; - } - else - return false; - } - - void changeIfGreater(T & from) - { - if (!has() || from > value) - { - has_value = true; - value = from; - } - } - - bool isEqualTo(const Self & to) const - { - return has() && to.value == value; - } - - bool isEqualTo(const IColumn & column, size_t row_num) const - { - return has() && assert_cast(column).getData()[row_num] == value; - } - - static bool allocatesMemoryInArena() - { - return false; - } - -#if USE_EMBEDDED_COMPILER - - static constexpr bool is_compilable = true; - - static llvm::Value * getValuePtrFromAggregateDataPtr(llvm::IRBuilderBase & builder, llvm::Value * aggregate_data_ptr) - { - llvm::IRBuilder<> & b = static_cast &>(builder); - - static constexpr size_t value_offset_from_structure = offsetof(SingleValueDataFixed, value); - auto * value_ptr = b.CreateConstInBoundsGEP1_64(b.getInt8Ty(), aggregate_data_ptr, value_offset_from_structure); - - return value_ptr; - } - - static llvm::Value * getValueFromAggregateDataPtr(llvm::IRBuilderBase & builder, llvm::Value * aggregate_data_ptr) - { - llvm::IRBuilder<> & b = static_cast &>(builder); - - auto * type = toNativeType(builder); - auto * value_ptr = getValuePtrFromAggregateDataPtr(builder, aggregate_data_ptr); - - return b.CreateLoad(type, value_ptr); - } - - static void compileChange(llvm::IRBuilderBase & builder, llvm::Value * aggregate_data_ptr, llvm::Value * value_to_check) - { - llvm::IRBuilder<> & b = static_cast &>(builder); - - auto * has_value_ptr = aggregate_data_ptr; - b.CreateStore(b.getInt1(true), has_value_ptr); - - auto * value_ptr = getValuePtrFromAggregateDataPtr(b, aggregate_data_ptr); - b.CreateStore(value_to_check, value_ptr); - } - - static void compileChangeMerge(llvm::IRBuilderBase & builder, llvm::Value * aggregate_data_dst_ptr, llvm::Value * aggregate_data_src_ptr) - { - auto * value_src = getValueFromAggregateDataPtr(builder, aggregate_data_src_ptr); - - compileChange(builder, aggregate_data_dst_ptr, value_src); - } - - static void compileChangeFirstTime(llvm::IRBuilderBase & builder, llvm::Value * aggregate_data_ptr, llvm::Value * value_to_check) - { - llvm::IRBuilder<> & b = static_cast &>(builder); - - auto * has_value_ptr = aggregate_data_ptr; - auto * has_value_value = b.CreateLoad(b.getInt1Ty(), has_value_ptr); - - auto * head = b.GetInsertBlock(); - - auto * join_block = llvm::BasicBlock::Create(head->getContext(), "join_block", head->getParent()); - auto * if_should_change = llvm::BasicBlock::Create(head->getContext(), "if_should_change", head->getParent()); - auto * if_should_not_change = llvm::BasicBlock::Create(head->getContext(), "if_should_not_change", head->getParent()); - - b.CreateCondBr(has_value_value, if_should_not_change, if_should_change); - - b.SetInsertPoint(if_should_not_change); - b.CreateBr(join_block); - - b.SetInsertPoint(if_should_change); - compileChange(builder, aggregate_data_ptr, value_to_check); - b.CreateBr(join_block); - - b.SetInsertPoint(join_block); - } - - static void compileChangeFirstTimeMerge(llvm::IRBuilderBase & builder, llvm::Value * aggregate_data_dst_ptr, llvm::Value * aggregate_data_src_ptr) - { - llvm::IRBuilder<> & b = static_cast &>(builder); - - auto * has_value_dst_ptr = aggregate_data_dst_ptr; - auto * has_value_dst = b.CreateLoad(b.getInt1Ty(), has_value_dst_ptr); - - auto * has_value_src_ptr = aggregate_data_src_ptr; - auto * has_value_src = b.CreateLoad(b.getInt1Ty(), has_value_src_ptr); - - auto * head = b.GetInsertBlock(); - - auto * join_block = llvm::BasicBlock::Create(head->getContext(), "join_block", head->getParent()); - auto * if_should_change = llvm::BasicBlock::Create(head->getContext(), "if_should_change", head->getParent()); - auto * if_should_not_change = llvm::BasicBlock::Create(head->getContext(), "if_should_not_change", head->getParent()); - - b.CreateCondBr(b.CreateAnd(b.CreateNot(has_value_dst), has_value_src), if_should_change, if_should_not_change); - - b.SetInsertPoint(if_should_change); - compileChangeMerge(builder, aggregate_data_dst_ptr, aggregate_data_src_ptr); - b.CreateBr(join_block); - - b.SetInsertPoint(if_should_not_change); - b.CreateBr(join_block); - - b.SetInsertPoint(join_block); - } - - static void compileChangeEveryTime(llvm::IRBuilderBase & builder, llvm::Value * aggregate_data_ptr, llvm::Value * value_to_check) - { - compileChange(builder, aggregate_data_ptr, value_to_check); - } - - static void compileChangeEveryTimeMerge(llvm::IRBuilderBase & builder, llvm::Value * aggregate_data_dst_ptr, llvm::Value * aggregate_data_src_ptr) - { - llvm::IRBuilder<> & b = static_cast &>(builder); - - auto * has_value_src_ptr = aggregate_data_src_ptr; - auto * has_value_src = b.CreateLoad(b.getInt1Ty(), has_value_src_ptr); - - auto * head = b.GetInsertBlock(); - - auto * join_block = llvm::BasicBlock::Create(head->getContext(), "join_block", head->getParent()); - auto * if_should_change = llvm::BasicBlock::Create(head->getContext(), "if_should_change", head->getParent()); - auto * if_should_not_change = llvm::BasicBlock::Create(head->getContext(), "if_should_not_change", head->getParent()); - - b.CreateCondBr(has_value_src, if_should_change, if_should_not_change); - - b.SetInsertPoint(if_should_change); - compileChangeMerge(builder, aggregate_data_dst_ptr, aggregate_data_src_ptr); - b.CreateBr(join_block); - - b.SetInsertPoint(if_should_not_change); - b.CreateBr(join_block); - - b.SetInsertPoint(join_block); - } - - template - static void compileChangeComparison(llvm::IRBuilderBase & builder, llvm::Value * aggregate_data_ptr, llvm::Value * value_to_check) - { - llvm::IRBuilder<> & b = static_cast &>(builder); - - auto * has_value_ptr = aggregate_data_ptr; - auto * has_value_value = b.CreateLoad(b.getInt1Ty(), has_value_ptr); - - auto * value = getValueFromAggregateDataPtr(b, aggregate_data_ptr); - - auto * head = b.GetInsertBlock(); - - auto * join_block = llvm::BasicBlock::Create(head->getContext(), "join_block", head->getParent()); - auto * if_should_change = llvm::BasicBlock::Create(head->getContext(), "if_should_change", head->getParent()); - auto * if_should_not_change = llvm::BasicBlock::Create(head->getContext(), "if_should_not_change", head->getParent()); - - auto is_signed = std::numeric_limits::is_signed; - - llvm::Value * should_change_after_comparison = nullptr; - - if constexpr (is_less) - { - if (value_to_check->getType()->isIntegerTy()) - should_change_after_comparison = is_signed ? b.CreateICmpSLT(value_to_check, value) : b.CreateICmpULT(value_to_check, value); - else - should_change_after_comparison = b.CreateFCmpOLT(value_to_check, value); - } - else - { - if (value_to_check->getType()->isIntegerTy()) - should_change_after_comparison = is_signed ? b.CreateICmpSGT(value_to_check, value) : b.CreateICmpUGT(value_to_check, value); - else - should_change_after_comparison = b.CreateFCmpOGT(value_to_check, value); - } - - b.CreateCondBr(b.CreateOr(b.CreateNot(has_value_value), should_change_after_comparison), if_should_change, if_should_not_change); - - b.SetInsertPoint(if_should_change); - compileChange(builder, aggregate_data_ptr, value_to_check); - b.CreateBr(join_block); - - b.SetInsertPoint(if_should_not_change); - b.CreateBr(join_block); - - b.SetInsertPoint(join_block); - } - - template - static void compileChangeComparisonMerge(llvm::IRBuilderBase & builder, llvm::Value * aggregate_data_dst_ptr, llvm::Value * aggregate_data_src_ptr) - { - llvm::IRBuilder<> & b = static_cast &>(builder); - - auto * has_value_dst_ptr = aggregate_data_dst_ptr; - auto * has_value_dst = b.CreateLoad(b.getInt1Ty(), has_value_dst_ptr); - - auto * value_dst = getValueFromAggregateDataPtr(b, aggregate_data_dst_ptr); - - auto * has_value_src_ptr = aggregate_data_src_ptr; - auto * has_value_src = b.CreateLoad(b.getInt1Ty(), has_value_src_ptr); - - auto * value_src = getValueFromAggregateDataPtr(b, aggregate_data_src_ptr); - - auto * head = b.GetInsertBlock(); - - auto * join_block = llvm::BasicBlock::Create(head->getContext(), "join_block", head->getParent()); - auto * if_should_change = llvm::BasicBlock::Create(head->getContext(), "if_should_change", head->getParent()); - auto * if_should_not_change = llvm::BasicBlock::Create(head->getContext(), "if_should_not_change", head->getParent()); - - auto is_signed = std::numeric_limits::is_signed; - - llvm::Value * should_change_after_comparison = nullptr; - - if constexpr (is_less) - { - if (value_src->getType()->isIntegerTy()) - should_change_after_comparison = is_signed ? b.CreateICmpSLT(value_src, value_dst) : b.CreateICmpULT(value_src, value_dst); - else - should_change_after_comparison = b.CreateFCmpOLT(value_src, value_dst); - } - else - { - if (value_src->getType()->isIntegerTy()) - should_change_after_comparison = is_signed ? b.CreateICmpSGT(value_src, value_dst) : b.CreateICmpUGT(value_src, value_dst); - else - should_change_after_comparison = b.CreateFCmpOGT(value_src, value_dst); - } - - b.CreateCondBr(b.CreateAnd(has_value_src, b.CreateOr(b.CreateNot(has_value_dst), should_change_after_comparison)), if_should_change, if_should_not_change); - - b.SetInsertPoint(if_should_change); - compileChangeMerge(builder, aggregate_data_dst_ptr, aggregate_data_src_ptr); - b.CreateBr(join_block); - - b.SetInsertPoint(if_should_not_change); - b.CreateBr(join_block); - - b.SetInsertPoint(join_block); - } - - static void compileChangeIfLess(llvm::IRBuilderBase & builder, llvm::Value * aggregate_data_ptr, llvm::Value * value_to_check) - { - static constexpr bool is_less = true; - compileChangeComparison(builder, aggregate_data_ptr, value_to_check); - } - - static void compileChangeIfLessMerge(llvm::IRBuilderBase & builder, llvm::Value * aggregate_data_dst_ptr, llvm::Value * aggregate_data_src_ptr) - { - static constexpr bool is_less = true; - compileChangeComparisonMerge(builder, aggregate_data_dst_ptr, aggregate_data_src_ptr); - } - - static void compileChangeIfGreater(llvm::IRBuilderBase & builder, llvm::Value * aggregate_data_ptr, llvm::Value * value_to_check) - { - static constexpr bool is_less = false; - compileChangeComparison(builder, aggregate_data_ptr, value_to_check); - } - - static void compileChangeIfGreaterMerge(llvm::IRBuilderBase & builder, llvm::Value * aggregate_data_dst_ptr, llvm::Value * aggregate_data_src_ptr) - { - static constexpr bool is_less = false; - compileChangeComparisonMerge(builder, aggregate_data_dst_ptr, aggregate_data_src_ptr); - } - - static llvm::Value * compileGetResult(llvm::IRBuilderBase & builder, llvm::Value * aggregate_data_ptr) - { - return getValueFromAggregateDataPtr(builder, aggregate_data_ptr); - } - -#endif -}; - -struct Compatibility -{ - /// Old versions used to store terminating null-character in SingleValueDataString. - /// Then -WithTerminatingZero methods were removed from IColumn interface, - /// because these methods are quite dangerous and easy to misuse. It introduced incompatibility. - /// See https://github.com/ClickHouse/ClickHouse/pull/41431 and https://github.com/ClickHouse/ClickHouse/issues/42916 - /// Here we keep these functions for compatibility. - /// It's safe because there's no way unsanitized user input (without \0 at the end) can reach these functions. - - static StringRef getDataAtWithTerminatingZero(const ColumnString & column, size_t n) - { - auto res = column.getDataAt(n); - /// ColumnString always reserves extra byte for null-character after string. - /// But getDataAt returns StringRef without the null-character. Let's add it. - chassert(res.data[res.size] == '\0'); - ++res.size; - return res; - } - - static void insertDataWithTerminatingZero(ColumnString & column, const char * pos, size_t length) - { - /// String already has terminating null-character. - /// But insertData will add another one unconditionally. Trim existing null-character to avoid duplication. - chassert(0 < length); - chassert(pos[length - 1] == '\0'); - column.insertData(pos, length - 1); - } -}; - -/** For strings. Short strings are stored in the object itself, and long strings are allocated separately. - * NOTE It could also be suitable for arrays of numbers. - */ -struct SingleValueDataString -{ -private: - using Self = SingleValueDataString; - - /// 0 size indicates that there is no value. Empty string must has terminating '\0' and, therefore, size of empty string is 1 - UInt32 size = 0; - UInt32 capacity = 0; /// power of two or zero - char * large_data; - -public: - static constexpr UInt32 AUTOMATIC_STORAGE_SIZE = 64; - static constexpr UInt32 MAX_SMALL_STRING_SIZE = AUTOMATIC_STORAGE_SIZE - sizeof(size) - sizeof(capacity) - sizeof(large_data); - static constexpr UInt32 MAX_STRING_SIZE = std::numeric_limits::max(); - -private: - char small_data[MAX_SMALL_STRING_SIZE]; /// Including the terminating zero. - -public: - static constexpr bool result_is_nullable = false; - static constexpr bool should_skip_null_arguments = true; - static constexpr bool is_any = false; - - bool has() const - { - return size; - } - -private: - char * getDataMutable() - { - return size <= MAX_SMALL_STRING_SIZE ? small_data : large_data; - } - - const char * getData() const - { - const char * data_ptr = size <= MAX_SMALL_STRING_SIZE ? small_data : large_data; - /// It must always be terminated with null-character - chassert(0 < size); - chassert(data_ptr[size - 1] == '\0'); - return data_ptr; - } - - StringRef getStringRef() const - { - return StringRef(getData(), size); - } - -public: - void insertResultInto(IColumn & to) const - { - if (has()) - Compatibility::insertDataWithTerminatingZero(assert_cast(to), getData(), size); - else - assert_cast(to).insertDefault(); - } - - void write(WriteBuffer & buf, const ISerialization & /*serialization*/) const - { - if (unlikely(MAX_STRING_SIZE < size)) - throw Exception(ErrorCodes::LOGICAL_ERROR, "String size is too big ({}), it's a bug", size); - - /// For serialization we use signed Int32 (for historical reasons), -1 means "no value" - Int32 size_to_write = size ? size : -1; - writeBinaryLittleEndian(size_to_write, buf); - if (has()) - buf.write(getData(), size); - } - - void allocateLargeDataIfNeeded(UInt32 size_to_reserve, Arena * arena) - { - if (capacity < size_to_reserve) - { - if (unlikely(MAX_STRING_SIZE < size_to_reserve)) - throw Exception(ErrorCodes::TOO_LARGE_STRING_SIZE, "String size is too big ({}), maximum: {}", - size_to_reserve, MAX_STRING_SIZE); - - size_t rounded_capacity = roundUpToPowerOfTwoOrZero(size_to_reserve); - chassert(rounded_capacity <= MAX_STRING_SIZE + 1); /// rounded_capacity <= 2^31 - capacity = static_cast(rounded_capacity); - - /// Don't free large_data here. - large_data = arena->alloc(capacity); - } - } - - void read(ReadBuffer & buf, const ISerialization & /*serialization*/, Arena * arena) - { - /// For serialization we use signed Int32 (for historical reasons), -1 means "no value" - Int32 rhs_size_signed; - readBinaryLittleEndian(rhs_size_signed, buf); - - if (rhs_size_signed < 0) - { - /// Don't free large_data here. - size = 0; - return; - } - - UInt32 rhs_size = rhs_size_signed; - if (rhs_size <= MAX_SMALL_STRING_SIZE) - { - /// Don't free large_data here. - size = rhs_size; - buf.readStrict(small_data, size); - } - else - { - /// Reserve one byte more for null-character - allocateLargeDataIfNeeded(rhs_size + 1, arena); - size = rhs_size; - buf.readStrict(large_data, size); - } - - /// Check if the string we read is null-terminated (getDataMutable does not have the assertion) - if (0 < size && getDataMutable()[size - 1] == '\0') - return; - - /// It's not null-terminated, but it must be (for historical reasons). There are two variants: - /// - The value was serialized by one of the incompatible versions of ClickHouse. We had some range of versions - /// that used to serialize SingleValueDataString without terminating '\0'. Let's just append it. - /// - An attacker sent crafted data. Sanitize it and append '\0'. - /// In all other cases the string must be already null-terminated. - - /// NOTE We cannot add '\0' unconditionally, because it will be duplicated. - /// NOTE It's possible that a string that actually ends with '\0' was written by one of the incompatible versions. - /// Unfortunately, we cannot distinguish it from normal string written by normal version. - /// So such strings will be trimmed. - - if (size == MAX_SMALL_STRING_SIZE) - { - /// Special case: We have to move value to large_data - allocateLargeDataIfNeeded(size + 1, arena); - memcpy(large_data, small_data, size); - } - - /// We have enough space to append - ++size; - getDataMutable()[size - 1] = '\0'; - } - - /// Assuming to.has() - void changeImpl(StringRef value, Arena * arena) - { - if (unlikely(MAX_STRING_SIZE < value.size)) - throw Exception(ErrorCodes::TOO_LARGE_STRING_SIZE, "String size is too big ({}), maximum: {}", - value.size, MAX_STRING_SIZE); - - UInt32 value_size = static_cast(value.size); - - if (value_size <= MAX_SMALL_STRING_SIZE) - { - /// Don't free large_data here. - size = value_size; - - if (size > 0) - memcpy(small_data, value.data, size); - } - else - { - allocateLargeDataIfNeeded(value_size, arena); - size = value_size; - memcpy(large_data, value.data, size); - } - } - - void change(const IColumn & column, size_t row_num, Arena * arena) - { - changeImpl(Compatibility::getDataAtWithTerminatingZero(assert_cast(column), row_num), arena); - } - - void change(const Self & to, Arena * arena) - { - changeImpl(to.getStringRef(), arena); - } - - bool changeFirstTime(const IColumn & column, size_t row_num, Arena * arena) - { - if (!has()) - { - change(column, row_num, arena); - return true; - } - else - return false; - } - - bool changeFirstTime(const Self & to, Arena * arena) - { - if (!has() && to.has()) - { - change(to, arena); - return true; - } - else - return false; - } - - bool changeEveryTime(const IColumn & column, size_t row_num, Arena * arena) - { - change(column, row_num, arena); - return true; - } - - bool changeEveryTime(const Self & to, Arena * arena) - { - if (to.has()) - { - change(to, arena); - return true; - } - else - return false; - } - - bool changeIfLess(const IColumn & column, size_t row_num, Arena * arena) - { - if (!has() || Compatibility::getDataAtWithTerminatingZero(assert_cast(column), row_num) < getStringRef()) - { - change(column, row_num, arena); - return true; - } - else - return false; - } - - bool changeIfLess(const Self & to, Arena * arena) - { - if (to.has() && (!has() || to.getStringRef() < getStringRef())) - { - change(to, arena); - return true; - } - else - return false; - } - - bool changeIfGreater(const IColumn & column, size_t row_num, Arena * arena) - { - if (!has() || Compatibility::getDataAtWithTerminatingZero(assert_cast(column), row_num) > getStringRef()) - { - change(column, row_num, arena); - return true; - } - else - return false; - } - - bool changeIfGreater(const Self & to, Arena * arena) - { - if (to.has() && (!has() || to.getStringRef() > getStringRef())) - { - change(to, arena); - return true; - } - else - return false; - } - - bool isEqualTo(const Self & to) const - { - return has() && to.getStringRef() == getStringRef(); - } - - bool isEqualTo(const IColumn & column, size_t row_num) const - { - return has() && Compatibility::getDataAtWithTerminatingZero(assert_cast(column), row_num) == getStringRef(); - } - - static bool allocatesMemoryInArena() - { - return true; - } - -#if USE_EMBEDDED_COMPILER - - static constexpr bool is_compilable = false; - -#endif - -}; - -static_assert( - sizeof(SingleValueDataString) == SingleValueDataString::AUTOMATIC_STORAGE_SIZE, - "Incorrect size of SingleValueDataString struct"); - - -/// For any other value types. -struct SingleValueDataGeneric -{ -private: - using Self = SingleValueDataGeneric; - Field value; - -public: - static constexpr bool result_is_nullable = false; - static constexpr bool should_skip_null_arguments = true; - static constexpr bool is_any = false; - - bool has() const { return !value.isNull(); } - - void insertResultInto(IColumn & to) const - { - if (has()) - to.insert(value); - else - to.insertDefault(); - } - - void write(WriteBuffer & buf, const ISerialization & serialization) const - { - if (!value.isNull()) - { - writeBinary(true, buf); - serialization.serializeBinary(value, buf, {}); - } - else - writeBinary(false, buf); - } - - void read(ReadBuffer & buf, const ISerialization & serialization, Arena *) - { - bool is_not_null; - readBinary(is_not_null, buf); - - if (is_not_null) - serialization.deserializeBinary(value, buf, {}); - } - - void change(const IColumn & column, size_t row_num, Arena *) { column.get(row_num, value); } - - void change(const Self & to, Arena *) { value = to.value; } - - bool changeFirstTime(const IColumn & column, size_t row_num, Arena * arena) - { - if (!has()) - { - change(column, row_num, arena); - return true; - } - else - return false; - } - - bool changeFirstTime(const Self & to, Arena * arena) - { - if (!has() && to.has()) - { - change(to, arena); - return true; - } - else - return false; - } - - bool changeEveryTime(const IColumn & column, size_t row_num, Arena * arena) - { - change(column, row_num, arena); - return true; - } - - bool changeEveryTime(const Self & to, Arena * arena) - { - if (to.has()) - { - change(to, arena); - return true; - } - else - return false; - } - - bool changeIfLess(const IColumn & column, size_t row_num, Arena * arena) - { - if (!has()) - { - change(column, row_num, arena); - return true; - } - else - { - Field new_value; - column.get(row_num, new_value); - if (new_value < value) - { - value = new_value; - return true; - } - else - return false; - } - } - - bool changeIfLess(const Self & to, Arena * arena) - { - if (!to.has()) - return false; - if (!has() || to.value < value) - { - change(to, arena); - return true; - } - else - return false; - } - - bool changeIfGreater(const IColumn & column, size_t row_num, Arena * arena) - { - if (!has()) - { - change(column, row_num, arena); - return true; - } - else - { - Field new_value; - column.get(row_num, new_value); - if (new_value > value) - { - value = new_value; - return true; - } - else - return false; - } - } - - bool changeIfGreater(const Self & to, Arena * arena) - { - if (!to.has()) - return false; - if (!has() || to.value > value) - { - change(to, arena); - return true; - } - else - return false; - } - - bool isEqualTo(const IColumn & column, size_t row_num) const { return has() && value == column[row_num]; } - - bool isEqualTo(const Self & to) const { return has() && to.value == value; } - - static bool allocatesMemoryInArena() - { - return false; - } - -#if USE_EMBEDDED_COMPILER - - static constexpr bool is_compilable = false; - -#endif - -}; - - -/** What is the difference between the aggregate functions min, max, any, anyLast - * (the condition that the stored value is replaced by a new one, - * as well as, of course, the name). - */ - -template -struct AggregateFunctionMinData : Data -{ - using Self = AggregateFunctionMinData; - using Impl = Data; - - bool changeIfBetter(const IColumn & column, size_t row_num, Arena * arena) { return this->changeIfLess(column, row_num, arena); } - bool changeIfBetter(const Self & to, Arena * arena) { return this->changeIfLess(to, arena); } - void addManyDefaults(const IColumn & column, size_t /*length*/, Arena * arena) { this->changeIfLess(column, 0, arena); } - - static const char * name() { return "min"; } - -#if USE_EMBEDDED_COMPILER - - static constexpr bool is_compilable = Data::is_compilable; - - static void compileChangeIfBetter(llvm::IRBuilderBase & builder, llvm::Value * aggregate_data_ptr, llvm::Value * value_to_check) - { - Data::compileChangeIfLess(builder, aggregate_data_ptr, value_to_check); - } - - static void compileChangeIfBetterMerge(llvm::IRBuilderBase & builder, llvm::Value * aggregate_data_dst_ptr, llvm::Value * aggregate_data_src_ptr) - { - Data::compileChangeIfLessMerge(builder, aggregate_data_dst_ptr, aggregate_data_src_ptr); - } - -#endif -}; - -template -struct AggregateFunctionMaxData : Data -{ - using Self = AggregateFunctionMaxData; - using Impl = Data; - - bool changeIfBetter(const IColumn & column, size_t row_num, Arena * arena) { return this->changeIfGreater(column, row_num, arena); } - bool changeIfBetter(const Self & to, Arena * arena) { return this->changeIfGreater(to, arena); } - void addManyDefaults(const IColumn & column, size_t /*length*/, Arena * arena) { this->changeIfGreater(column, 0, arena); } - - static const char * name() { return "max"; } - -#if USE_EMBEDDED_COMPILER - - static constexpr bool is_compilable = Data::is_compilable; - - static void compileChangeIfBetter(llvm::IRBuilderBase & builder, llvm::Value * aggregate_data_ptr, llvm::Value * value_to_check) - { - Data::compileChangeIfGreater(builder, aggregate_data_ptr, value_to_check); - } - - static void compileChangeIfBetterMerge(llvm::IRBuilderBase & builder, llvm::Value * aggregate_data_dst_ptr, llvm::Value * aggregate_data_src_ptr) - { - Data::compileChangeIfGreaterMerge(builder, aggregate_data_dst_ptr, aggregate_data_src_ptr); - } - -#endif -}; - -template -struct AggregateFunctionAnyData : Data -{ - using Self = AggregateFunctionAnyData; - static constexpr bool is_any = true; - - bool changeIfBetter(const IColumn & column, size_t row_num, Arena * arena) { return this->changeFirstTime(column, row_num, arena); } - bool changeIfBetter(const Self & to, Arena * arena) { return this->changeFirstTime(to, arena); } - void addManyDefaults(const IColumn & column, size_t /*length*/, Arena * arena) { this->changeFirstTime(column, 0, arena); } - - static const char * name() { return "any"; } - -#if USE_EMBEDDED_COMPILER - - static constexpr bool is_compilable = Data::is_compilable; - - static void compileChangeIfBetter(llvm::IRBuilderBase & builder, llvm::Value * aggregate_data_ptr, llvm::Value * value_to_check) - { - Data::compileChangeFirstTime(builder, aggregate_data_ptr, value_to_check); - } - - static void compileChangeIfBetterMerge(llvm::IRBuilderBase & builder, llvm::Value * aggregate_data_dst_ptr, llvm::Value * aggregate_data_src_ptr) - { - Data::compileChangeFirstTimeMerge(builder, aggregate_data_dst_ptr, aggregate_data_src_ptr); - } - -#endif -}; - -template -struct AggregateFunctionAnyLastData : Data -{ - using Self = AggregateFunctionAnyLastData; - - bool changeIfBetter(const IColumn & column, size_t row_num, Arena * arena) { return this->changeEveryTime(column, row_num, arena); } - bool changeIfBetter(const Self & to, Arena * arena) { return this->changeEveryTime(to, arena); } - void addManyDefaults(const IColumn & column, size_t /*length*/, Arena * arena) { this->changeEveryTime(column, 0, arena); } - - static const char * name() { return "anyLast"; } - -#if USE_EMBEDDED_COMPILER - - static constexpr bool is_compilable = Data::is_compilable; - - static void compileChangeIfBetter(llvm::IRBuilderBase & builder, llvm::Value * aggregate_data_ptr, llvm::Value * value_to_check) - { - Data::compileChangeEveryTime(builder, aggregate_data_ptr, value_to_check); - } - - static void compileChangeIfBetterMerge(llvm::IRBuilderBase & builder, llvm::Value * aggregate_data_dst_ptr, llvm::Value * aggregate_data_src_ptr) - { - Data::compileChangeEveryTimeMerge(builder, aggregate_data_dst_ptr, aggregate_data_src_ptr); - } - -#endif -}; - - -/** The aggregate function 'singleValueOrNull' is used to implement subquery operators, - * such as x = ALL (SELECT ...) - * It checks if there is only one unique non-NULL value in the data. - * If there is only one unique value - returns it. - * If there are zero or at least two distinct values - returns NULL. - */ -template -struct AggregateFunctionSingleValueOrNullData : Data -{ - using Self = AggregateFunctionSingleValueOrNullData; - - static constexpr bool result_is_nullable = true; - - bool first_value = true; - bool is_null = false; - - void changeIfBetter(const IColumn & column, size_t row_num, Arena * arena) - { - if (first_value) - { - first_value = false; - this->change(column, row_num, arena); - } - else if (!this->isEqualTo(column, row_num)) - { - is_null = true; - } - } - - void changeIfBetter(const Self & to, Arena * arena) - { - if (!to.has()) - return; - - if (first_value && !to.first_value) - { - first_value = false; - this->change(to, arena); - } - else if (!this->isEqualTo(to)) - { - is_null = true; - } - } - - void addManyDefaults(const IColumn & column, size_t /*length*/, Arena * arena) { this->changeIfBetter(column, 0, arena); } - - void insertResultInto(IColumn & to) const - { - if (is_null || first_value) - { - to.insertDefault(); - } - else - { - ColumnNullable & col = typeid_cast(to); - col.getNullMapColumn().insertDefault(); - this->Data::insertResultInto(col.getNestedColumn()); - } - } - - static const char * name() { return "singleValueOrNull"; } - -#if USE_EMBEDDED_COMPILER - - static constexpr bool is_compilable = false; - -#endif -}; - -/** Implement 'heavy hitters' algorithm. - * Selects most frequent value if its frequency is more than 50% in each thread of execution. - * Otherwise, selects some arbitrary value. - * http://www.cs.umd.edu/~samir/498/karp.pdf - */ -template -struct AggregateFunctionAnyHeavyData : Data -{ - UInt64 counter = 0; - - using Self = AggregateFunctionAnyHeavyData; - - bool changeIfBetter(const IColumn & column, size_t row_num, Arena * arena) - { - if (this->isEqualTo(column, row_num)) - { - ++counter; - } - else - { - if (counter == 0) - { - this->change(column, row_num, arena); - ++counter; - return true; - } - else - --counter; - } - return false; - } - - bool changeIfBetter(const Self & to, Arena * arena) - { - if (!to.has()) - return false; - - if (this->isEqualTo(to)) - { - counter += to.counter; - } - else - { - if ((!this->has() && to.has()) || counter < to.counter) - { - this->change(to, arena); - return true; - } - else - counter -= to.counter; - } - return false; - } - - void addManyDefaults(const IColumn & column, size_t length, Arena * arena) - { - for (size_t i = 0; i < length; ++i) - changeIfBetter(column, 0, arena); - } - - void write(WriteBuffer & buf, const ISerialization & serialization) const - { - Data::write(buf, serialization); - writeBinaryLittleEndian(counter, buf); - } - - void read(ReadBuffer & buf, const ISerialization & serialization, Arena * arena) - { - Data::read(buf, serialization, arena); - readBinaryLittleEndian(counter, buf); - } - - static const char * name() { return "anyHeavy"; } - -#if USE_EMBEDDED_COMPILER - - static constexpr bool is_compilable = false; - -#endif - -}; - - -template -class AggregateFunctionsSingleValue : public IAggregateFunctionDataHelper> -{ - static constexpr bool is_any = Data::is_any; - -private: - SerializationPtr serialization; - -public: - explicit AggregateFunctionsSingleValue(const DataTypePtr & type) - : IAggregateFunctionDataHelper>({type}, {}, createResultType(type)) - , serialization(type->getDefaultSerialization()) - { - if (StringRef(Data::name()) == StringRef("min") - || StringRef(Data::name()) == StringRef("max")) - { - if (!type->isComparable()) - throw Exception( - ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT, - "Illegal type {} of argument of aggregate function {} because the values of that data type are not comparable", - type->getName(), - Data::name()); - } - } - - String getName() const override { return Data::name(); } - - static DataTypePtr createResultType(const DataTypePtr & type_) - { - if constexpr (Data::result_is_nullable) - return makeNullable(type_); - return type_; - } - - void add(AggregateDataPtr __restrict place, const IColumn ** columns, size_t row_num, Arena * arena) const override - { - this->data(place).changeIfBetter(*columns[0], row_num, arena); - } - - void addManyDefaults( - AggregateDataPtr __restrict place, - const IColumn ** columns, - size_t length, - Arena * arena) const override - { - this->data(place).addManyDefaults(*columns[0], length, arena); - } - - void addBatchSinglePlace( - size_t row_begin, - size_t row_end, - AggregateDataPtr place, - const IColumn ** columns, - Arena * arena, - ssize_t if_argument_pos) const override - { - if constexpr (is_any) - if (this->data(place).has()) - return; - if (if_argument_pos >= 0) - { - const auto & flags = assert_cast(*columns[if_argument_pos]).getData(); - for (size_t i = row_begin; i < row_end; ++i) - { - if (flags[i]) - { - this->data(place).changeIfBetter(*columns[0], i, arena); - if constexpr (is_any) - break; - } - } - } - else - { - for (size_t i = row_begin; i < row_end; ++i) - { - this->data(place).changeIfBetter(*columns[0], i, arena); - if constexpr (is_any) - break; - } - } - } - - void addBatchSinglePlaceNotNull( /// NOLINT - size_t row_begin, - size_t row_end, - AggregateDataPtr place, - const IColumn ** columns, - const UInt8 * null_map, - Arena * arena, - ssize_t if_argument_pos = -1) const override - { - if constexpr (is_any) - if (this->data(place).has()) - return; - - if (if_argument_pos >= 0) - { - const auto & flags = assert_cast(*columns[if_argument_pos]).getData(); - for (size_t i = row_begin; i < row_end; ++i) - { - if (!null_map[i] && flags[i]) - { - this->data(place).changeIfBetter(*columns[0], i, arena); - if constexpr (is_any) - break; - } - } - } - else - { - for (size_t i = row_begin; i < row_end; ++i) - { - if (!null_map[i]) - { - this->data(place).changeIfBetter(*columns[0], i, arena); - if constexpr (is_any) - break; - } - } - } - } - - void merge(AggregateDataPtr __restrict place, ConstAggregateDataPtr rhs, Arena * arena) const override - { - this->data(place).changeIfBetter(this->data(rhs), arena); - } - - void serialize(ConstAggregateDataPtr __restrict place, WriteBuffer & buf, std::optional /* version */) const override - { - this->data(place).write(buf, *serialization); - } - - void deserialize(AggregateDataPtr place, ReadBuffer & buf, std::optional /* version */, Arena * arena) const override - { - this->data(place).read(buf, *serialization, arena); - } - - bool allocatesMemoryInArena() const override - { - return Data::allocatesMemoryInArena(); - } - - void insertResultInto(AggregateDataPtr __restrict place, IColumn & to, Arena *) const override - { - this->data(place).insertResultInto(to); - } - - AggregateFunctionPtr getOwnNullAdapter( - const AggregateFunctionPtr & original_function, - const DataTypes & /*arguments*/, - const Array & /*params*/, - const AggregateFunctionProperties & /*properties*/) const override - { - if (Data::result_is_nullable && !Data::should_skip_null_arguments) - return original_function; - return nullptr; - } - -#if USE_EMBEDDED_COMPILER - - bool isCompilable() const override - { - if constexpr (!Data::is_compilable) - return false; - - return canBeNativeType(*this->argument_types[0]); - } - - - void compileCreate(llvm::IRBuilderBase & builder, llvm::Value * aggregate_data_ptr) const override - { - llvm::IRBuilder<> & b = static_cast &>(builder); - - b.CreateMemSet(aggregate_data_ptr, llvm::ConstantInt::get(b.getInt8Ty(), 0), this->sizeOfData(), llvm::assumeAligned(this->alignOfData())); - } - - void compileAdd(llvm::IRBuilderBase & builder, llvm::Value * aggregate_data_ptr, const ValuesWithType & arguments) const override - { - if constexpr (Data::is_compilable) - { - Data::compileChangeIfBetter(builder, aggregate_data_ptr, arguments[0].value); - } - else - { - throw Exception(ErrorCodes::NOT_IMPLEMENTED, "{} is not JIT-compilable", getName()); - } - } - - void compileMerge(llvm::IRBuilderBase & builder, llvm::Value * aggregate_data_dst_ptr, llvm::Value * aggregate_data_src_ptr) const override - { - if constexpr (Data::is_compilable) - { - Data::compileChangeIfBetterMerge(builder, aggregate_data_dst_ptr, aggregate_data_src_ptr); - } - else - { - throw Exception(ErrorCodes::NOT_IMPLEMENTED, "{} is not JIT-compilable", getName()); - } - } - - llvm::Value * compileGetResult(llvm::IRBuilderBase & builder, llvm::Value * aggregate_data_ptr) const override - { - if constexpr (Data::is_compilable) - { - return Data::compileGetResult(builder, aggregate_data_ptr); - } - else - { - throw Exception(ErrorCodes::NOT_IMPLEMENTED, "{} is not JIT-compilable", getName()); - } - } - -#endif -}; - -} diff --git a/src/AggregateFunctions/AggregateFunctionSingleValueOrNull.cpp b/src/AggregateFunctions/AggregateFunctionSingleValueOrNull.cpp index cd897dfcf6e..71ab5c59820 100644 --- a/src/AggregateFunctions/AggregateFunctionSingleValueOrNull.cpp +++ b/src/AggregateFunctions/AggregateFunctionSingleValueOrNull.cpp @@ -8,19 +8,19 @@ namespace DB { struct Settings; -namespace +//namespace +//{ +// +//AggregateFunctionPtr createAggregateFunctionSingleValueOrNull(const std::string & name, const DataTypes & argument_types, const Array & parameters, const Settings * settings) +//{ +// return AggregateFunctionPtr(createAggregateFunctionSingleValue(name, argument_types, parameters, settings)); +//} +// +//} +// +void registerAggregateFunctionSingleValueOrNull(AggregateFunctionFactory &) { - -AggregateFunctionPtr createAggregateFunctionSingleValueOrNull(const std::string & name, const DataTypes & argument_types, const Array & parameters, const Settings * settings) -{ - return AggregateFunctionPtr(createAggregateFunctionSingleValue(name, argument_types, parameters, settings)); -} - -} - -void registerAggregateFunctionSingleValueOrNull(AggregateFunctionFactory & factory) -{ - factory.registerFunction("singleValueOrNull", createAggregateFunctionSingleValueOrNull); + // factory.registerFunction("singleValueOrNull", createAggregateFunctionSingleValueOrNull); } diff --git a/src/AggregateFunctions/Combinators/AggregateFunctionArgMinMax.cpp b/src/AggregateFunctions/Combinators/AggregateFunctionArgMinMax.cpp index 38e19a0b8da..946b4a07a9a 100644 --- a/src/AggregateFunctions/Combinators/AggregateFunctionArgMinMax.cpp +++ b/src/AggregateFunctions/Combinators/AggregateFunctionArgMinMax.cpp @@ -1,7 +1,7 @@ #include "AggregateFunctionArgMinMax.h" #include "AggregateFunctionCombinatorFactory.h" -#include +#include #include #include #include @@ -13,81 +13,81 @@ namespace ErrorCodes { extern const int NUMBER_OF_ARGUMENTS_DOESNT_MATCH; } - -namespace +// +//namespace +//{ +//template