From 6386e839536979e30aa06b6f4bf94dbe921fa458 Mon Sep 17 00:00:00 2001
From: Dmitry Novik <n0vik@clickhouse.com>
Date: Tue, 3 Jan 2023 16:50:06 +0000
Subject: [PATCH] Fixes after review

---
 src/Analyzer/FunctionNode.cpp         |  2 +-
 src/Analyzer/FunctionNode.h           |  2 +-
 src/Analyzer/QueryTreePassManager.cpp |  2 +-
 src/Interpreters/ActionsDAG.cpp       | 53 +++++++++++++++------------
 src/Interpreters/ActionsDAG.h         |  2 -
 5 files changed, 32 insertions(+), 29 deletions(-)

diff --git a/src/Analyzer/FunctionNode.cpp b/src/Analyzer/FunctionNode.cpp
index e9970babdec..84a1d5025e3 100644
--- a/src/Analyzer/FunctionNode.cpp
+++ b/src/Analyzer/FunctionNode.cpp
@@ -43,7 +43,7 @@ ColumnsWithTypeAndName FunctionNode::getArgumentColumns() const
         argument.type = arg->getResultType();
         if (auto * constant = arg->as<ConstantNode>())
             argument.column = argument.type->createColumnConst(1, constant->getValue());
-        argument_columns.push_back(argument);
+        argument_columns.push_back(std::move(argument));
     }
     return argument_columns;
 }
diff --git a/src/Analyzer/FunctionNode.h b/src/Analyzer/FunctionNode.h
index 4fe8880ce5d..49e66ba32c1 100644
--- a/src/Analyzer/FunctionNode.h
+++ b/src/Analyzer/FunctionNode.h
@@ -5,9 +5,9 @@
 #include <Analyzer/ConstantValue.h>
 #include <Analyzer/IQueryTreeNode.h>
 #include <Analyzer/ListNode.h>
+#include <Common/typeid_cast.h>
 #include <Core/ColumnsWithTypeAndName.h>
 #include <Core/IResolvedFunction.h>
-#include <Common/typeid_cast.h>
 #include <Functions/IFunction.h>
 
 namespace DB
diff --git a/src/Analyzer/QueryTreePassManager.cpp b/src/Analyzer/QueryTreePassManager.cpp
index 1fa6297c8c0..4148d42ee23 100644
--- a/src/Analyzer/QueryTreePassManager.cpp
+++ b/src/Analyzer/QueryTreePassManager.cpp
@@ -59,7 +59,7 @@ class ValidationChecker : public InDepthQueryTreeVisitor<ValidationChecker>
         if (!function->isResolved())
             throw Exception(ErrorCodes::LOGICAL_ERROR,
             "Function {} is not resolved after running {} pass",
-            function->toAST()->dumpTree(), pass_name);
+            function->toAST()->formatForErrorMessage(), pass_name);
     }
 
 public:
diff --git a/src/Interpreters/ActionsDAG.cpp b/src/Interpreters/ActionsDAG.cpp
index bceffcc5708..6e935898ddd 100644
--- a/src/Interpreters/ActionsDAG.cpp
+++ b/src/Interpreters/ActionsDAG.cpp
@@ -33,6 +33,35 @@ namespace ErrorCodes
     extern const int BAD_ARGUMENTS;
 }
 
+namespace
+{
+
+std::pair<ColumnsWithTypeAndName, bool> getFunctionArguments(const ActionsDAG::NodeRawConstPtrs & children)
+{
+    size_t num_arguments = children.size();
+
+    bool all_const = true;
+    ColumnsWithTypeAndName arguments(num_arguments);
+
+    for (size_t i = 0; i < num_arguments; ++i)
+    {
+        const auto & child = *children[i];
+
+        ColumnWithTypeAndName argument;
+        argument.column = child.column;
+        argument.type = child.result_type;
+        argument.name = child.result_name;
+
+        if (!argument.column || !isColumnConst(*argument.column))
+            all_const = false;
+
+        arguments[i] = std::move(argument);
+    }
+    return { std::move(arguments), all_const };
+}
+
+}
+
 void ActionsDAG::Node::toTree(JSONBuilder::JSONMap & map) const
 {
     map.add("Node Type", magic_enum::enum_name(type));
@@ -250,30 +279,6 @@ const ActionsDAG::Node & ActionsDAG::addFunctionImpl(
     return addNode(std::move(node));
 }
 
-std::pair<ColumnsWithTypeAndName, bool> ActionsDAG::getFunctionArguments(const NodeRawConstPtrs & children)
-{
-    size_t num_arguments = children.size();
-
-    bool all_const = true;
-    ColumnsWithTypeAndName arguments(num_arguments);
-
-    for (size_t i = 0; i < num_arguments; ++i)
-    {
-        const auto & child = *children[i];
-
-        ColumnWithTypeAndName argument;
-        argument.column = child.column;
-        argument.type = child.result_type;
-        argument.name = child.result_name;
-
-        if (!argument.column || !isColumnConst(*argument.column))
-            all_const = false;
-
-        arguments[i] = std::move(argument);
-    }
-    return { std::move(arguments), all_const };
-}
-
 const ActionsDAG::Node & ActionsDAG::findInOutputs(const std::string & name) const
 {
     if (const auto * node = tryFindInOutputs(name))
diff --git a/src/Interpreters/ActionsDAG.h b/src/Interpreters/ActionsDAG.h
index 8d1ea470277..a26694e00f5 100644
--- a/src/Interpreters/ActionsDAG.h
+++ b/src/Interpreters/ActionsDAG.h
@@ -358,8 +358,6 @@ private:
         std::string result_name,
         bool all_const);
 
-    static std::pair<ColumnsWithTypeAndName, bool> getFunctionArguments(const NodeRawConstPtrs & children);
-
 #if USE_EMBEDDED_COMPILER
     void compileFunctions(size_t min_count_to_compile_expression, const std::unordered_set<const Node *> & lazy_executed_nodes = {});
 #endif