Insert vertex only #3335

cangfengzhs · 2021-11-19T09:48:55Z

What type of PR is this?

bug
feature
enhancement

What does this PR do?

add insert vertex only statement
For example:

Insert vertex (1,2,3);

About parentheses in statement.at the beginning, I wanted to implement it in the following format, but there was ambiguity:

Insert vertex t();  // stmt1
Insert vertex t() Values 1:(); // stmt2

In stmt1, t() represents a function, but in stmt2, t() represents a tag

Which issue(s)/PR(s) this PR relates to?

part of issue: #3123
need PR: #3328

Special notes for your reviewer, ex. impact of this fix, etc:

Additional context:

Checklist：

Documentation affected （Please add the label if documentation needs to be modified.)
Incompatible （If it is incompatible, please describe it and add corresponding label.）
Need to cherry-pick （If need to cherry-pick to some branches, please label the destination version(s).)
Performance impacted: Consumes more CPU/Memory

Release notes：

Please confirm whether to reflect in release notes and how to describe:

Shylock-Hg · 2021-11-23T09:53:36Z

How about insert vertex values 1:(), 2:() ...?

cangfengzhs · 2021-11-24T02:15:32Z

How about insert vertex values 1:(), 2:() ...?

This problem has been initially solved. insert vertex ifNotExists? VALUES vid_list syntax is used.

add insert vertex only validator fix grammar adjust insert vertex parser

Shylock-Hg · 2021-12-15T06:40:30Z

src/graph/validator/MutateValidator.h

@@ -38,6 +38,20 @@ class InsertVerticesValidator final : public Validator {
  std::vector<storage::cpp2::NewVertex> vertices_;
 };

