From 567c1a33f4f159fcf963f0e449f1024aa5381583 Mon Sep 17 00:00:00 2001
From: jakevin <30525741+jackwener@users.noreply.github.com>
Date: Fri, 7 Jan 2022 16:01:55 +0800
Subject: [PATCH] Fix coalesce bug (#3653)

* fix coalesce

* fix test

* add test

* add tck

* fix

* fix

* fix
---
 src/common/function/FunctionManager.cpp       | 29 +++----------
 .../function/test/FunctionManagerTest.cpp     | 11 +++--
 tests/tck/features/function/coalesce.feature  | 41 +++++++++++++++++++
 3 files changed, 54 insertions(+), 27 deletions(-)
 create mode 100644 tests/tck/features/function/coalesce.feature

diff --git a/src/common/function/FunctionManager.cpp b/src/common/function/FunctionManager.cpp
index b03d253c6b9..7262b7fea9c 100644
--- a/src/common/function/FunctionManager.cpp
+++ b/src/common/function/FunctionManager.cpp
@@ -38,6 +38,7 @@ std::unordered_map<std::string, Value::Type> FunctionManager::variadicFunReturnT
     {"concat", Value::Type::STRING},
     {"concat_ws", Value::Type::STRING},
     {"cos_similarity", Value::Type::FLOAT},
+    {"coalesce", Value::Type::__EMPTY__},
 };
 
 std::unordered_map<std::string, std::vector<TypeSignature>> FunctionManager::typeSignature_ = {
@@ -283,10 +284,6 @@ std::unordered_map<std::string, std::vector<TypeSignature>> FunctionManager::typ
      {
          TypeSignature({Value::Type::LIST}, Value::Type::__EMPTY__),
      }},
-    {"coalesce",
-     {
-         TypeSignature({Value::Type::LIST}, Value::Type::__EMPTY__),
-     }},
     {"range",
      {TypeSignature({Value::Type::INT, Value::Type::INT}, Value::Type::LIST),
       TypeSignature({Value::Type::INT, Value::Type::INT, Value::Type::INT}, Value::Type::LIST)}},
@@ -2092,29 +2089,15 @@ FunctionManager::FunctionManager() {
   {
     auto &attr = functions_["coalesce"];
     attr.minArity_ = 1;
-    attr.maxArity_ = 1;
+    attr.maxArity_ = INT64_MAX;
     attr.isPure_ = true;
     attr.body_ = [](const auto &args) -> Value {
-      switch (args[0].get().type()) {
-        case Value::Type::NULLVALUE: {
-          return Value::kNullValue;
-        }
-        case Value::Type::LIST: {
-          auto &list = args[0].get().getList();
-          if (list.values.empty()) {
-            return Value::kNullValue;
-          }
-          for (auto &i : list.values) {
-            if (i != Value::kNullValue) {
-              return i;
-            }
-          }
-          return Value::kNullValue;
-        }
-        default: {
-          return Value::kNullBadType;
+      for (size_t i = 0; i < args.size(); ++i) {
+        if (args[i].get().type() != Value::Type::NULLVALUE) {
+          return args[i].get();
         }
       }
+      return Value::kNullValue;
     };
   }
   {
diff --git a/src/common/function/test/FunctionManagerTest.cpp b/src/common/function/test/FunctionManagerTest.cpp
index 0bcc1f133d7..7c0dc1a423e 100644
--- a/src/common/function/test/FunctionManagerTest.cpp
+++ b/src/common/function/test/FunctionManagerTest.cpp
@@ -1648,26 +1648,29 @@ TEST_F(FunctionManagerTest, ScalarFunctionTest) {
     TEST_FUNCTION(last, args_["nullvalue"], Value::kNullValue);
     TEST_FUNCTION(coalesce, args_["nullvalue"], Value::kNullValue);
 
+    TEST_FUNCTION(coalesce, args_["range1"], 1);
+    TEST_FUNCTION(coalesce, args_["float"], 1.1);
+
     std::vector<Value> args;
     List list;
     list.values.emplace_back(Value::kNullValue);
-    args.push_back(list);
+    args.emplace_back(list);
 
     TEST_FUNCTION(head, args, Value::kNullValue);
     TEST_FUNCTION(last, args, Value::kNullValue);
-    TEST_FUNCTION(coalesce, args, Value::kNullValue);
+    TEST_FUNCTION(coalesce, args, list);
 
     list.values.insert(list.values.begin(), "head");
     args[0] = list;
     TEST_FUNCTION(head, args, "head");
     TEST_FUNCTION(last, args, Value::kNullValue);
-    TEST_FUNCTION(coalesce, args, "head");
+    TEST_FUNCTION(coalesce, args, list);
 
     list.values.emplace_back("last");
     args[0] = list;
     TEST_FUNCTION(head, args, "head")
     TEST_FUNCTION(last, args, "last");
-    TEST_FUNCTION(coalesce, args, "head");
+    TEST_FUNCTION(coalesce, args, list);
   }
   {
     // length(null) return null
diff --git a/tests/tck/features/function/coalesce.feature b/tests/tck/features/function/coalesce.feature
new file mode 100644
index 00000000000..c06ae41ec50
--- /dev/null
+++ b/tests/tck/features/function/coalesce.feature
@@ -0,0 +1,41 @@
+Feature: Fetch Int Vid Edges
+
+  Background:
+    Test coalesce function
+
+  Scenario: test normal case
+    When executing query:
+      """
+      RETURN coalesce(null,1) as result;
+      """
+    Then the result should be, in any order:
+      | result |
+      | 1      |
+    When executing query:
+      """
+      RETURN coalesce(1,2,3) as result;
+      """
+    Then the result should be, in any order:
+      | result |
+      | 1      |
+    When executing query:
+      """
+      RETURN coalesce(null) as result;
+      """
+    Then the result should be, in any order:
+      | result |
+      | NULL   |
+    When executing query:
+      """
+      RETURN coalesce(null,[1,2,3]) as result;
+      """
+    Then the result should be, in any order:
+      | result    |
+      | [1, 2, 3] |
+    When executing query:
+      """
+      RETURN coalesce(null,1.234) as result;
+      """
+    Then the result should be, in any order:
+      | result |
+      | 1.234  |