[urgent] Support delete vertices #1317

darionyaphet · 2019-11-22T08:42:47Z

DELETE VERTEX vid_list

zhangguoqing · 2019-11-27T02:28:30Z

I suggest using a flag to indicate if deleting a vertex is allowed to delete all its associated edges or only delete one vertex without any associated edges.

darionyaphet · 2019-11-27T07:06:26Z

I suggest using a flag to indicate if deleting a vertex is allowed to delete all its associated edges or only delete one vertex without any associated edges.

If the vertex does not exist, is the edge meaningful?

wadeliuyi · 2019-11-27T15:19:15Z

src/graph/DeleteVerticesExecutor.cpp

@@ -70,7 +86,7 @@ void DeleteVertexExecutor::execute() {
    std::move(future).via(runner).thenValue(cb).thenError(error);
 }

-void DeleteVertexExecutor::deleteEdges(std::vector<storage::cpp2::EdgeKey>* edges) {
+void DeleteVerticesExecutor::deleteEdges(std::vector<storage::cpp2::EdgeKey>* edges) {


why define the parameter is a pointer?

src/graph/DeleteVerticesExecutor.cpp

nebula-community-bot · 2019-11-28T04:42:37Z

Unit testing failed.

nebula-community-bot · 2019-11-28T06:33:06Z

Unit testing passed.

laura-ding · 2019-12-02T06:47:09Z

src/parser/Clauses.h

+    std::vector<nebula::OptVariantType> eval() const {
+        std::vector<nebula::OptVariantType> vertices;
+        for (auto& vertex : vidList_) {
+            auto vid = vertex->eval();


when vid is function expression, please do vertex->prepare() before vid->eval()

Good Catch 👍

nebula-community-bot · 2019-12-02T10:35:56Z

Unit testing passed.

Conflicts: src/graph/CMakeLists.txt src/graph/Executor.cpp src/storage/QueryEdgeKeysProcessor.cpp

nebula-community-bot · 2019-12-11T11:27:20Z

Unit testing passed.

zhangguoqing · 2019-12-27T12:55:54Z

I suggest using a flag to indicate if deleting a vertex is allowed to delete all its associated edges or only delete one vertex without any associated edges.

If the vertex does not exist, is the edge meaningful?

FYI. 我的意思是添加一个bool类型的配置项，由用户决定删除一个顶点的方式：如果为true时，则可以删除此顶点以及与它关联的所有边；如果为false时，则只允许删除没有任何边的顶点，否则就应该向用户发出警告。

Conflicts: src/graph/DeleteVertexExecutor.cpp src/storage/CMakeLists.txt src/storage/StorageServiceHandler.cpp src/storage/mutate/DeleteVertexProcessor.cpp src/storage/mutate/DeleteVertexProcessor.h src/storage/test/DeleteVerticesTest.cpp

darionyaphet · 2019-12-28T13:48:27Z

I suggest using a flag to indicate if deleting a vertex is allowed to delete all its associated edges or only delete one vertex without any associated edges.

If the vertex does not exist, is the edge meaningful?

FYI. 我的意思是添加一个bool类型的配置项，由用户决定删除一个顶点的方式：如果为true时，则可以删除此顶点以及与它关联的所有边；如果为false时，则只允许删除没有任何边的顶点，否则就应该向用户发出警告。

Sounds reasonable. Maybe we should implement it in another one. thanks

nebula-community-bot · 2019-12-28T14:10:44Z

Unit testing passed.

Conflicts: src/storage/BaseProcessor.h

src/graph/DeleteVerticesExecutor.cpp

src/storage/query/QueryEdgeKeysProcessor.cpp

Conflicts: src/storage/test/CMakeLists.txt

src/storage/mutate/DeleteVerticesProcessor.cpp

src/graph/DeleteEdgesExecutor.cpp

src/storage/mutate/DeleteVerticesProcessor.cpp

dangleptr · 2020-02-13T05:35:57Z

src/storage/mutate/DeleteVerticesProcessor.cpp

+
+    if (indexes_.empty()) {
+        std::for_each(partVertices.begin(), partVertices.end(), [&](auto& pv) {
+            callingNum_ += pv.second.size();


callingNum_ is on part Level, it means one part one call.

Because we process for each Partition and vertex. So this is the vertex's number.

callingNum_ means how many calls to kvstore.

dangleptr · 2020-02-13T05:36:55Z

src/graph/DeleteEdgesExecutor.cpp

        return;
    }

-    status = setupEdgeKeys();
+    auto space = ectx()->rctx()->session()->space();


What's the purpose about the changes in DeleteEdgesExecutor.cpp . I can't see any difference

We shouldn't check it in prepare step, for example; Use space; delete ...

why? Is there any difference? You could move checkIfGraphSpaceChosen into prepare.

For example:

(user@127.0.0.1:13699) [t]> INSERT VERTEX player(name, age) VALUES 100:("Tim Duncan", 42) Execution succeeded (Time spent: 2.233/3.439 ms) Fri Feb 14 19:45:25 2020 (user@127.0.0.1:13699) [t]> FETCH PROP ON player 100; ======================================= | VertexID | player.name | player.age | ======================================= | 100 | Tim Duncan | 42 | --------------------------------------- Got 1 rows (Time spent: 5.353/6.825 ms) Fri Feb 14 19:45:27 2020 (user@127.0.0.1:13699) [t]> use test Execution succeeded (Time spent: 1.81/3.018 ms) Fri Feb 14 19:45:32 2020 (user@127.0.0.1:13699) [test]> use t ; DELETE VERTEX 100; Execution succeeded (Time spent: 3.388/4.067 ms) Fri Feb 14 19:45:37 2020 (user@127.0.0.1:13699) [t]> FETCH PROP ON player 100; ======================================= | VertexID | player.name | player.age | ======================================= | 100 | Tim Duncan | 42 | --------------------------------------- Got 1 rows (Time spent: 3.496/4.686 ms) Fri Feb 14 19:45:40 2020

dangleptr · 2020-02-14T03:14:02Z

src/graph/DeleteEdgesExecutor.cpp

        return;
    }

-    status = setupEdgeKeys();
+    auto space = ectx()->rctx()->session()->space();


why? Is there any difference? You could move checkIfGraphSpaceChosen into prepare.

src/graph/DeleteVerticesExecutor.cpp

src/graph/test/DeleteVerticesTest.cpp

src/storage/mutate/DeleteVerticesProcessor.cpp

dangleptr · 2020-02-14T03:19:57Z

src/storage/mutate/DeleteVerticesProcessor.cpp

+
+    if (indexes_.empty()) {
+        std::for_each(partVertices.begin(), partVertices.end(), [&](auto& pv) {
+            callingNum_ += pv.second.size();


callingNum_ means how many calls to kvstore.

dangleptr · 2020-02-14T03:26:45Z

src/interface/storage.thrift

@@ -218,16 +213,19 @@ struct AddEdgesRequest {
    3: bool overwritable,
 }

-struct EdgeKeyRequest {
+struct EdgeKeysRequest {


Could we use get neighbors directly. Actually, "getEdgeKeys" will be deleted in the future. It is meaningless to change it again.

dutor · 2020-02-19T15:56:32Z

Please resolve the conflicts.

dangleptr · 2020-02-26T10:54:02Z

Please rebase the master

Conflicts: src/storage/test/CMakeLists.txt

* delete vertices * address laura-ding's comment Co-authored-by: dutor <440396+dutor@users.noreply.github.com>

darionyaphet added 2 commits November 21, 2019 16:58

delete vertices

bbec6b1

Merge branch 'master' into delete_vertices

a352a7e

darionyaphet requested review from dutor, CPWstatic, critical27, dangleptr, whitewum, jude-zhu, liuyu85cn and bright-starry-sky November 22, 2019 08:42

wadeliuyi reviewed Nov 27, 2019

View reviewed changes

src/graph/DeleteVerticesExecutor.cpp Outdated Show resolved Hide resolved

wadeliuyi reviewed Nov 27, 2019

View reviewed changes

src/graph/DeleteVerticesExecutor.cpp Outdated Show resolved Hide resolved

darionyaphet added the ready-for-testing PR: ready for the CI test label Nov 28, 2019

Merge branch 'master' into delete_vertices

30aacd6

Merge branch 'master' into delete_vertices

f11a549

laura-ding reviewed Dec 2, 2019

View reviewed changes

Merge branch 'master' into delete_vertices

4a8d1a9

Merge branch 'master' into delete_vertices

99bbfa3

Conflicts: src/graph/CMakeLists.txt src/graph/Executor.cpp src/storage/QueryEdgeKeysProcessor.cpp

Merge branch 'master' into delete_vertices

b7756f2

Conflicts: src/graph/DeleteVertexExecutor.cpp src/storage/CMakeLists.txt src/storage/StorageServiceHandler.cpp src/storage/mutate/DeleteVertexProcessor.cpp src/storage/mutate/DeleteVertexProcessor.h src/storage/test/DeleteVerticesTest.cpp

jude-zhu requested a review from laura-ding December 30, 2019 08:53

darionyaphet added 2 commits February 11, 2020 11:29

Merge branch 'master' into delete_vertices

2b1b0e6

Conflicts: src/storage/BaseProcessor.h

Merge branch 'master' into delete_vertices

5f86308

laura-ding reviewed Feb 12, 2020

View reviewed changes

src/graph/DeleteVerticesExecutor.cpp Show resolved Hide resolved

src/storage/query/QueryEdgeKeysProcessor.cpp Show resolved Hide resolved

amber-moe self-assigned this Feb 12, 2020

address laura-ding's comment

9847c5b

amber-moe mentioned this pull request Feb 12, 2020

[doc]update delete vertices #1759

Merged

Merge branch 'master' into delete_vertices

c352d71

Conflicts: src/storage/test/CMakeLists.txt

laura-ding previously approved these changes Feb 12, 2020

View reviewed changes

src/storage/mutate/DeleteVerticesProcessor.cpp Outdated Show resolved Hide resolved

CPWstatic previously approved these changes Feb 12, 2020

View reviewed changes

src/storage/mutate/DeleteVerticesProcessor.cpp Show resolved Hide resolved

Merge branch 'master' into delete_vertices

8aaa104

bright-starry-sky reviewed Feb 13, 2020

View reviewed changes

src/storage/mutate/DeleteVerticesProcessor.cpp Show resolved Hide resolved

bright-starry-sky reviewed Feb 13, 2020

View reviewed changes

src/graph/DeleteEdgesExecutor.cpp Show resolved Hide resolved

dangleptr reviewed Feb 13, 2020

View reviewed changes

jude-zhu requested a review from dangleptr February 13, 2020 11:43

dangleptr reviewed Feb 14, 2020

View reviewed changes

jude-zhu requested review from CPWstatic and laura-ding February 27, 2020 01:42

Merge branch 'master' into delete_vertices

a62a7ac

Conflicts: src/storage/test/CMakeLists.txt

darionyaphet dismissed stale reviews from CPWstatic and laura-ding via a62a7ac February 27, 2020 10:29

jude-zhu requested a review from dangleptr February 28, 2020 06:24

laura-ding approved these changes Feb 28, 2020

View reviewed changes

CPWstatic approved these changes Feb 29, 2020

View reviewed changes

Merge branch 'master' into delete_vertices

9064c52

dutor merged commit f048b76 into vesoft-inc:master Mar 3, 2020

darionyaphet deleted the delete_vertices branch March 3, 2020 02:35

jude-zhu pushed a commit to jude-zhu/nebula that referenced this pull request Mar 4, 2020

Support delete vertices (vesoft-inc#1317)

d187735

* delete vertices * address laura-ding's comment Co-authored-by: dutor <440396+dutor@users.noreply.github.com>

tong-hao pushed a commit to tong-hao/nebula that referenced this pull request Jun 1, 2021

Support delete vertices (vesoft-inc#1317)

a98bc1e

* delete vertices * address laura-ding's comment Co-authored-by: dutor <440396+dutor@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[urgent] Support delete vertices #1317

[urgent] Support delete vertices #1317

darionyaphet commented Nov 22, 2019

zhangguoqing commented Nov 27, 2019

darionyaphet commented Nov 27, 2019

wadeliuyi Nov 27, 2019

nebula-community-bot commented Nov 28, 2019

nebula-community-bot commented Nov 28, 2019

laura-ding Dec 2, 2019

darionyaphet Dec 2, 2019

nebula-community-bot commented Dec 2, 2019

nebula-community-bot commented Dec 11, 2019

zhangguoqing commented Dec 27, 2019

darionyaphet commented Dec 28, 2019

nebula-community-bot commented Dec 28, 2019

dangleptr Feb 13, 2020

darionyaphet Feb 13, 2020

dangleptr Feb 14, 2020

dangleptr Feb 13, 2020

darionyaphet Feb 13, 2020

dangleptr Feb 14, 2020

darionyaphet Feb 14, 2020

dangleptr Feb 14, 2020

dangleptr Feb 14, 2020

dangleptr Feb 14, 2020

dutor commented Feb 19, 2020

dangleptr commented Feb 26, 2020

[urgent] Support delete vertices #1317

[urgent] Support delete vertices #1317

Conversation

darionyaphet commented Nov 22, 2019

zhangguoqing commented Nov 27, 2019

darionyaphet commented Nov 27, 2019

Choose a reason for hiding this comment

nebula-community-bot commented Nov 28, 2019

nebula-community-bot commented Nov 28, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nebula-community-bot commented Dec 2, 2019

nebula-community-bot commented Dec 11, 2019

zhangguoqing commented Dec 27, 2019

darionyaphet commented Dec 28, 2019

nebula-community-bot commented Dec 28, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dutor commented Feb 19, 2020

dangleptr commented Feb 26, 2020