+class InsertVerticesOnlyValidator final : public Validator {


Why don't reuse InsertVerticesValidator?

The syntax of InsertVerticesOnly and the syntax of InsertVertices are quite different in the parser process. This causes the data structure contained in the two Sentences to be different. If both Sentences use InsertVerticesValidator, two completely different toPlan logics are also required. So simply use a new InsertVerticesOnlyValidator. This also ensures that the impact on InsertVertices is minimal, and other bugs will not be introduced during the modification process.

nebula/src/parser/parser.yy

Line 2839 in df3781c

vertex_row_item

I think you just need add a new reduce rule like vid { $$ = new VertexRowIten($1, new ValueList())} in vertex_row_item rule.

But, InsertVertices can not use vertexID list as "1,2,3".It needs "1:(),2:(),3:()" instead. So if add a new reduce rule in vertex_row_item, the parse process of insertVertices will be very strange.

Shylock-Hg · 2021-12-15T06:40:55Z

src/parser/Sentence.h

@@ -52,6 +52,7 @@ class Sentence {
    kDropTag,
    kDropEdge,
    kInsertVertices,
+    kInsertVerticesOnly,


codecov-commenter · 2021-12-15T07:38:41Z

Codecov Report

Merging #3335 (4750d17) into master (53b85dc) will increase coverage by 0.02%.
The diff coverage is 76.02%.

@@            Coverage Diff             @@
##           master    #3335      +/-   ##
==========================================
+ Coverage   85.19%   85.22%   +0.02%     
==========================================
  Files        1306     1307       +1     
  Lines      122158   122239      +81     
==========================================
+ Hits       104078   104182     +104     
+ Misses      18080    18057      -23

Impacted Files	Coverage Δ
src/common/expression/VariableExpression.cpp	`84.84% <ø> (-0.45%)`	⬇️
...rc/graph/executor/query/AppendVerticesExecutor.cpp	`98.30% <ø> (ø)`
src/graph/executor/query/GetEdgesExecutor.cpp	`94.00% <ø> (ø)`
src/graph/executor/query/GetVerticesExecutor.cpp	`96.77% <ø> (ø)`
src/graph/executor/query/IndexScanExecutor.cpp	`88.88% <ø> (ø)`
src/graph/optimizer/rule/IndexScanRule.h	`100.00% <ø> (ø)`
...mizer/rule/PushLimitDownEdgeIndexRangeScanRule.cpp	`20.00% <0.00%> (-0.69%)`	⬇️
...raph/optimizer/rule/PushLimitDownIndexScanRule.cpp	`14.81% <0.00%> (-0.57%)`	⬇️
...imizer/rule/PushLimitDownTagIndexRangeScanRule.cpp	`20.00% <0.00%> (-0.69%)`	⬇️
src/graph/planner/plan/Query.h	`97.22% <ø> (ø)`
... and 64 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9ea4889...4750d17. Read the comment docs.

Shylock-Hg · 2021-12-20T11:03:42Z

tests/tck/features/insert/insertVertexOnly.feature

+      INSERT EDGE e() VALUES 1->2:(),2->3:();
+      """
+    Then the execution should be successful
+    When executing query:


Add test case like fetch prop on * 1

wenhaocs

In previous conversations, there were more files. Just want to make sure this PR contains the files you want. Otherwise, good to go after fixing shylock's comment.

cangfengzhs · 2021-12-21T02:34:44Z

In previous conversations, there were more files. Just want to make sure this PR contains the files you want. Otherwise, good to go after fixing shylock's comment.

I adjust the implement according to shylock's suggestion and use another way which minimize the changes of previous code.

Shylock-Hg · 2021-12-21T06:13:22Z

src/clients/storage/StorageClient.cpp

@@ -218,11 +218,12 @@ StorageRpcRespFuture<cpp2::GetPropResponse> StorageClient::getProps(
    req.set_space_id(param.space);
    req.set_parts(std::move(c.second));
    req.set_dedup(dedup);
-    if (vertexProps != nullptr) {


Why change this?

wait fix in #3443

The same. Fixing something else?

@cangfengzhs Please check why vertexProp equals to null, maybe not fixed in #3443

FetchVerticesPlanner::buildVertexProps return a nullptr when props is empty

…to insert_vertex_only2

…sert_vertex_only2

cangfengzhs added 2 commits December 15, 2021 12:46

add insert vertex only parser

e786d0f

add insert vertex only validator fix grammar adjust insert vertex parser

add test

0f6de68

cangfengzhs force-pushed the insert_vertex_only branch from 6ff67a2 to 0f6de68 Compare December 15, 2021 06:26

cangfengzhs added the ready-for-testing PR: ready for the CI test label Dec 15, 2021

format

430fd0b

Shylock-Hg reviewed Dec 15, 2021

View reviewed changes

cangfengzhs marked this pull request as ready for review December 15, 2021 06:58

cangfengzhs requested review from yixinglu, CPWstatic, Shylock-Hg, critical27 and a team December 15, 2021 06:58

Merge remote-tracking branch 'vesoft/master' into insert_vertex_only

5f6c4d1

Sophie-Xie requested review from wenhaocs and removed request for a team December 20, 2021 04:01

cangfengzhs added 2 commits December 20, 2021 12:22

merge master

9017391

Merge remote-tracking branch 'vesoft/master' into insert_vertex_only

d1ad1c3

cangfengzhs force-pushed the insert_vertex_only branch from 14a35f7 to d1ad1c3 Compare December 20, 2021 04:30

modify parser

8c4cf94

Shylock-Hg reviewed Dec 20, 2021

View reviewed changes

wenhaocs reviewed Dec 20, 2021

View reviewed changes

Shylock-Hg added the doc affected PR: improvements or additions to documentation label Dec 21, 2021

cangfengzhs added 2 commits December 21, 2021 14:08

add fetch test

545bf34

Merge branch 'master' into insert_vertex_only

c60463b

Shylock-Hg reviewed Dec 21, 2021

View reviewed changes

cangfengzhs added 2 commits December 21, 2021 14:25

format

4546ea7

Merge branch 'insert_vertex_only' of github.com:cangfengzhs/nebula in…

4750d17

…to insert_vertex_only2

cangfengzhs requested review from Shylock-Hg and wenhaocs December 21, 2021 08:29

Sophie-Xie removed request for yixinglu, CPWstatic and critical27 December 23, 2021 01:30

Sophie-Xie added the ready for review label Dec 23, 2021

cangfengzhs added 3 commits December 24, 2021 12:00

Merge branch 'master' of https://github.com/vesoft-inc/nebula into in…

f973567

…sert_vertex_only2

Merge remote-tracking branch 'vesoft/master' into insert_vertex_only2

6874699

fix fetch vertex without tag error

9209622

Shylock-Hg approved these changes Dec 27, 2021

View reviewed changes

cangfengzhs requested a review from liuyu85cn December 27, 2021 06:38

Merge branch 'master' into insert_vertex_only

aa2350b

Shylock-Hg requested a review from nevermore3 December 27, 2021 06:42

nevermore3 approved these changes Dec 27, 2021

View reviewed changes

Sophie-Xie added 2 commits December 27, 2021 15:36

Merge branch 'master' into insert_vertex_only

63519a6

Merge branch 'master' into insert_vertex_only

414faf1

yixinglu merged commit e6fd4a9 into vesoft-inc:master Dec 27, 2021

nevermore3 mentioned this pull request Dec 29, 2021

Empty tag endpoint returns an incorrect result #3340

Closed

jamieliu1023 mentioned this pull request Jan 1, 2022

Weekly Report 2021-12-31 vesoft-inc/nebula-community#83

Closed

cooper-lzy requested a review from foesa-yang January 5, 2022 03:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Insert vertex only #3335

Insert vertex only #3335

cangfengzhs commented Nov 19, 2021 •

edited by Shylock-Hg

Loading

Shylock-Hg commented Nov 23, 2021

cangfengzhs commented Nov 24, 2021

Shylock-Hg Dec 15, 2021

cangfengzhs Dec 15, 2021

Shylock-Hg Dec 15, 2021 •

edited

Loading

cangfengzhs Dec 15, 2021

Shylock-Hg Dec 15, 2021

codecov-commenter commented Dec 15, 2021 •

edited

Loading

Shylock-Hg Dec 20, 2021

wenhaocs left a comment •

edited

Loading

cangfengzhs commented Dec 21, 2021

Shylock-Hg Dec 21, 2021

Shylock-Hg Dec 21, 2021

wenhaocs Dec 21, 2021

Shylock-Hg Dec 23, 2021 •

edited

Loading

cangfengzhs Dec 27, 2021

Insert vertex only #3335

Insert vertex only #3335

Conversation

cangfengzhs commented Nov 19, 2021 • edited by Shylock-Hg Loading

What type of PR is this?

What does this PR do?

Which issue(s)/PR(s) this PR relates to?

Special notes for your reviewer, ex. impact of this fix, etc:

Additional context:

Checklist：

Release notes：

Shylock-Hg commented Nov 23, 2021

cangfengzhs commented Nov 24, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Shylock-Hg Dec 15, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Dec 15, 2021 • edited Loading

Codecov Report

Choose a reason for hiding this comment

wenhaocs left a comment • edited Loading

Choose a reason for hiding this comment

cangfengzhs commented Dec 21, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Shylock-Hg Dec 23, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cangfengzhs commented Nov 19, 2021 •

edited by Shylock-Hg

Loading

Shylock-Hg Dec 15, 2021 •

edited

Loading

codecov-commenter commented Dec 15, 2021 •

edited

Loading

wenhaocs left a comment •

edited

Loading

Shylock-Hg Dec 23, 2021 •

edited

Loading