Fix performance, make only one setting

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(ShortCircuitFunctionEvaluation, short_circuit_function_evaluation, ShortCircuitFunctionEvaluation::ENABLE, "Setting for short-circuit function evaluation configuration. Possible values: 'enable', 'disable', 'force_enable'", 0) \
-    M(Bool, optimize_short_circuit_function_evaluation, true, "Enable lazy execution only for heavy functions or for functions that can throw", 0) \
     \
     /** Experimental functions */ \
     M(Bool, allow_experimental_funnel_functions, false, "Enable experimental functions for funnel analysis.", 0) \

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}})
 }

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)
+
 }

src/Functions/FunctionsConversion.h

@@ -1454,6 +1454,10 @@ public:
     static constexpr bool to_string_or_fixed_string = std::is_same_v<ToDataType, DataTypeFixedString> ||
                                                       std::is_same_v<ToDataType, DataTypeString>;
 
+    static constexpr bool to_date_or_datetime = std::is_same_v<ToDataType, DataTypeDate> ||
+                                                std::is_same_v<ToDataType, DataTypeDate32> ||
+                                                std::is_same_v<ToDataType, DataTypeDateTime>;
+
     static FunctionPtr create(ContextPtr) { return std::make_shared<FunctionConvert>(); }
     static FunctionPtr create() { return std::make_shared<FunctionConvert>(); }
 
@@ -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<Name, NameToString>; }
-    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<DataTypePtr(const ColumnsWithTypeAndName &)>;
     static DataTypePtr getReturnTypeDefaultImplementationForNulls(const ColumnsWithTypeAndName & arguments, const DefaultReturnTypeGetter & getter)

src/Interpreters/ExpressionActions.cpp

@@ -17,6 +17,7 @@
 #include <queue>
 #include <stack>
 #include <Common/JSONBuilder.h>
+#include <Core/SettingsEnums.h>
 
 
 #if defined(MEMORY_SANITIZER)
@@ -47,7 +48,7 @@ namespace ErrorCodes
 
 ExpressionActions::~ExpressionActions() = default;
 
-static std::unordered_set<const ActionsDAG::Node *> processShortCircuitFunctions(const ActionsDAG & actions_dag, bool optimize_short_circuit_function_evaluation);
+static std::unordered_set<const ActionsDAG::Node *> 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<const ActionsDAG::Node *> lazy_executed_nodes;
+    std::unordered_set<const ActionsDAG::Node *> lazy_executed_nodes = processShortCircuitFunctions(*actions_dag, settings.short_circuit_function_evaluation);
-    if (settings.use_short_circuit_function_evaluation)
-        lazy_executed_nodes = processShortCircuitFunctions(*actions_dag, settings.optimize_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<const ActionsDAG::Node *, LazyExecutionInfo> lazy_execution_infos,
+    std::unordered_map<const ActionsDAG::Node *, LazyExecutionInfo> & lazy_execution_infos,
     bool force_enable_lazy_execution,
     std::unordered_set<const ActionsDAG::Node *> & lazy_executed_nodes_out)
 {
@@ -278,8 +277,11 @@ static bool findLazyExecutedNodes(
     return has_lazy_node;
 }
 
-static std::unordered_set<const ActionsDAG::Node *> processShortCircuitFunctions(const ActionsDAG & actions_dag, bool optimize_short_circuit_function_evaluation)
+static std::unordered_set<const ActionsDAG::Node *> 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<const ActionsDAG::Node *> 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;

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.short_circuit_function_evaluation = from.short_circuit_function_evaluation;
-    settings.optimize_short_circuit_function_evaluation = from.optimize_short_circuit_function_evaluation;
 
     return settings;
 }

src/Interpreters/ExpressionActionsSettings.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <Interpreters/Context_fwd.h>
+#include <Core/SettingsEnums.h>
 
 #include <cstddef>
 
@@ -25,8 +26,7 @@ struct ExpressionActionsSettings
 
     CompileExpressions compile_expressions = CompileExpressions::no;
 
-    bool use_short_circuit_function_evaluation = false;
+    ShortCircuitFunctionEvaluation short_circuit_function_evaluation = ShortCircuitFunctionEvaluation::DISABLE;
-    bool optimize_short_circuit_function_evaluation = false;
 
     static ExpressionActionsSettings fromSettings(const Settings & from, CompileExpressions compile_expressions = CompileExpressions::no);
     static ExpressionActionsSettings fromContext(ContextPtr from, CompileExpressions compile_expressions = CompileExpressions::no);

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);