diff --git a/src/Core/Settings.h b/src/Core/Settings.h index 122a37eb23b..6096ce00a0b 100644 --- a/src/Core/Settings.h +++ b/src/Core/Settings.h @@ -492,8 +492,7 @@ class IColumn; M(UInt64, offset, 0, "Offset on read rows from the most 'end' result for select query", 0) \ \ M(UInt64, function_range_max_elements_in_block, 500000000, "Maximum number of values generated by function 'range' per block of data (sum of array sizes for every row in a block, see also 'max_block_size' and 'min_insert_block_size_rows'). It is a safety threshold.", 0) \ - M(Bool, use_short_circuit_function_evaluation, true, "Enable short-circuit function evaluation", 0) \ - M(Bool, optimize_short_circuit_function_evaluation, true, "Enable lazy execution only for heavy functions or for functions that can throw", 0) \ + M(ShortCircuitFunctionEvaluation, short_circuit_function_evaluation, ShortCircuitFunctionEvaluation::ENABLE, "Setting for short-circuit function evaluation configuration. Possible values: 'enable', 'disable', 'force_enable'", 0) \ \ /** Experimental functions */ \ M(Bool, allow_experimental_funnel_functions, false, "Enable experimental functions for funnel analysis.", 0) \ diff --git a/src/Core/SettingsEnums.cpp b/src/Core/SettingsEnums.cpp index 26c2bd9b6af..213d365ad96 100644 --- a/src/Core/SettingsEnums.cpp +++ b/src/Core/SettingsEnums.cpp @@ -111,4 +111,9 @@ IMPLEMENT_SETTING_ENUM(DistributedDDLOutputMode, ErrorCodes::BAD_ARGUMENTS, IMPLEMENT_SETTING_ENUM(HandleKafkaErrorMode, ErrorCodes::BAD_ARGUMENTS, {{"default", HandleKafkaErrorMode::DEFAULT}, {"stream", HandleKafkaErrorMode::STREAM}}) + +IMPLEMENT_SETTING_ENUM(ShortCircuitFunctionEvaluation, ErrorCodes::BAD_ARGUMENTS, + {{"enable", ShortCircuitFunctionEvaluation::ENABLE}, + {"force_enable", ShortCircuitFunctionEvaluation::FORCE_ENABLE}, + {"disable", ShortCircuitFunctionEvaluation::DISABLE}}) } diff --git a/src/Core/SettingsEnums.h b/src/Core/SettingsEnums.h index f0dd10aacfb..d1dc71f621f 100644 --- a/src/Core/SettingsEnums.h +++ b/src/Core/SettingsEnums.h @@ -157,4 +157,14 @@ enum class HandleKafkaErrorMode }; DECLARE_SETTING_ENUM(HandleKafkaErrorMode) + +enum class ShortCircuitFunctionEvaluation +{ + ENABLE, // Use short-circuit function evaluation for functions that are suitable for it. + FORCE_ENABLE, // Use short-circuit function evaluation for all functions. + DISABLE, // Disable short-circuit function evaluation. +}; + +DECLARE_SETTING_ENUM(ShortCircuitFunctionEvaluation) + } diff --git a/src/Functions/FunctionsConversion.h b/src/Functions/FunctionsConversion.h index c0c07c766a8..eba8ff06a4f 100644 --- a/src/Functions/FunctionsConversion.h +++ b/src/Functions/FunctionsConversion.h @@ -1454,6 +1454,10 @@ public: static constexpr bool to_string_or_fixed_string = std::is_same_v || std::is_same_v; + static constexpr bool to_date_or_datetime = std::is_same_v || + std::is_same_v || + std::is_same_v; + static FunctionPtr create(ContextPtr) { return std::make_shared(); } static FunctionPtr create() { return std::make_shared(); } @@ -1465,7 +1469,11 @@ public: bool isVariadic() const override { return true; } size_t getNumberOfArguments() const override { return 0; } bool isInjective(const ColumnsWithTypeAndName &) const override { return std::is_same_v; } - bool isSuitableForShortCircuitArgumentsExecution(const DataTypesWithConstInfo & /*arguments*/) const override { return true; } + bool isSuitableForShortCircuitArgumentsExecution(const DataTypesWithConstInfo & arguments) const override + { + /// TODO: We can make more optimizations here. + return !(to_date_or_datetime && isNumber(*arguments[0].type)); + } using DefaultReturnTypeGetter = std::function; static DataTypePtr getReturnTypeDefaultImplementationForNulls(const ColumnsWithTypeAndName & arguments, const DefaultReturnTypeGetter & getter) diff --git a/src/Interpreters/ExpressionActions.cpp b/src/Interpreters/ExpressionActions.cpp index 6b66443ba84..5b25ad9609e 100644 --- a/src/Interpreters/ExpressionActions.cpp +++ b/src/Interpreters/ExpressionActions.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #if defined(MEMORY_SANITIZER) @@ -47,7 +48,7 @@ namespace ErrorCodes ExpressionActions::~ExpressionActions() = default; -static std::unordered_set processShortCircuitFunctions(const ActionsDAG & actions_dag, bool optimize_short_circuit_function_evaluation); +static std::unordered_set processShortCircuitFunctions(const ActionsDAG & actions_dag, ShortCircuitFunctionEvaluation short_circuit_function_evaluation); ExpressionActions::ExpressionActions(ActionsDAGPtr actions_dag_, const ExpressionActionsSettings & settings_) : settings(settings_) @@ -55,9 +56,7 @@ ExpressionActions::ExpressionActions(ActionsDAGPtr actions_dag_, const Expressio actions_dag = actions_dag_->clone(); /// It's important to determine lazy executed nodes before compiling expressions. - std::unordered_set lazy_executed_nodes; - if (settings.use_short_circuit_function_evaluation) - lazy_executed_nodes = processShortCircuitFunctions(*actions_dag, settings.optimize_short_circuit_function_evaluation); + std::unordered_set lazy_executed_nodes = processShortCircuitFunctions(*actions_dag, settings.short_circuit_function_evaluation); #if USE_EMBEDDED_COMPILER if (settings.can_compile_expressions && settings.compile_expressions == CompileExpressions::yes) @@ -224,7 +223,7 @@ static void setLazyExecutionInfo( /// Enable lazy execution for short-circuit function arguments. static bool findLazyExecutedNodes( const ActionsDAG::NodeRawConstPtrs & children, - std::unordered_map lazy_execution_infos, + std::unordered_map & lazy_execution_infos, bool force_enable_lazy_execution, std::unordered_set & lazy_executed_nodes_out) { @@ -278,8 +277,11 @@ static bool findLazyExecutedNodes( return has_lazy_node; } -static std::unordered_set processShortCircuitFunctions(const ActionsDAG & actions_dag, bool optimize_short_circuit_function_evaluation) +static std::unordered_set processShortCircuitFunctions(const ActionsDAG & actions_dag, ShortCircuitFunctionEvaluation short_circuit_function_evaluation) { + if (short_circuit_function_evaluation == ShortCircuitFunctionEvaluation::DISABLE) + return {}; + const auto & nodes = actions_dag.getNodes(); /// Firstly, find all short-circuit functions and get their settings. @@ -305,7 +307,7 @@ static std::unordered_set processShortCircuitFunctions findLazyExecutedNodes( node->children, lazy_execution_infos, - settings.force_enable_lazy_execution || !optimize_short_circuit_function_evaluation, + settings.force_enable_lazy_execution || short_circuit_function_evaluation == ShortCircuitFunctionEvaluation::FORCE_ENABLE, lazy_executed_nodes); } return lazy_executed_nodes; diff --git a/src/Interpreters/ExpressionActionsSettings.cpp b/src/Interpreters/ExpressionActionsSettings.cpp index b881edfc630..8aec7afef4d 100644 --- a/src/Interpreters/ExpressionActionsSettings.cpp +++ b/src/Interpreters/ExpressionActionsSettings.cpp @@ -14,8 +14,7 @@ ExpressionActionsSettings ExpressionActionsSettings::fromSettings(const Settings settings.max_temporary_columns = from.max_temporary_columns; settings.max_temporary_non_const_columns = from.max_temporary_non_const_columns; settings.compile_expressions = compile_expressions; - settings.use_short_circuit_function_evaluation = from.use_short_circuit_function_evaluation; - settings.optimize_short_circuit_function_evaluation = from.optimize_short_circuit_function_evaluation; + settings.short_circuit_function_evaluation = from.short_circuit_function_evaluation; return settings; } diff --git a/src/Interpreters/ExpressionActionsSettings.h b/src/Interpreters/ExpressionActionsSettings.h index b5fca4991ae..326de983748 100644 --- a/src/Interpreters/ExpressionActionsSettings.h +++ b/src/Interpreters/ExpressionActionsSettings.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include @@ -25,8 +26,7 @@ struct ExpressionActionsSettings CompileExpressions compile_expressions = CompileExpressions::no; - bool use_short_circuit_function_evaluation = false; - bool optimize_short_circuit_function_evaluation = false; + ShortCircuitFunctionEvaluation short_circuit_function_evaluation = ShortCircuitFunctionEvaluation::DISABLE; static ExpressionActionsSettings fromSettings(const Settings & from, CompileExpressions compile_expressions = CompileExpressions::no); static ExpressionActionsSettings fromContext(ContextPtr from, CompileExpressions compile_expressions = CompileExpressions::no); diff --git a/tests/queries/0_stateless/01822_short_circuit.sql b/tests/queries/0_stateless/01822_short_circuit.sql index 349a62e2e91..16908642c52 100644 --- a/tests/queries/0_stateless/01822_short_circuit.sql +++ b/tests/queries/0_stateless/01822_short_circuit.sql @@ -1,4 +1,4 @@ -set use_short_circuit_function_evaluation = 1; +set short_circuit_function_evaluation = 'enable'; select if(number > 0, intDiv(number + 100, number), throwIf(number)) from numbers(10); select multiIf(number == 0, 0, number == 1, intDiv(1, number), number == 2, intDiv(1, number - 1), number == 3, intDiv(1, number - 2), intDiv(1, number - 3)) from numbers(10);