From c9bffa9541b98229cac027e7afb2a584a2b0d4c1 Mon Sep 17 00:00:00 2001 From: jackwener <30525741+jackwener@users.noreply.github.com> Date: Thu, 21 Oct 2021 18:46:02 +0800 Subject: [PATCH 1/6] round modification --- src/common/function/FunctionManager.cpp | 26 +++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/common/function/FunctionManager.cpp b/src/common/function/FunctionManager.cpp index 07b402e74bb..80693f67e55 100644 --- a/src/common/function/FunctionManager.cpp +++ b/src/common/function/FunctionManager.cpp @@ -56,7 +56,9 @@ std::unordered_map> FunctionManager::typ TypeSignature({Value::Type::FLOAT}, Value::Type::FLOAT)}}, {"round", {TypeSignature({Value::Type::INT}, Value::Type::FLOAT), - TypeSignature({Value::Type::FLOAT}, Value::Type::FLOAT)}}, + TypeSignature({Value::Type::INT, Value::Type::INT}, Value::Type::FLOAT), + TypeSignature({Value::Type::FLOAT}, Value::Type::FLOAT), + TypeSignature({Value::Type::FLOAT, Value::Type::INT}, Value::Type::FLOAT)}}, {"sqrt", {TypeSignature({Value::Type::INT}, Value::Type::FLOAT), TypeSignature({Value::Type::FLOAT}, Value::Type::FLOAT)}}, @@ -531,21 +533,33 @@ FunctionManager::FunctionManager() { } }; } + {"round", + {TypeSignature({Value::Type::INT}, Value::Type::FLOAT), + TypeSignature({Value::Type::INT, Value::Type::INT}, Value::Type::FLOAT), + TypeSignature({Value::Type::FLOAT}, Value::Type::FLOAT), + TypeSignature({Value::Type::FLOAT, Value::Type::INT}, Value::Type::FLOAT)}}, + { // to nearest integral (as a floating-point value) auto &attr = functions_["round"]; attr.minArity_ = 1; - attr.maxArity_ = 1; + attr.maxArity_ = 2; attr.isPure_ = true; attr.body_ = [](const auto &args) -> Value { switch (args[0].get().type()) { case Value::Type::NULLVALUE: { return Value::kNullValue; } - case Value::Type::INT: { - return std::round(args[0].get().getInt()); - } - case Value::Type::FLOAT: { + case Value::Type::FLOAT, Value::Type::INT: { + if (args.size() == 2) { + switch (args[1].get().type()) { + case Value::Type::INT: + return std::round(args[0].get().getFloat() * pow(10, args[1].get().getInt())) / + pow(10, args[1].get().getInt()); + default: + return Value::kNullBadType; + } + } return std::round(args[0].get().getFloat()); } default: { From 10121e97e6c1f76d5dd2f56de1e991f494b1bd8a Mon Sep 17 00:00:00 2001 From: jackwener <30525741+jackwener@users.noreply.github.com> Date: Fri, 22 Oct 2021 15:01:05 +0800 Subject: [PATCH 2/6] add test and fix problem --- src/common/function/FunctionManager.cpp | 9 ++------- src/common/function/test/FunctionManagerTest.cpp | 8 ++++++++ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/common/function/FunctionManager.cpp b/src/common/function/FunctionManager.cpp index 80693f67e55..daf245fe336 100644 --- a/src/common/function/FunctionManager.cpp +++ b/src/common/function/FunctionManager.cpp @@ -533,12 +533,6 @@ FunctionManager::FunctionManager() { } }; } - {"round", - {TypeSignature({Value::Type::INT}, Value::Type::FLOAT), - TypeSignature({Value::Type::INT, Value::Type::INT}, Value::Type::FLOAT), - TypeSignature({Value::Type::FLOAT}, Value::Type::FLOAT), - TypeSignature({Value::Type::FLOAT, Value::Type::INT}, Value::Type::FLOAT)}}, - { // to nearest integral (as a floating-point value) auto &attr = functions_["round"]; @@ -550,7 +544,8 @@ FunctionManager::FunctionManager() { case Value::Type::NULLVALUE: { return Value::kNullValue; } - case Value::Type::FLOAT, Value::Type::INT: { + case Value::Type::INT: + case Value::Type::FLOAT: { if (args.size() == 2) { switch (args[1].get().type()) { case Value::Type::INT: diff --git a/src/common/function/test/FunctionManagerTest.cpp b/src/common/function/test/FunctionManagerTest.cpp index b8747be60c6..e1e583dfd22 100644 --- a/src/common/function/test/FunctionManagerTest.cpp +++ b/src/common/function/test/FunctionManagerTest.cpp @@ -917,11 +917,19 @@ TEST_F(FunctionManagerTest, returnType) { ASSERT_TRUE(result.ok()); EXPECT_EQ(result.value(), Value::Type::FLOAT); } + { + auto result = FunctionManager::getReturnType("round", {Value::Type::Int, Value::Type::Int}) + ASSERT_TRUE(result.ok()) EXPECT_EQ(result.value(), Value::Type::FLOAT); + } { auto result = FunctionManager::getReturnType("round", {Value::Type::FLOAT}); ASSERT_TRUE(result.ok()); EXPECT_EQ(result.value(), Value::Type::FLOAT); } + { + auto result = FunctionManager::getReturnType("round", {Value::Type::FLOAT, Value::Type::Int}) + ASSERT_TRUE(result.ok()) EXPECT_EQ(result.value(), Value::Type::FLOAT); + } { auto result = FunctionManager::getReturnType("cbrt", {Value::Type::INT}); ASSERT_TRUE(result.ok()); From 068fb09cd922897b08736ba6c3f570f10f0439eb Mon Sep 17 00:00:00 2001 From: jackwener <30525741+jackwener@users.noreply.github.com> Date: Fri, 22 Oct 2021 18:20:28 +0800 Subject: [PATCH 3/6] fix --- src/common/function/test/FunctionManagerTest.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/common/function/test/FunctionManagerTest.cpp b/src/common/function/test/FunctionManagerTest.cpp index e1e583dfd22..379920a5a81 100644 --- a/src/common/function/test/FunctionManagerTest.cpp +++ b/src/common/function/test/FunctionManagerTest.cpp @@ -918,8 +918,9 @@ TEST_F(FunctionManagerTest, returnType) { EXPECT_EQ(result.value(), Value::Type::FLOAT); } { - auto result = FunctionManager::getReturnType("round", {Value::Type::Int, Value::Type::Int}) - ASSERT_TRUE(result.ok()) EXPECT_EQ(result.value(), Value::Type::FLOAT); + auto result = FunctionManager::getReturnType("round", {Value::Type::INT, Value::Type::INT}); + ASSERT_TRUE(result.ok()); + EXPECT_EQ(result.value(), Value::Type::FLOAT); } { auto result = FunctionManager::getReturnType("round", {Value::Type::FLOAT}); @@ -927,8 +928,9 @@ TEST_F(FunctionManagerTest, returnType) { EXPECT_EQ(result.value(), Value::Type::FLOAT); } { - auto result = FunctionManager::getReturnType("round", {Value::Type::FLOAT, Value::Type::Int}) - ASSERT_TRUE(result.ok()) EXPECT_EQ(result.value(), Value::Type::FLOAT); + auto result = FunctionManager::getReturnType("round", {Value::Type::FLOAT, Value::Type::INT}); + ASSERT_TRUE(result.ok()); + EXPECT_EQ(result.value(), Value::Type::FLOAT); } { auto result = FunctionManager::getReturnType("cbrt", {Value::Type::INT}); From 4d4a39e31b1fea8b4b5209e78d625a0bea63e934 Mon Sep 17 00:00:00 2001 From: jackwener <30525741+jackwener@users.noreply.github.com> Date: Wed, 27 Oct 2021 18:28:31 +0800 Subject: [PATCH 4/6] add test --- src/common/function/FunctionManager.cpp | 10 ++++--- .../function/test/FunctionManagerTest.cpp | 8 +++++ .../features/expression/FunctionCall.feature | 30 +++++++++++++++++++ 3 files changed, 44 insertions(+), 4 deletions(-) diff --git a/src/common/function/FunctionManager.cpp b/src/common/function/FunctionManager.cpp index daf245fe336..a7837eb2252 100644 --- a/src/common/function/FunctionManager.cpp +++ b/src/common/function/FunctionManager.cpp @@ -548,11 +548,13 @@ FunctionManager::FunctionManager() { case Value::Type::FLOAT: { if (args.size() == 2) { switch (args[1].get().type()) { - case Value::Type::INT: - return std::round(args[0].get().getFloat() * pow(10, args[1].get().getInt())) / - pow(10, args[1].get().getInt()); - default: + case Value::Type::INT: { + auto decimal = args[1].get().getInt(); + return std::round(args[0].get().getFloat() * pow(10, decimal)) / pow(10, decimal); + } + default: { return Value::kNullBadType; + } } } return std::round(args[0].get().getFloat()); diff --git a/src/common/function/test/FunctionManagerTest.cpp b/src/common/function/test/FunctionManagerTest.cpp index 379920a5a81..41f1ad8a706 100644 --- a/src/common/function/test/FunctionManagerTest.cpp +++ b/src/common/function/test/FunctionManagerTest.cpp @@ -104,6 +104,9 @@ std::unordered_map> FunctionManagerTest::args_ = {"one", {-1.2}}, {"two", {2, 4}}, {"pow", {2, 3}}, + {"round1", {11111.11111, 2}}, + {"round2", {11111.11111, -1}}, + {"round3", {11111.11111, -5}}, {"radians", {180}}, {"range1", {1, 5}}, {"range2", {1, 5, 2}}, @@ -269,6 +272,11 @@ TEST_F(FunctionManagerTest, functionCall) { TEST_FUNCTION(log, args_["int"], std::log(4)); TEST_FUNCTION(log2, args_["int"], 2.0); } + { + TEST_FUNCTION(round, args_["round1"], 11111.11); + TEST_FUNCTION(round, args_["round2"], 11110.0); + TEST_FUNCTION(round, args_["round3"], 0.0); + } { TEST_FUNCTION(range, args_["range1"], Value(List({1, 2, 3, 4, 5}))); TEST_FUNCTION(range, args_["range2"], Value(List({1, 3, 5}))); diff --git a/tests/tck/features/expression/FunctionCall.feature b/tests/tck/features/expression/FunctionCall.feature index bb5dcc1602e..6994180f08b 100644 --- a/tests/tck/features/expression/FunctionCall.feature +++ b/tests/tck/features/expression/FunctionCall.feature @@ -122,6 +122,36 @@ Feature: Function Call Expression | result | | NULL | + Scenario: round + When executing query: + """ + YIELD round(1.1111111111, 8) as result + """ + Then the result should be, in any order: + | result | + | 1.11111111 | + When executing query: + """ + YIELD round(1111.111111, 0) as result + """ + Then the result should be, in any order: + | result | + | 1111.0 | + When executing query: + """ + YIELD round(1111.111111, -3) as result + """ + Then the result should be, in any order: + | result | + | 1000.0 | + When executing query: + """ + YIELD round(1111.111111, -4) as result + """ + Then the result should be, in any order: + | result | + | 0.0 | + Scenario: error check When executing query: """ From 6cfc8575ce4054c87bc58d50bd7c74ab36a7b711 Mon Sep 17 00:00:00 2001 From: jackwener <30525741+jackwener@users.noreply.github.com> Date: Thu, 28 Oct 2021 10:09:54 +0800 Subject: [PATCH 5/6] switch to if --- src/common/function/FunctionManager.cpp | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/common/function/FunctionManager.cpp b/src/common/function/FunctionManager.cpp index 5fea6845d37..cd42c1d19d0 100644 --- a/src/common/function/FunctionManager.cpp +++ b/src/common/function/FunctionManager.cpp @@ -552,14 +552,11 @@ FunctionManager::FunctionManager() { case Value::Type::INT: case Value::Type::FLOAT: { if (args.size() == 2) { - switch (args[1].get().type()) { - case Value::Type::INT: { - auto decimal = args[1].get().getInt(); - return std::round(args[0].get().getFloat() * pow(10, decimal)) / pow(10, decimal); - } - default: { - return Value::kNullBadType; - } + if (args[1].get().type() == Value::Type::INT) { + auto decimal = args[1].get().getInt(); + return std::round(args[0].get().getFloat() * pow(10, decimal)) / pow(10, decimal); + } else { + return Value::kNullBadType; } } return std::round(args[0].get().getFloat()); From 19d61c8b9b206bbdae6779f09b31ed144c02ca4f Mon Sep 17 00:00:00 2001 From: jackwener <30525741+jackwener@users.noreply.github.com> Date: Thu, 28 Oct 2021 17:21:16 +0800 Subject: [PATCH 6/6] add test --- .../features/expression/FunctionCall.feature | 30 ++++++++++++++----- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/tests/tck/features/expression/FunctionCall.feature b/tests/tck/features/expression/FunctionCall.feature index 6994180f08b..1930e4f17b1 100644 --- a/tests/tck/features/expression/FunctionCall.feature +++ b/tests/tck/features/expression/FunctionCall.feature @@ -125,28 +125,42 @@ Feature: Function Call Expression Scenario: round When executing query: """ - YIELD round(1.1111111111, 8) as result + YIELD round(3.1415926, 9) as result """ Then the result should be, in any order: - | result | - | 1.11111111 | + | result | + | 3.1415926 | When executing query: """ - YIELD round(1111.111111, 0) as result + YIELD round(3.1415926, 2) as result """ Then the result should be, in any order: | result | - | 1111.0 | + | 3.14 | When executing query: """ - YIELD round(1111.111111, -3) as result + YIELD round(3.1415926, 3) as result """ Then the result should be, in any order: | result | - | 1000.0 | + | 3.142 | When executing query: """ - YIELD round(1111.111111, -4) as result + YIELD round(3.14159265359, 0) as result + """ + Then the result should be, in any order: + | result | + | 3.0 | + When executing query: + """ + YIELD round(35543.14159265359, -3) as result + """ + Then the result should be, in any order: + | result | + | 36000.0 | + When executing query: + """ + YIELD round(35543.14159265359, -5) as result """ Then the result should be, in any order: | result |