diff --git a/dbms/src/Functions/FunctionsConversion.h b/dbms/src/Functions/FunctionsConversion.h index c16f439c3fa..7f89ea2430f 100644 --- a/dbms/src/Functions/FunctionsConversion.h +++ b/dbms/src/Functions/FunctionsConversion.h @@ -1540,6 +1540,9 @@ public: String getName() const override { return name; } + bool isDeterministic() const override { return true; } + bool isDeterministicInScopeOfQuery() const override { return true; } + bool hasInformationAboutMonotonicity() const override { return static_cast(monotonicity_for_range); diff --git a/dbms/src/Functions/FunctionsMiscellaneous.h b/dbms/src/Functions/FunctionsMiscellaneous.h index 96539f9559f..de1177dd8ed 100644 --- a/dbms/src/Functions/FunctionsMiscellaneous.h +++ b/dbms/src/Functions/FunctionsMiscellaneous.h @@ -26,6 +26,9 @@ public: String getName() const override { return "FunctionExpression"; } + bool isDeterministic() const override { return true; } + bool isDeterministicInScopeOfQuery() const override { return true; } + const DataTypes & getArgumentTypes() const override { return argument_types; } const DataTypePtr & getReturnType() const override { return return_type; } @@ -110,6 +113,9 @@ public: String getName() const override { return name; } + bool isDeterministic() const override { return true; } + bool isDeterministicInScopeOfQuery() const override { return true; } + const DataTypes & getArgumentTypes() const override { return captured_types; } const DataTypePtr & getReturnType() const override { return return_type; } diff --git a/dbms/src/Functions/FunctionsRandom.h b/dbms/src/Functions/FunctionsRandom.h index 069c0afa86b..9559f6121fd 100644 --- a/dbms/src/Functions/FunctionsRandom.h +++ b/dbms/src/Functions/FunctionsRandom.h @@ -53,9 +53,11 @@ public: return name; } + bool isDeterministic() const override { return false; } + bool isDeterministicInScopeOfQuery() const override { return false; } + bool isVariadic() const override { return true; } size_t getNumberOfArguments() const override { return 0; } - bool isDeterministicInScopeOfQuery() const override { return false; } DataTypePtr getReturnTypeImpl(const DataTypes & arguments) const override { diff --git a/dbms/src/Functions/IFunction.h b/dbms/src/Functions/IFunction.h index a3b4c790926..04e9783a519 100644 --- a/dbms/src/Functions/IFunction.h +++ b/dbms/src/Functions/IFunction.h @@ -198,9 +198,9 @@ public: * Example: now(). Another example: functions that work with periodically updated dictionaries. */ - virtual bool isDeterministic() const { return true; } + virtual bool isDeterministic() const = 0; - virtual bool isDeterministicInScopeOfQuery() const { return true; } + virtual bool isDeterministicInScopeOfQuery() const = 0; /** Lets you know if the function is monotonic in a range of values. * This is used to work with the index in a sorted chunk of data. @@ -240,11 +240,16 @@ public: /// Get the main function name. virtual String getName() const = 0; + /// See the comment for the same method in IFunctionBase + virtual bool isDeterministic() const = 0; + + virtual bool isDeterministicInScopeOfQuery() const = 0; + /// Override and return true if function needs to depend on the state of the data. - virtual bool isStateful() const { return false; } + virtual bool isStateful() const = 0; /// Override and return true if function could take different number of arguments. - virtual bool isVariadic() const { return false; } + virtual bool isVariadic() const = 0; /// For non-variadic functions, return number of arguments; otherwise return zero (that should be ignored). virtual size_t getNumberOfArguments() const = 0; @@ -277,6 +282,11 @@ public: return buildImpl(arguments, getReturnType(arguments)); } + bool isDeterministic() const override { return true; } + bool isDeterministicInScopeOfQuery() const override { return true; } + bool isStateful() const override { return false; } + bool isVariadic() const override { return false; } + /// Default implementation. Will check only in non-variadic case. void checkNumberOfArguments(size_t number_of_arguments) const override; @@ -357,6 +367,8 @@ public: ColumnNumbers getArgumentsThatAreAlwaysConstant() const override { return {}; } bool canBeExecutedOnDefaultArguments() const override { return true; } bool canBeExecutedOnLowCardinalityDictionary() const override { return isDeterministicInScopeOfQuery(); } + bool isDeterministic() const override { return true; } + bool isDeterministicInScopeOfQuery() const override { return true; } using PreparedFunctionImpl::execute; using PreparedFunctionImpl::executeImplDryRun; @@ -506,6 +518,9 @@ public: return function->checkNumberOfArguments(number_of_arguments); } + bool isDeterministic() const override { return function->isDeterministic(); } + bool isDeterministicInScopeOfQuery() const override { return function->isDeterministicInScopeOfQuery(); } + String getName() const override { return function->getName(); } bool isStateful() const override { return function->isStateful(); } bool isVariadic() const override { return function->isVariadic(); } diff --git a/dbms/src/Functions/now.cpp b/dbms/src/Functions/now.cpp index b5b4d9d7918..35ecf75e420 100644 --- a/dbms/src/Functions/now.cpp +++ b/dbms/src/Functions/now.cpp @@ -53,6 +53,7 @@ public: } bool isDeterministic() const override { return false; } + bool isDeterministicInScopeOfQuery() const override { return true; } private: time_t time_value; @@ -65,8 +66,10 @@ public: static constexpr auto name = "now"; String getName() const override { return name; } - size_t getNumberOfArguments() const override { return 0; } + bool isDeterministic() const override { return false; } + + size_t getNumberOfArguments() const override { return 0; } static FunctionBuilderPtr create(const Context &) { return std::make_shared(); } protected: diff --git a/dbms/src/Functions/randConstant.cpp b/dbms/src/Functions/randConstant.cpp index e854484154c..3bbb3f56b0e 100644 --- a/dbms/src/Functions/randConstant.cpp +++ b/dbms/src/Functions/randConstant.cpp @@ -50,6 +50,7 @@ public: } bool isDeterministic() const override { return false; } + bool isDeterministicInScopeOfQuery() const override { return true; } private: ToType value; @@ -64,6 +65,8 @@ public: static constexpr auto name = Name::name; String getName() const override { return name; } + bool isDeterministic() const override { return false; } + bool isVariadic() const override { return true; } size_t getNumberOfArguments() const override { return 0; } diff --git a/dbms/src/Functions/runningDifference.h b/dbms/src/Functions/runningDifference.h index 374142311e9..7cda3b0be94 100644 --- a/dbms/src/Functions/runningDifference.h +++ b/dbms/src/Functions/runningDifference.h @@ -141,6 +141,7 @@ public: return 1; } + bool isDeterministic() const override { return false; } bool isDeterministicInScopeOfQuery() const override { return false; diff --git a/dbms/src/Functions/toTypeName.cpp b/dbms/src/Functions/toTypeName.cpp index 55d602167a5..202274787bc 100644 --- a/dbms/src/Functions/toTypeName.cpp +++ b/dbms/src/Functions/toTypeName.cpp @@ -38,6 +38,9 @@ public: static constexpr auto name = "toTypeName"; String getName() const override { return name; } + bool isDeterministic() const override { return true; } + bool isDeterministicInScopeOfQuery() const override { return true; } + const DataTypes & getArgumentTypes() const override { return argument_types; } const DataTypePtr & getReturnType() const override { return return_type; } diff --git a/dbms/src/Functions/today.cpp b/dbms/src/Functions/today.cpp index 72e4867a950..8f692333170 100644 --- a/dbms/src/Functions/today.cpp +++ b/dbms/src/Functions/today.cpp @@ -52,6 +52,7 @@ public: } bool isDeterministic() const override { return false; } + bool isDeterministicInScopeOfQuery() const override { return true; } private: DayNum day_value; @@ -64,6 +65,9 @@ public: static constexpr auto name = "today"; String getName() const override { return name; } + + bool isDeterministic() const override { return false; } + size_t getNumberOfArguments() const override { return 0; } static FunctionBuilderPtr create(const Context &) { return std::make_shared(); } diff --git a/dbms/src/Functions/yesterday.cpp b/dbms/src/Functions/yesterday.cpp index 565a2c40913..88aa6896f5a 100644 --- a/dbms/src/Functions/yesterday.cpp +++ b/dbms/src/Functions/yesterday.cpp @@ -52,6 +52,7 @@ public: } bool isDeterministic() const override { return false; } + bool isDeterministicInScopeOfQuery() const override { return true; } private: DayNum day_value; @@ -64,6 +65,9 @@ public: static constexpr auto name = "yesterday"; String getName() const override { return name; } + + bool isDeterministic() const override { return false; } + size_t getNumberOfArguments() const override { return 0; } static FunctionBuilderPtr create(const Context &) { return std::make_shared(); } diff --git a/dbms/src/Interpreters/MutationsInterpreter.cpp b/dbms/src/Interpreters/MutationsInterpreter.cpp index a025ba87c1a..2641ab2a5c4 100644 --- a/dbms/src/Interpreters/MutationsInterpreter.cpp +++ b/dbms/src/Interpreters/MutationsInterpreter.cpp @@ -1,6 +1,11 @@ +#include "MutationsInterpreter.h" + +#include +#include +#include +#include #include #include -#include #include #include #include @@ -14,7 +19,6 @@ #include #include #include -#include "MutationsInterpreter.h" namespace DB @@ -27,6 +31,67 @@ namespace ErrorCodes extern const int CANNOT_UPDATE_COLUMN; } +namespace +{ +struct FirstNonDeterministicFuncData +{ + using TypeToVisit = ASTFunction; + + explicit FirstNonDeterministicFuncData(const Context & context_) + : context{context_} + {} + + const Context & context; + std::optional nondeterministic_function_name; + + void visit(ASTFunction & function, ASTPtr &) + { + if (nondeterministic_function_name) + return; + + const auto func = FunctionFactory::instance().get(function.name, context); + if (!func->isDeterministic()) + nondeterministic_function_name = func->getName(); + } +}; + +using FirstNonDeterministicFuncFinder = + InDepthNodeVisitor, true>; + +std::optional findFirstNonDeterministicFuncName(const MutationCommand & command, const Context & context) +{ + FirstNonDeterministicFuncData finder_data(context); + + switch (command.type) + { + case MutationCommand::UPDATE: + { + auto update_assignments_ast = command.ast->as().update_assignments->clone(); + FirstNonDeterministicFuncFinder(finder_data).visit(update_assignments_ast); + + if (finder_data.nondeterministic_function_name) + return finder_data.nondeterministic_function_name; + + [[fallthrough]]; + } + + case MutationCommand::DELETE: + { + auto predicate_ast = command.predicate->clone(); + FirstNonDeterministicFuncFinder(finder_data).visit(predicate_ast); + + return finder_data.nondeterministic_function_name; + } + + default: + break; + } + + return {}; +} +}; + + bool MutationsInterpreter::isStorageTouchedByMutations() const { if (commands.empty()) @@ -440,6 +505,21 @@ BlockInputStreamPtr MutationsInterpreter::addStreamsForLaterStages(const std::ve void MutationsInterpreter::validate(TableStructureReadLockHolder &) { + /// For Replicated* storages mutations cannot employ non-deterministic functions + /// because that produces inconsistencies between replicas + if (startsWith(storage->getName(), "Replicated")) + { + for (const auto & command : commands) + { + const auto nondeterministic_func_name = findFirstNonDeterministicFuncName(command, context); + if (nondeterministic_func_name) + throw Exception( + "ALTER UPDATE/ALTER DELETE statements must use only deterministic functions! " + "Function '" + *nondeterministic_func_name + "' is non-deterministic", + ErrorCodes::BAD_ARGUMENTS); + } + } + const auto & select_query = prepare(/* dry_run = */ true); InterpreterSelectQuery interpreter{select_query, context, storage, SelectQueryOptions().analyze(/* dry_run = */ true).ignoreLimits()}; /// Do not use getSampleBlock in order to check the whole pipeline. diff --git a/dbms/tests/queries/0_stateless/01017_mutations_with_nondeterministic_functions_zookeeper.reference b/dbms/tests/queries/0_stateless/01017_mutations_with_nondeterministic_functions_zookeeper.reference new file mode 100644 index 00000000000..b462a5a7baa --- /dev/null +++ b/dbms/tests/queries/0_stateless/01017_mutations_with_nondeterministic_functions_zookeeper.reference @@ -0,0 +1,4 @@ +OK +OK +OK +OK diff --git a/dbms/tests/queries/0_stateless/01017_mutations_with_nondeterministic_functions_zookeeper.sh b/dbms/tests/queries/0_stateless/01017_mutations_with_nondeterministic_functions_zookeeper.sh new file mode 100755 index 00000000000..ac66dbc352a --- /dev/null +++ b/dbms/tests/queries/0_stateless/01017_mutations_with_nondeterministic_functions_zookeeper.sh @@ -0,0 +1,43 @@ +#!/usr/bin/env bash + +CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +. $CURDIR/../shell_config.sh + + +R1=table_1017_1 +R2=table_1017_2 +T1=table_1017_merge + +${CLICKHOUSE_CLIENT} -n -q " + DROP TABLE IF EXISTS $R1; + DROP TABLE IF EXISTS $R2; + + CREATE TABLE $R1 (x UInt32, y UInt32) ENGINE ReplicatedMergeTree('/clickhouse/tables/${CLICKHOUSE_DATABASE}.table_1017', 'r1') ORDER BY x; + CREATE TABLE $R2 (x UInt32, y UInt32) ENGINE ReplicatedMergeTree('/clickhouse/tables/${CLICKHOUSE_DATABASE}.table_1017', 'r2') ORDER BY x; + CREATE TABLE $T1 (x UInt32, y UInt32) ENGINE MergeTree() ORDER BY x; + + INSERT INTO $R1 VALUES (0, 1)(1, 2)(2, 3)(3, 4); + INSERT INTO $T1 VALUES (0, 1)(1, 2)(2, 3)(3, 4); +" + +# Check that in mutations of replicated tables predicates do not contain non-deterministic functions +${CLICKHOUSE_CLIENT} --query "ALTER TABLE $R1 DELETE WHERE ignore(rand())" 2>&1 \ +| fgrep -q "must use only deterministic functions" && echo 'OK' || echo 'FAIL' + +${CLICKHOUSE_CLIENT} --query "ALTER TABLE $R1 UPDATE y = y + rand() % 1 WHERE not ignore()" 2>&1 \ +| fgrep -q "must use only deterministic functions" && echo 'OK' || echo 'FAIL' + + +# For regular tables we do not enforce deterministic functions +${CLICKHOUSE_CLIENT} --query "ALTER TABLE $T1 DELETE WHERE rand() = 0" 2>&1 > /dev/null \ +&& echo 'OK' || echo 'FAIL' + +${CLICKHOUSE_CLIENT} --query "ALTER TABLE $T1 UPDATE y = y + rand() % 1 WHERE not ignore()" 2>&1 > /dev/null \ +&& echo 'OK' || echo 'FAIL' + + +${CLICKHOUSE_CLIENT} -n -q " + DROP TABLE IF EXISTS $R2; + DROP TABLE IF EXISTS $R1; + DROP TABLE IF EXISTS $T1; +"