From 3a800956fa3dca756f59344f2fa4757eddc396e1 Mon Sep 17 00:00:00 2001
From: "kyle.cao" <kyle.cao@vesoft.com>
Date: Tue, 2 Mar 2021 22:41:55 +0800
Subject: [PATCH] fix update/upsert (#772)

* fix update/upsert when clause

fix non bool col filter

add test cases

fix ut

fix tck

* fix copy elision

Co-authored-by: jie.wang <38901892+jievince@users.noreply.github.com>
Co-authored-by: Yee <2520865+yixinglu@users.noreply.github.com>
---
 src/validator/MutateValidator.cpp          | 29 +++++++++----
 src/validator/test/MutateValidatorTest.cpp |  2 +-
 tests/tck/features/update/Update.feature   | 50 +++++++++++++++++++++-
 3 files changed, 71 insertions(+), 10 deletions(-)

diff --git a/src/validator/MutateValidator.cpp b/src/validator/MutateValidator.cpp
index 1c6c69c2..3a0b3f32 100644
--- a/src/validator/MutateValidator.cpp
+++ b/src/validator/MutateValidator.cpp
@@ -489,15 +489,28 @@ Status UpdateValidator::initProps() {
 
 Status UpdateValidator::getCondition() {
     auto *clause = sentence_->whenClause();
-    if (clause != nullptr) {
-        auto filter = clause->filter();
-        if (filter != nullptr) {
-            std::string encodeStr;
-            auto copyFilterExpr = filter->clone();
-            NG_LOG_AND_RETURN_IF_ERROR(
-                checkAndResetSymExpr(copyFilterExpr.get(), name_, encodeStr));
-            condition_ = std::move(encodeStr);
+    if (clause && clause->filter()) {
+        auto filter = clause->filter()->clone();
+        bool hasWrongType = false;
+        auto symExpr = rewriteSymExpr(filter.get(), name_, hasWrongType, isEdge_);
+        if (hasWrongType) {
+            return Status::SemanticError("Has wrong expr in `%s'",
+                                         filter->toString().c_str());
+        }
+        if (symExpr != nullptr) {
+            filter.reset(symExpr.release());
+        }
+        auto typeStatus = deduceExprType(filter.get());
+        NG_RETURN_IF_ERROR(typeStatus);
+        auto type = typeStatus.value();
+        if (type != Value::Type::BOOL
+            && type != Value::Type::NULLVALUE
+            && type != Value::Type::__EMPTY__) {
+            std::stringstream ss;
+            ss << "`" << filter->toString() << "', expected Boolean, but was `" << type << "'";
+            return Status::SemanticError(ss.str());
         }
+        condition_ = filter->encode();
     }
     return Status::OK();
 }
diff --git a/src/validator/test/MutateValidatorTest.cpp b/src/validator/test/MutateValidatorTest.cpp
index 35737387..1e2e4df3 100644
--- a/src/validator/test/MutateValidatorTest.cpp
+++ b/src/validator/test/MutateValidatorTest.cpp
@@ -164,7 +164,7 @@ TEST_F(MutateValidatorTest, UpdateVertexTest) {
     {
         auto cmd = "UPDATE VERTEX ON person \"Tom\""
                    "SET age = age + 1 "
-                   "WHEN page == 18 "
+                   "WHEN age == 18 "
                    "YIELD name AS name, age AS age";
         ASSERT_TRUE(checkResult(cmd, {PK::kUpdateVertex, PK::kStart}));
     }
diff --git a/tests/tck/features/update/Update.feature b/tests/tck/features/update/Update.feature
index 781e66bc..c1dbc727 100644
--- a/tests/tck/features/update/Update.feature
+++ b/tests/tck/features/update/Update.feature
@@ -226,6 +226,54 @@ Feature: Update string vid of vertex and edge
       YIELD $^.course.name AS Name, $^.course.credits AS Credits, $$.building.name
       """
     Then a SemanticError should be raised at runtime: Has wrong expr in `($$.course.credits+1)'
+    When executing query:
+      """
+      UPDATE VERTEX "101"
+      SET course.credits = 1
+      WHEN 123
+      YIELD $^.course.name AS Name, $^.course.credits AS Credits
+      """
+    Then a SemanticError should be raised at runtime: `123', expected Boolean, but was `INT'
+    When executing query:
+      """
+      UPDATE VERTEX "101"
+      SET course.credits = 1
+      WHEN credits
+      YIELD $^.course.name AS Name, $^.course.credits AS Credits
+      """
+    Then a SemanticError should be raised at runtime: `$^.course.credits', expected Boolean, but was `INT'
+    When executing query:
+      """
+      UPSERT VERTEX "101"
+      SET course.credits = 1
+      WHEN credits
+      YIELD $^.course.name AS Name, $^.course.credits AS Credits
+      """
+    Then a SemanticError should be raised at runtime: `$^.course.credits', expected Boolean, but was `INT'
+    When executing query:
+      """
+      UPSERT VERTEX "101"
+      SET course.credits = 1
+      WHEN "xyz"
+      YIELD $^.course.name AS Name, $^.course.credits AS Credits
+      """
+    Then a SemanticError should be raised at runtime: `xyz', expected Boolean, but was `STRING'
+    When executing query:
+      """
+      UPDATE VERTEX ON course "101"
+      SET credits = 1
+      WHEN credits
+      YIELD $^.course.name AS Name, $^.course.credits AS Credits
+      """
+    Then a SemanticError should be raised at runtime: `$^.course.credits', expected Boolean, but was `INT'
+    When executing query:
+      """
+      UPSERT VERTEX ON course "101"
+      SET credits = 1
+      WHEN "xyz"
+      YIELD $^.course.name AS Name, $^.course.credits AS Credits
+      """
+    Then a SemanticError should be raised at runtime: `xyz', expected Boolean, but was `STRING'
     # make sure TagName and PropertyName must exist in all clauses
     When executing query:
       """
@@ -986,7 +1034,7 @@ Feature: Update string vid of vertex and edge
       WHEN grade > 4 AND age > 15
       YIELD grade AS Grade, year AS Year
       """
-    Then a ExecutionError should be raised at runtime: Storage Error: Edge prop not found
+    Then a SemanticError should be raised at runtime: `select.age', not found the property `age'.
     # make sure the edge(src, ranking, dst) must not exist
     When executing query:
       """
-- 
GitLab