Implement delete a vertex (#868)

* implement delete a vertex

* address the comments of wadeliuyi and dangleptr

* address dangleptr's comments

* address comments of dangleptr and critical27

* address darionyaphet's comments

src/executor/CMakeLists.txt

@@ -45,6 +45,7 @@ add_library(
     SetExecutor.cpp
     FindExecutor.cpp
     MatchExecutor.cpp
+    DeleteVertexExecutor.cpp
 )
 add_dependencies(
     graph_obj

src/executor/DeleteVertexExecutor.cpp

+/* Copyright (c) 2019 vesoft inc. All rights reserved.
+ *
+ * This source code is licensed under Apache 2.0 License,
+ * attached with Common Clause Condition 1.0, found in the LICENSES directory.
+ */
+
+#include "graph/DeleteVertexExecutor.h"
+#include "storage/client/StorageClient.h"
+
+namespace nebula {
+namespace graph {
+
+DeleteVertexExecutor::DeleteVertexExecutor(Sentence *sentence,
+                                           ExecutionContext *ectx) : Executor(ectx) {
+    sentence_ = static_cast<DeleteVertexSentence*>(sentence);
+}
+
+Status DeleteVertexExecutor::prepare() {
+    auto ovalue = sentence_->vid()->eval();
+    auto v = ovalue.value();
+    if (!Expression::isInt(v)) {
+        return Status::Error("Vertex ID should be of type integer");
+    }
+    vid_ = Expression::asInt(v);
+    return Status::OK();
+}
+
+void DeleteVertexExecutor::execute() {
+    GraphSpaceID space = ectx()->rctx()->session()->space();
+    // TODO(zlcook) Get edgeKes of a vertex by Go
+    auto future = ectx()->storage()->getEdgeKeys(space, vid_);
+    auto *runner = ectx()->rctx()->runner();
+    auto cb = [this] (auto &&resp) {
+        if (!resp.ok()) {
+            DCHECK(onError_);
+            onError_(Status::Error("Internal Error"));
+            return;
+        }
+        auto rpcResp = std::move(resp).value();
+        std::vector<storage::cpp2::EdgeKey> allEdges;
+        for (auto& edge : *rpcResp.get_edge_keys()) {
+            auto reverseEdge = storage::cpp2::EdgeKey(apache::thrift::FragileConstructor::FRAGILE,
+                                                      edge.get_dst(),
+                                                      -(edge.get_edge_type()),
+                                                      edge.get_ranking(),
+                                                      edge.get_src());
+            allEdges.emplace_back(std::move(edge));
+            allEdges.emplace_back(std::move(reverseEdge));
+        }
+        deleteEdges(&allEdges);
+        return;
+    };
+
+    auto error = [this] (auto &&e) {
+        LOG(ERROR) << "Exception caught: " << e.what();
+        DCHECK(onError_);
+        onError_(Status::Error("Internal error"));
+        return;
+    };
+    std::move(future).via(runner).thenValue(cb).thenError(error);
+}
+
+void DeleteVertexExecutor::deleteEdges(std::vector<storage::cpp2::EdgeKey>* edges) {
+    GraphSpaceID space = ectx()->rctx()->session()->space();
+    auto future = ectx()->storage()->deleteEdges(space, *edges);
+    auto *runner = ectx()->rctx()->runner();
+    auto cb = [this] (auto &&resp) {
+        auto completeness = resp.completeness();
+        if (completeness != 100) {
+            DCHECK(onError_);
+            onError_(Status::Error("Internal Error"));
+            return;
+        }
+        deleteVertex();
+        return;
+    };
+
+    auto error = [this] (auto &&e) {
+        LOG(ERROR) << "Exception caught: " << e.what();
+        DCHECK(onError_);
+        onError_(Status::Error("Internal error"));
+        return;
+    };
+    std::move(future).via(runner).thenValue(cb).thenError(error);
+}
+
+void DeleteVertexExecutor::deleteVertex() {
+    GraphSpaceID space = ectx()->rctx()->session()->space();
+    auto future = ectx()->storage()->deleteVertex(space, vid_);
+    auto *runner = ectx()->rctx()->runner();
+    auto cb = [this] (auto &&resp) {
+        if (!resp.ok()) {
+            DCHECK(onError_);
+            onError_(Status::Error("Internal Error"));
+            return;
+        }
+        DCHECK(onFinish_);
+        onFinish_();
+        return;
+    };
+
+    auto error = [this] (auto &&e) {
+        LOG(ERROR) << "Exception caught: " << e.what();
+        DCHECK(onError_);
+        onError_(Status::Error("Internal error"));
+        return;
+    };
+    std::move(future).via(runner).thenValue(cb).thenError(error);
+}
+
+}   // namespace graph
+}   // namespace nebula
+

src/executor/DeleteVertexExecutor.h

+/* Copyright (c) 2019 vesoft inc. All rights reserved.
+ *
+ * This source code is licensed under Apache 2.0 License,
+ * attached with Common Clause Condition 1.0, found in the LICENSES directory.
+ */
+
+#ifndef GRAPH_DELETEVERTEXEXECUTOR_H_
+#define GRAPH_DELETEVERTEXEEXECUTOR_H_
+
+#include "base/Base.h"
+#include "graph/Executor.h"
+
+namespace nebula {
+namespace graph {
+
+class DeleteVertexExecutor final : public Executor {
+public:
+    DeleteVertexExecutor(Sentence *sentence, ExecutionContext *ectx);
+
+    const char* name() const override {
+        return "DeleteVertexExecutor";
+    }
+
+    Status MUST_USE_RESULT prepare() override;
+
+    void execute() override;
+
+private:
+    void deleteEdges(std::vector<storage::cpp2::EdgeKey>* edges);
+    void deleteVertex();
+
+private:
+    DeleteVertexSentence       *sentence_{nullptr};
+    VertexID                   vid_;
+};
+
+}   // namespace graph
+}   // namespace nebula
+
+#endif  // GRAPH_DELETEVERTEXEXECUTOR_H_

src/executor/Executor.cpp

@@ -42,6 +42,7 @@
 #include "graph/FindExecutor.h"
 #include "graph/MatchExecutor.h"
 #include "graph/BalanceExecutor.h"
+#include "graph/DeleteVertexExecutor.h"
 
 namespace nebula {
 namespace graph {
@@ -143,6 +144,9 @@ std::unique_ptr<Executor> Executor::makeExecutor(Sentence *sentence) {
         case Sentence::Kind::kBalance:
             executor = std::make_unique<BalanceExecutor>(sentence, ectx());
             break;
+        case Sentence::Kind::kDeleteVertex:
+            executor = std::make_unique<DeleteVertexExecutor>(sentence, ectx());
+            break;
         case Sentence::Kind::kUnknown:
             LOG(FATAL) << "Sentence kind unknown";
             break;

src/executor/test/CMakeLists.txt

@@ -204,3 +204,21 @@ nebula_add_test(
         wangle
         gtest
 )
+
+nebula_add_test(
+    NAME
+        deleteVertex_test
+    SOURCES
+        DeleteVertexTest.cpp
+    OBJECTS
+        $<TARGET_OBJECTS:graph_test_common_obj>
+        $<TARGET_OBJECTS:client_cpp_obj>
+        $<TARGET_OBJECTS:adHocSchema_obj>
+        $<TARGET_OBJECTS:http_client_obj>
+        ${GRAPH_TEST_LIBS}
+    LIBRARIES
+        ${THRIFT_LIBRARIES}
+        ${ROCKSDB_LIBRARIES}
+        wangle
+        gtest
+)

src/executor/test/DeleteVertexTest.cpp

+/* Copyright (c) 2019 vesoft inc. All rights reserved.
+ *
+ * This source code is licensed under Apache 2.0 License,
+ * attached with Common Clause Condition 1.0, found in the LICENSES directory.
+ */
+
+#include "base/Base.h"
+#include "graph/test/TestEnv.h"
+#include "graph/test/TestBase.h"
+#include "graph/test/TraverseTestBase.h"
+#include "meta/test/TestUtils.h"
+
+
+namespace nebula {
+namespace graph {
+
+class DeleteVertexTest : public TraverseTestBase {
+ protected:
+  void SetUp() override {
+    TraverseTestBase::SetUp();
+  }
+
+  void TearDown() override {
+    TraverseTestBase::TearDown();
+  }
+};
+
+TEST_F(DeleteVertexTest, base) {
+    // Check
+    {
+        cpp2::ExecutionResponse resp;
+        auto *fmt = "GO FROM %ld OVER like";
+        auto query = folly::stringPrintf(fmt, players_["Boris Diaw"].vid());
+        auto code = client_->execute(query, resp);
+        ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
+        std::vector<std::tuple<int64_t>> expected = {
+            {players_["Tony Parker"].vid()},
+            {players_["Tim Duncan"].vid()},
+        };
+        ASSERT_TRUE(verifyResult(resp, expected));
+    }
+    {
+        cpp2::ExecutionResponse resp;
+        auto &player = players_["Tony Parker"];
+        auto *fmt = "FETCH PROP ON player %ld YIELD player.name, player.age";
+        auto query = folly::stringPrintf(fmt, player.vid());
+        auto code = client_->execute(query, resp);
+        ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
+        std::vector<std::tuple<std::string, int64_t>> expected = {
+            {player.name(), player.age()},
+        };
+        ASSERT_TRUE(verifyResult(resp, expected));
+    }
+    {
+        cpp2::ExecutionResponse resp;
+        auto &player = players_["Tony Parker"];
+        auto &serve = player.serves()[0];
+        auto &team = teams_[std::get<0>(serve)];
+        auto *fmt = "FETCH PROP ON serve %ld->%ld"
+                    " YIELD serve.start_year, serve.end_year";
+        auto query = folly::stringPrintf(fmt, player.vid(), team.vid());
+        auto code = client_->execute(query, resp);
+        ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
+        std::vector<std::tuple<int64_t, int64_t>> expected = {
+            {std::get<1>(serve), std::get<2>(serve)},
+        };
+        ASSERT_TRUE(verifyResult(resp, expected));
+    }
+    // Delete vertex
+    {
+        cpp2::ExecutionResponse resp;
+        auto *fmt = "DELETE VERTEX %ld";
+        auto query = folly::stringPrintf(fmt, players_["Tony Parker"].vid());
+        auto code = client_->execute(query, resp);
+        ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
+    }
+    // Check again
+    {
+        cpp2::ExecutionResponse resp;
+        auto *fmt = "GO FROM %ld OVER like";
+        auto query = folly::stringPrintf(fmt, players_["Boris Diaw"].vid());
+        auto code = client_->execute(query, resp);
+        ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
+        std::vector<std::tuple<int64_t>> expected = {
+            {players_["Tim Duncan"].vid()},
+        };
+        ASSERT_TRUE(verifyResult(resp, expected));
+    }
+    {
+        cpp2::ExecutionResponse resp;
+        auto &player = players_["Tony Parker"];
+        auto *fmt = "FETCH PROP ON player %ld YIELD player.name, player.age";
+        auto query = folly::stringPrintf(fmt, player.vid());
+        auto code = client_->execute(query, resp);
+        ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
+        std::vector<std::tuple<std::string, int64_t>> expected = {
+        };
+        ASSERT_TRUE(verifyResult(resp, expected));
+    }
+    {
+        cpp2::ExecutionResponse resp;
+        auto &player = players_["Tony Parker"];
+        auto &serve = player.serves()[0];
+        auto &team = teams_[std::get<0>(serve)];
+        auto *fmt = "FETCH PROP ON serve %ld->%ld"
+                    " YIELD serve.start_year, serve.end_year";
+        auto query = folly::stringPrintf(fmt, player.vid(), team.vid());
+        auto code = client_->execute(query, resp);
+        ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
+        std::vector<std::tuple<int64_t, int64_t>> expected = {
+        };
+        ASSERT_TRUE(verifyResult(resp, expected));
+    }
+}
+
+}  // namespace graph
+}  // namespace nebula
+

src/parser/MutateSentences.cpp

@@ -230,11 +230,7 @@ std::string DeleteVertexSentence::toString() const {
     std::string buf;
     buf.reserve(256);
     buf += "DELETE VERTEX ";
-    buf += vidList_->toString();
-    if (whereClause_ != nullptr) {
-        buf += " ";
-        buf += whereClause_->toString();
-    }
+    buf += vid_->toString();
     return buf;
 }
 

src/parser/MutateSentences.h

@@ -405,28 +405,19 @@ private:
 
 class DeleteVertexSentence final : public Sentence {
 public:
-    explicit DeleteVertexSentence(VertexIDList *vidList) {
-        vidList_.reset(vidList);
+    explicit DeleteVertexSentence(Expression *vid) {
+        vid_.reset(vid);
         kind_ = Kind::kDeleteVertex;
     }
 
-    auto vidList() const {
-        return vidList_->vidList();
-    }
-
-    void setWhereClause(WhereClause *clause) {
-        whereClause_.reset(clause);
-    }
-
-    const WhereClause* whereClause() const {
-        return whereClause_.get();
+    Expression* vid() const {
+        return vid_.get();
     }
 
     std::string toString() const override;
 
 private:
-    std::unique_ptr<VertexIDList>               vidList_;
-    std::unique_ptr<WhereClause>                whereClause_;
+    std::unique_ptr<Expression>                  vid_;
 };
 
 

src/parser/parser.yy

@@ -1155,9 +1155,8 @@ update_edge_sentence
     ;
 
 delete_vertex_sentence
-    : KW_DELETE KW_VERTEX vid_list where_clause {
+    : KW_DELETE KW_VERTEX vid {
         auto sentence = new DeleteVertexSentence($3);
-        sentence->setWhereClause($4);
         $$ = sentence;
     }
     ;

src/parser/test/ParserTest.cpp

@@ -662,19 +662,7 @@ TEST(Parser, DeleteVertex) {
     }
     {
         GQLParser parser;
-        std::string query = "DELETE VERTEX 123,456,789";
-        auto result = parser.parse(query);
-        ASSERT_TRUE(result.ok()) << result.status();
-    }
-    {
-        GQLParser parser;
-        std::string query = "DELETE VERTEX 12345 WHERE salary > 10000";
-        auto result = parser.parse(query);
-        ASSERT_TRUE(result.ok()) << result.status();
-    }
-    {
-        GQLParser parser;
-        std::string query = "DELETE VERTEX 123,456,789 WHERE salary > 10000";
+        std::string query = "DELETE VERTEX hash(\"zhangsan\")";
         auto result = parser.parse(query);
         ASSERT_TRUE(result.ok()) << result.status();
     }