From f28d67a6546c7bfb59db279c74cad1870ad3b996 Mon Sep 17 00:00:00 2001 From: sunset3000 Date: Wed, 11 Oct 2023 16:43:56 +0800 Subject: [PATCH] Fix review comments-1. --- .../AggregateFunctionJavaScriptAdapter.cpp | 5 +++- .../AggregateFunctionJavaScriptAdapter.h | 2 +- .../InterpreterCreateFunctionQuery.cpp | 4 +-- src/Parsers/ASTCreateFunctionQuery.cpp | 3 +- src/Parsers/ASTCreateFunctionQuery.h | 5 +++- src/Parsers/ParserCreateFunctionQuery.cpp | 2 +- src/Parsers/Streaming/ParserArguments.cpp | 4 +-- ...rgument.cpp => ParserFunctionArgument.cpp} | 4 +-- ...uncArgument.h => ParserFunctionArgument.h} | 2 +- .../tests/gtest_create_func_parser.cpp | 28 +++++++------------ 10 files changed, 29 insertions(+), 30 deletions(-) rename src/Parsers/Streaming/{ParserFuncArgument.cpp => ParserFunctionArgument.cpp} (79%) rename src/Parsers/Streaming/{ParserFuncArgument.h => ParserFunctionArgument.h} (89%) diff --git a/src/AggregateFunctions/AggregateFunctionJavaScriptAdapter.cpp b/src/AggregateFunctions/AggregateFunctionJavaScriptAdapter.cpp index c6426b91fc9..11fd1a545e5 100644 --- a/src/AggregateFunctions/AggregateFunctionJavaScriptAdapter.cpp +++ b/src/AggregateFunctions/AggregateFunctionJavaScriptAdapter.cpp @@ -234,7 +234,10 @@ void JavaScriptAggrFunctionState::reinitCache() } AggregateFunctionJavaScriptAdapter::AggregateFunctionJavaScriptAdapter( - const JavaScriptUserDefinedFunctionConfigurationPtr config_, const DataTypes & types, const Array & params_, size_t max_v8_heap_size_in_bytes_) + JavaScriptUserDefinedFunctionConfigurationPtr config_, + const DataTypes & types, + const Array & params_, + size_t max_v8_heap_size_in_bytes_) : IAggregateFunctionHelper(types, params_) , config(config_) , num_arguments(types.size()) diff --git a/src/AggregateFunctions/AggregateFunctionJavaScriptAdapter.h b/src/AggregateFunctions/AggregateFunctionJavaScriptAdapter.h index e1cfd6396d0..2cbaaaf0f2f 100644 --- a/src/AggregateFunctions/AggregateFunctionJavaScriptAdapter.h +++ b/src/AggregateFunctions/AggregateFunctionJavaScriptAdapter.h @@ -87,7 +87,7 @@ class AggregateFunctionJavaScriptAdapter final : public IAggregateFunctionHelper public: AggregateFunctionJavaScriptAdapter( - const JavaScriptUserDefinedFunctionConfigurationPtr config_, + JavaScriptUserDefinedFunctionConfigurationPtr config_, const DataTypes & types, const Array & params_, size_t max_v8_heap_size_in_bytes_); diff --git a/src/Interpreters/InterpreterCreateFunctionQuery.cpp b/src/Interpreters/InterpreterCreateFunctionQuery.cpp index 6f326abbe51..11c506324d1 100644 --- a/src/Interpreters/InterpreterCreateFunctionQuery.cpp +++ b/src/Interpreters/InterpreterCreateFunctionQuery.cpp @@ -54,7 +54,7 @@ BlockIO InterpreterCreateFunctionQuery::execute() bool replace_if_exists = create_function_query.or_replace; /// proton: starts. Handle javascript UDF - if (create_function_query.is_javascript_func) + if (create_function_query.isJavaScript()) return handleJavaScriptUDF(throw_if_exists, replace_if_exists); /// proton: ends @@ -67,7 +67,7 @@ BlockIO InterpreterCreateFunctionQuery::execute() BlockIO InterpreterCreateFunctionQuery::handleJavaScriptUDF(bool throw_if_exists, bool replace_if_exists) { ASTCreateFunctionQuery & create = query_ptr->as(); - assert(create.is_javascript_func); + assert(create.isJavaScript()); const auto func_name = create.getFunctionName(); Poco::JSON::Object::Ptr func = create.toJSON(); diff --git a/src/Parsers/ASTCreateFunctionQuery.cpp b/src/Parsers/ASTCreateFunctionQuery.cpp index d22d653806e..7efbc9cc4be 100644 --- a/src/Parsers/ASTCreateFunctionQuery.cpp +++ b/src/Parsers/ASTCreateFunctionQuery.cpp @@ -49,6 +49,7 @@ void ASTCreateFunctionQuery::formatImpl(const IAST::FormatSettings & settings, I settings.ostr << (settings.hilite ? hilite_identifier : "") << backQuoteIfNeed(getFunctionName()) << (settings.hilite ? hilite_none : ""); /// proton: starts + bool is_javascript_func = isJavaScript(); if (is_javascript_func) { /// arguments @@ -88,7 +89,7 @@ Poco::JSON::Object::Ptr ASTCreateFunctionQuery::toJSON() const Poco::JSON::Object::Ptr func = new Poco::JSON::Object(Poco::JSON_PRESERVE_KEY_ORDER); Poco::JSON::Object::Ptr inner_func = new Poco::JSON::Object(Poco::JSON_PRESERVE_KEY_ORDER); inner_func->set("name", getFunctionName()); - if (!is_javascript_func) + if (!isJavaScript()) { WriteBufferFromOwnString source_buf; formatAST(*function_core, source_buf, false); diff --git a/src/Parsers/ASTCreateFunctionQuery.h b/src/Parsers/ASTCreateFunctionQuery.h index c7e1ae57e5a..4260738b323 100644 --- a/src/Parsers/ASTCreateFunctionQuery.h +++ b/src/Parsers/ASTCreateFunctionQuery.h @@ -21,7 +21,7 @@ class ASTCreateFunctionQuery : public IAST, public ASTQueryWithOnCluster /// proton: starts bool is_aggregation = false; - bool is_javascript_func = false; + String lang = "SQL"; ASTPtr arguments; ASTPtr return_type; /// proton: ends @@ -38,6 +38,9 @@ class ASTCreateFunctionQuery : public IAST, public ASTQueryWithOnCluster /// proton: starts Poco::JSON::Object::Ptr toJSON() const; + + /// If it is a JavaScript UDF + bool isJavaScript() const noexcept { return lang == "JavaScript"; } /// proton: ends }; diff --git a/src/Parsers/ParserCreateFunctionQuery.cpp b/src/Parsers/ParserCreateFunctionQuery.cpp index 3944f7db52c..de8943e1297 100644 --- a/src/Parsers/ParserCreateFunctionQuery.cpp +++ b/src/Parsers/ParserCreateFunctionQuery.cpp @@ -127,7 +127,7 @@ bool ParserCreateFunctionQuery::parseImpl(IParser::Pos & pos, ASTPtr & node, Exp /// proton: starts create_function_query->is_aggregation = is_aggregation; - create_function_query->is_javascript_func = is_javascript_func; + create_function_query->lang = is_javascript_func ? "JavaScript" : "SQL"; create_function_query->arguments = std::move(arguments); create_function_query->return_type = std::move(return_type); /// proton: ends diff --git a/src/Parsers/Streaming/ParserArguments.cpp b/src/Parsers/Streaming/ParserArguments.cpp index bcc5726795b..407aefd36aa 100644 --- a/src/Parsers/Streaming/ParserArguments.cpp +++ b/src/Parsers/Streaming/ParserArguments.cpp @@ -1,6 +1,6 @@ #include -#include +#include #include #include @@ -24,7 +24,7 @@ bool ParserArguments::parseImpl(Pos & pos, ASTPtr & node, Expected & expected, [ ++pos; - ParserList arg_list_parser(std::make_unique(), std::make_unique(TokenType::Comma), false); + ParserList arg_list_parser(std::make_unique(), std::make_unique(TokenType::Comma), false); ASTPtr expr_list_args; if (!arg_list_parser.parse(pos, expr_list_args, expected)) diff --git a/src/Parsers/Streaming/ParserFuncArgument.cpp b/src/Parsers/Streaming/ParserFunctionArgument.cpp similarity index 79% rename from src/Parsers/Streaming/ParserFuncArgument.cpp rename to src/Parsers/Streaming/ParserFunctionArgument.cpp index f618f5713e9..41c74baa399 100644 --- a/src/Parsers/Streaming/ParserFuncArgument.cpp +++ b/src/Parsers/Streaming/ParserFunctionArgument.cpp @@ -1,4 +1,4 @@ -#include +#include #include #include @@ -9,7 +9,7 @@ namespace DB { -bool ParserFuncArgument::parseImpl(Pos & pos, ASTPtr & node, Expected & expected, [[maybe_unused]] bool hint) +bool ParserFunctionArgument::parseImpl(Pos & pos, ASTPtr & node, Expected & expected, [[maybe_unused]] bool hint) { if (!pos.isValid()) return false; diff --git a/src/Parsers/Streaming/ParserFuncArgument.h b/src/Parsers/Streaming/ParserFunctionArgument.h similarity index 89% rename from src/Parsers/Streaming/ParserFuncArgument.h rename to src/Parsers/Streaming/ParserFunctionArgument.h index 8a194cc05eb..dda201f54b6 100644 --- a/src/Parsers/Streaming/ParserFuncArgument.h +++ b/src/Parsers/Streaming/ParserFunctionArgument.h @@ -8,7 +8,7 @@ namespace DB /** Query like this used as UDF argument list: * complex tuple(float32, datetime64(3)) */ -class ParserFuncArgument : public IParserBase +class ParserFunctionArgument : public IParserBase { private: const char * getName() const override { return "Function Argument query"; } diff --git a/src/Parsers/tests/gtest_create_func_parser.cpp b/src/Parsers/tests/gtest_create_func_parser.cpp index e3f7952317d..b6556d219a7 100644 --- a/src/Parsers/tests/gtest_create_func_parser.cpp +++ b/src/Parsers/tests/gtest_create_func_parser.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -18,15 +19,6 @@ using namespace DB; -[[maybe_unused]] static String astToString(IAST * ast) -{ - WriteBufferFromOwnString buf; - formatAST(*ast, buf, false); - return buf.str(); -} - -/// Tests for external dictionaries DDL parser - TEST(ParserCreateFunctionQuery, UDFFunction) { String input = " CREATE FUNCTION add_five(value float32)" @@ -41,22 +33,22 @@ TEST(ParserCreateFunctionQuery, UDFFunction) ASTPtr ast = parseQuery(parser, input.data(), input.data() + input.size(), "", 0, 0); ASTCreateFunctionQuery * create = ast->as(); EXPECT_EQ(create->getFunctionName(), "add_five"); - EXPECT_EQ(create->is_javascript_func, true); + EXPECT_EQ(create->lang, "JavaScript"); EXPECT_NE(create->function_core, nullptr); EXPECT_NE(create->arguments, nullptr); /// Check arguments - String args = astToString(create->arguments.get()); + String args = queryToString(*create->arguments.get(), true); EXPECT_EQ(args, "(value float32)"); /// Check return type - String ret = astToString(create->return_type.get()); + String ret = queryToString(*create->return_type.get(), true); EXPECT_EQ(ret, "float32"); ASTLiteral * js_src = create->function_core->as(); EXPECT_EQ(js_src->value.safeGet(), " function add_five(value){for(let i=0;iarguments, nullptr); /// Check arguments - String args = astToString(create->arguments.get()); + String args = queryToString(*create->arguments.get(), true); EXPECT_EQ(args, "(value float32, complex array(datetime64(3)), t tuple(i int64, f float32))"); /// Check return type - String ret = astToString(create->return_type.get()); + String ret = queryToString(*create->return_type.get(), true); EXPECT_EQ(ret, "array(float64)"); auto json = create->toJSON(); @@ -141,11 +133,11 @@ TEST(ParserCreateFunctionQuery, ReturnType) EXPECT_NE(create->arguments, nullptr); /// Check arguments - String args = astToString(create->arguments.get()); + String args = queryToString(*create->arguments.get(), true); EXPECT_EQ(args, "(value float32, dt datetime64(3))"); /// Check return type - String ret = astToString(create->return_type.get()); + String ret = queryToString(*create->return_type.get(), true); EXPECT_EQ(ret, "tuple(v float64, dt datetime64(1), id string)"); auto json = create->toJSON(); @@ -155,4 +147,4 @@ TEST(ParserCreateFunctionQuery, ReturnType) EXPECT_EQ( json_str, R"###({"function":{"name":"add_five","arguments":[{"name":"value","type":"float32"},{"name":"dt","type":"datetime64(3)"}],"type":"javascript","is_aggregation":true,"return_type":"tuple(v float64, dt datetime64(1), id string)","source":" return false;"}})###"); -} \ No newline at end of file +}