Catch exception in set op (#2362)

* Catch exception when handle subquery. * Add test. * Handle some error. * Fix asyc call. Co-authored-by: Yee <[email protected]>
vesoft-inc · Dec 2, 2020 · 53f56b6 · 53f56b6
1 parent 3434739
commit 53f56b6
Show file tree

Hide file tree

Showing 4 changed files with 110 additions and 15 deletions.
diff --git a/src/graph/SetExecutor.cpp b/src/graph/SetExecutor.cpp
@@ -114,18 +114,43 @@ void SetExecutor::execute() {
     }
 
     auto *runner = ectx()->rctx()->runner();
-    runner->add([this] () mutable { left_->execute(); });
-    runner->add([this] () mutable { right_->execute(); });
 
-    auto cb = [this] (auto &&result) {
-        UNUSED(result);
-        if (!leftS_.ok() || !rightS_.ok()) {
+    auto lTask = [this] () { left_->execute(); };
+    auto lError = [this](auto &&e) {
+        UNUSED(e);
+        std::stringstream ss;
+        ss << "Exception when handle left subquery: " << sentence_->left()->toString();
+        LOG(ERROR) << ss.str();
+        leftS_ = Status::Error(ss.str());
+        leftP_.setValue();
+    };
+    folly::makeFuture().via(runner).then(lTask).thenError(lError);
+
+    auto rTask = [this]() { right_->execute(); };
+    auto rError = [this](auto &&e) {
+        UNUSED(e);
+        std::stringstream ss;
+        ss << "Exception when handle right subquery: " << sentence_->right()->toString();
+        LOG(ERROR) << ss.str();
+        rightS_ = Status::Error(ss.str());
+        rightP_.setValue();
+    };
+    folly::makeFuture().via(runner).then(rTask).thenError(rError);
+
+    auto cb = [this] () {
+        if (!leftS_.ok()) {
             std::string msg;
-            msg += "lhs has error: ";
+            msg += "left subquery has error: ";
             msg += leftS_.toString();
-            msg += " rhs has error: ";
+            doError(Status::Error(std::move(msg)));
+            return;
+        }
+
+        if (!rightS_.ok()) {
+            std::string msg;
+            msg += "right subquery has error: ";
             msg += rightS_.toString();
-            doError(Status::Error(msg));
+            doError(Status::Error(std::move(msg)));
             return;
         }
 
@@ -163,7 +188,13 @@ void SetExecutor::execute() {
                 break;
         }
     };
-    folly::collectAll(futures_).via(runner).thenValue(cb);
+    auto error = [this](auto &&e) {
+        std::stringstream ss;
+        ss << "Exception when handle set operation: " << e.what();
+        LOG(ERROR) << ss.str();
+        doError(Status::Error(ss.str()));
+    };
+    folly::collectAll(futures_).via(runner).then(cb).thenError(error);
 }
 
 void SetExecutor::doUnion() {

diff --git a/src/graph/TraverseExecutor.cpp b/src/graph/TraverseExecutor.cpp
@@ -237,8 +237,12 @@ Status Collector::collectWithoutSchema(VariantType &var, RowWriter *writer) {
 OptVariantType Collector::getProp(const meta::SchemaProviderIf *schema,
                                   const std::string &prop,
                                   const RowReader *reader) {
-    DCHECK(reader != nullptr);
-    DCHECK(schema != nullptr);
+    if (schema == nullptr) {
+        return Status::Error("Schema is nullptr when getProp.");
+    }
+    if (reader == nullptr) {
+        return Status::Error("reader is nullptr when getProp.");
+    }
     using nebula::cpp2::SupportedType;
     auto type = schema->getFieldType(prop).type;
     switch (type) {
@@ -293,9 +297,15 @@ Status Collector::getSchema(const std::vector<VariantType> &vals,
                             const std::vector<std::string> &colNames,
                             const std::vector<nebula::cpp2::SupportedType> &colTypes,
                             SchemaWriter *outputSchema) {
-    DCHECK(outputSchema != nullptr);
-    DCHECK_EQ(vals.size(), colNames.size());
-    DCHECK_EQ(vals.size(), colTypes.size());
+    if (outputSchema == nullptr) {
+        return Status::Error("Schema is nullptr.");
+    }
+    if (vals.size() != colNames.size() || vals.size() != colTypes.size()) {
+        return Status::Error("column size not match, %ld vs. %ld vs. %ld",
+                             vals.size(),
+                             colNames.size(),
+                             colTypes.size());
+    }
     using nebula::cpp2::SupportedType;
     auto index = 0u;
     for (auto &it : colTypes) {

diff --git a/src/graph/YieldExecutor.cpp b/src/graph/YieldExecutor.cpp
@@ -332,7 +332,10 @@ Status YieldExecutor::getOutputSchema(const InterimResult *inputs,
         }
         return Status::OK();
     };
-    inputs->applyTo(visitor, 1);
+    auto status = inputs->applyTo(visitor, 1);
+    if (!status.ok()) {
+        return status;
+    }
 
     return Collector::getSchema(record, resultColNames_, colTypes_, outputSchema);
 }

diff --git a/src/graph/test/SetTest.cpp b/src/graph/test/SetTest.cpp
@@ -719,5 +719,56 @@ TEST_F(SetTest, ExecutionError) {
         ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code);
     }
 }
+
+TEST_F(SetTest, CatchExceptionInSetOp) {
+    {
+        cpp2::ExecutionResponse resp;
+        auto *fmt =
+            "$var1 = GO FROM %ld OVER like YIELD like._dst as dst;"
+            "$var2 = YIELD left($var1.dst, 4) + \"x\" UNION YIELD left($var1.dst, 4) + \"y\";"
+            "YIELD $var2.*;";
+        auto query = folly::stringPrintf(fmt, players_["Tim Duncan"].vid());
+        auto code = client_->execute(query, resp);
+        ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code);
+    }
+    {
+        cpp2::ExecutionResponse resp;
+        auto *fmt = "$var1 = GO FROM %ld OVER like YIELD like._dst as dst;"
+                    "$var2 = YIELD left((string)$var1.dst, 4) + \"x\" UNION YIELD left($var1.dst, "
+                    "4) + \"y\";"
+                    "YIELD $var2.*;";
+        auto query = folly::stringPrintf(fmt, players_["Tim Duncan"].vid());
+        auto code = client_->execute(query, resp);
+        ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code);
+    }
+    {
+        cpp2::ExecutionResponse resp;
+        auto *fmt = "$var1 = GO FROM %ld OVER like YIELD like._dst as dst;"
+                    "$var2 = YIELD left($var1.dst, 4) + \"x\" UNION YIELD left((string)$var1.dst, "
+                    "4) + \"y\";"
+                    "YIELD $var2.*;";
+        auto query = folly::stringPrintf(fmt, players_["Tim Duncan"].vid());
+        auto code = client_->execute(query, resp);
+        ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code);
+    }
+    {
+        cpp2::ExecutionResponse resp;
+        auto *fmt = "$var1 = GO FROM %ld OVER like YIELD like._dst as dst;"
+                    "$var2 = YIELD left((string)$var1.dst, 4) + \"x\" UNION YIELD "
+                    "left((string)$var1.dst, 4) + \"y\";"
+                    "YIELD $var2.*;";
+        auto query = folly::stringPrintf(fmt, players_["Tim Duncan"].vid());
+        auto code = client_->execute(query, resp);
+        ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
+
+        std::vector<std::tuple<std::string>> expected = {
+            {folly::to<std::string>(players_["Tony Parker"].vid()).substr(0, 4) + "x"},
+            {folly::to<std::string>(players_["Manu Ginobili"].vid()).substr(0, 4) + "x"},
+            {folly::to<std::string>(players_["Tony Parker"].vid()).substr(0, 4) + "y"},
+            {folly::to<std::string>(players_["Manu Ginobili"].vid()).substr(0, 4) + "y"},
+        };
+        ASSERT_TRUE(verifyResult(resp, expected));
+    }
+}
 }  // namespace graph
 }  // namespace nebula