Skip to content

Commit

Permalink
Fix review comments-1.
Browse files Browse the repository at this point in the history
  • Loading branch information
sunset3000 committed Oct 11, 2023
1 parent 619a5a4 commit f28d67a
Show file tree
Hide file tree
Showing 10 changed files with 29 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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<AggregateFunctionJavaScriptAdapter>(types, params_)
, config(config_)
, num_arguments(types.size())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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_);
Expand Down
4 changes: 2 additions & 2 deletions src/Interpreters/InterpreterCreateFunctionQuery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -67,7 +67,7 @@ BlockIO InterpreterCreateFunctionQuery::execute()
BlockIO InterpreterCreateFunctionQuery::handleJavaScriptUDF(bool throw_if_exists, bool replace_if_exists)
{
ASTCreateFunctionQuery & create = query_ptr->as<ASTCreateFunctionQuery &>();
assert(create.is_javascript_func);
assert(create.isJavaScript());

const auto func_name = create.getFunctionName();
Poco::JSON::Object::Ptr func = create.toJSON();
Expand Down
3 changes: 2 additions & 1 deletion src/Parsers/ASTCreateFunctionQuery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 4 additions & 1 deletion src/Parsers/ASTCreateFunctionQuery.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
};

Expand Down
2 changes: 1 addition & 1 deletion src/Parsers/ParserCreateFunctionQuery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/Parsers/Streaming/ParserArguments.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#include <Parsers/Streaming/ParserArguments.h>

#include <Parsers/Streaming/ParserFuncArgument.h>
#include <Parsers/Streaming/ParserFunctionArgument.h>

#include <Parsers/ASTFunctionWithKeyValueArguments.h>
#include <Parsers/CommonParsers.h>
Expand All @@ -24,7 +24,7 @@ bool ParserArguments::parseImpl(Pos & pos, ASTPtr & node, Expected & expected, [

++pos;

ParserList arg_list_parser(std::make_unique<ParserFuncArgument>(), std::make_unique<ParserToken>(TokenType::Comma), false);
ParserList arg_list_parser(std::make_unique<ParserFunctionArgument>(), std::make_unique<ParserToken>(TokenType::Comma), false);

ASTPtr expr_list_args;
if (!arg_list_parser.parse(pos, expr_list_args, expected))
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#include <Parsers/Streaming/ParserFuncArgument.h>
#include <Parsers/Streaming/ParserFunctionArgument.h>

#include <Parsers/ASTIdentifier.h>
#include <Parsers/ASTNameTypePair.h>
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"; }
Expand Down
28 changes: 10 additions & 18 deletions src/Parsers/tests/gtest_create_func_parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,14 @@
#include <Parsers/TablePropertiesQueriesASTs.h>
#include <Parsers/formatAST.h>
#include <Parsers/parseQuery.h>
#include <Parsers/queryToString.h>
#include <base/types.h>

#include <gtest/gtest.h>
#include <Poco/JSON/Parser.h>

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)"
Expand All @@ -41,22 +33,22 @@ TEST(ParserCreateFunctionQuery, UDFFunction)
ASTPtr ast = parseQuery(parser, input.data(), input.data() + input.size(), "", 0, 0);
ASTCreateFunctionQuery * create = ast->as<ASTCreateFunctionQuery>();
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<ASTLiteral>();
EXPECT_EQ(js_src->value.safeGet<String>(), " function add_five(value){for(let i=0;i<value.length;i++){value[i]=value[i]+5}return value}");

String func_str = astToString(create);
String func_str = queryToString(*create, true);
EXPECT_EQ(
func_str,
"CREATE FUNCTION add_five(value float32) RETURNS float32 AS $$\n function add_five(value){for(let "
Expand Down Expand Up @@ -107,11 +99,11 @@ TEST(ParserCreateFunctionQuery, ArgTypes)
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, 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();
Expand Down Expand Up @@ -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();
Expand All @@ -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;"}})###");
}
}

0 comments on commit f28d67a

Please sign in to comment.