add unwind & check vidType when executing not validate #4456

nevermore3 · 2022-07-23T12:40:33Z

What type of PR is this?

bug
feature
enhancement

What problem(s) does this PR solve?

Issue(s) number:

Description:

affect GO、Fetch prop on vertices 、Fetch prop on edges、Find Path 、Get Subgraph
check vidType when executing. not validate

2、add unwindSentence, use unwind in nGql

(root@nebula) [nba]> yield {a:1, b:'Tim Duncan'} as t |yield $-.t["a"] as c |go from $-.c over like yield edge as e
[ERROR (-1005)]: `1', the srcs should be type of FIXED_STRING, but was`INT'

(root@nebula) [nba]> yield {a:1, b:'Tim Duncan'} as t |yield $-.t["a"] as c | find shortest path from 'Tony Parker' to $-.c over like yield path as p
[ERROR (-1005)]: `1', the srcs should be type of FIXED_STRING, but was`INT'

(root@nebula) [nba]> yield {a:1, b:'Tim Duncan'} as t |yield $-.t["a"] as c | fetch prop on * $-.c yield vertex as v
[ERROR (-1005)]: `1', the srcs should be type of FIXED_STRING, but was`INT'


(root@nebula) [nba]> yield {a:1, b:'Tim Duncan', d:'Tony Parker'} as t |yield $-.t["b"] as c, $-.t["a"] as d | fetch prop on like $-.c->$-.d yield edge as e
[ERROR (-1005)]: `1', the dst should be type of FIXED_STRING, but was`INT'


(root@nebula) [nba]> yield {a:1, b:'Tim Duncan', d:'Tony Parker'} as t |yield $-.t["a"] as c|get subgraph from $-.c out like yield edges as e
[ERROR (-1005)]: `1', the srcs should be type of FIXED_STRING, but was`INT'

(root@nebula) [nba]> yield {a:1, b:'Tim Duncan'} as t |yield $-.t["b"] as c |go from $-.c over like yield edge as e
+---------------------------------------------------------+
| e                                                       |
+---------------------------------------------------------+
| [:like "Tim Duncan"->"Manu Ginobili" @0 {likeness: 95}] |
| [:like "Tim Duncan"->"Tony Parker" @0 {likeness: 95}]   |
+---------------------------------------------------------+

(root@nebula) [nba]> yield {a:1, b:'Tim Duncan'} as t |yield $-.t["b"] as c | find shortest path from 'Tony Parker' to $-.c over like yield path as p
+------------------------------------------------+
| p                                              |
+------------------------------------------------+
| <("Tony Parker")-[:like@0 {}]->("Tim Duncan")> |
+------------------------------------------------+

(root@nebula) [nba]> yield {a:1, b:'Tim Duncan'} as t |yield $-.t["b"] as c | fetch prop on * $-.c yield vertex as v
+-------------------------------------------------------------------------------------------------------------+
| v                                                                                                           |
+-------------------------------------------------------------------------------------------------------------+
| ("Tim Duncan" :bachelor{name: "Tim Duncan", speciality: "psychology"} :player{age: 42, name: "Tim Duncan"}) |
+-------------------------------------------------------------------------------------------------------------+

(root@nebula) [nba]> yield {a:1, b:'Tim Duncan', d:'Tony Parker'} as t |yield $-.t["b"] as c, $-.t["d"] as d | fetch prop on like $-.c->$-.d yield edge as e
+-------------------------------------------------------+
| e                                                     |
+-------------------------------------------------------+
| [:like "Tim Duncan"->"Tony Parker" @0 {likeness: 95}] |
+-------------------------------------------------------+

(root@nebula) [nba]> yield {a:1, b:'Tim Duncan', d:'Tony Parker'} as t |yield $-.t["b"] as c|get subgraph from $-.c out like yield edges as e
+----------------------------------------------------------------------------------------------------------------------------------------+
| e                                                                                                                                      |
+----------------------------------------------------------------------------------------------------------------------------------------+
| [[:like "Tim Duncan"->"Manu Ginobili" @0 {}], [:like "Tim Duncan"->"Tony Parker" @0 {}]]                                               |
| [[:like "Manu Ginobili"->"Tim Duncan" @0 {}], [:like "Tony Parker"->"Manu Ginobili" @0 {}], [:like "Tony Parker"->"Tim Duncan" @0 {}]] |
+----------------------------------------------------------------------------------------------------------------------------------------+


(root@nebula) [nba]> yield ['Tim Duncan','Tony Parker'] as c | unwind $-.c as k |go from $-.k over like yield edge as e
+--------------------------------------------------------------+
| e                                                            |
+--------------------------------------------------------------+
| [:like "Tim Duncan"->"Manu Ginobili" @0 {likeness: 95}]      |
| [:like "Tim Duncan"->"Tony Parker" @0 {likeness: 95}]        |
| [:like "Tony Parker"->"LaMarcus Aldridge" @0 {likeness: 90}] |
| [:like "Tony Parker"->"Manu Ginobili" @0 {likeness: 95}]     |
| [:like "Tony Parker"->"Tim Duncan" @0 {likeness: 95}]        |
+--------------------------------------------------------------+

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

Checklist:

Tests:

Unit test(positive and negative cases)
Function test
Performance test
N/A

Affects:

Documentation affected (Please add the label if documentation needs to be modified.)
Incompatibility (If it breaks the compatibility, please describe it and add the label.）
If it's needed to cherry-pick (If cherry-pick to some branches is required, please label the destination version(s).)
Performance impacted: Consumes more CPU/Memory

Release notes:

Please confirm whether to be reflected in release notes and how to describe:

ex. Fixed the bug .....

codecov-commenter · 2022-07-23T17:26:30Z

Codecov Report

Merging #4456 (f84da9d) into master (4ed8da4) will increase coverage by 0.05%.
The diff coverage is 71.51%.

@@            Coverage Diff             @@
##           master    #4456      +/-   ##
==========================================
+ Coverage   84.55%   84.61%   +0.05%     
==========================================
  Files        1344     1346       +2     
  Lines      134103   134189      +86     
==========================================
+ Hits       113397   113545     +148     
+ Misses      20706    20644      -62

Impacted Files	Coverage Δ
src/graph/executor/StorageAccessExecutor.h	`60.00% <ø> (+2.22%)`	⬆️
src/graph/executor/query/AppendVerticesExecutor.h	`100.00% <ø> (ø)`
src/graph/executor/query/GetEdgesExecutor.h	`100.00% <ø> (ø)`
src/graph/executor/query/GetNeighborsExecutor.h	`100.00% <ø> (ø)`
src/graph/executor/query/GetVerticesExecutor.h	`100.00% <ø> (ø)`
src/graph/optimizer/rule/RemoveNoopProjectRule.cpp	`93.61% <ø> (ø)`
src/graph/service/PermissionCheck.cpp	`79.59% <ø> (ø)`
src/graph/validator/FetchEdgesValidator.h	`100.00% <ø> (ø)`
.../graph/validator/test/GetSubgraphValidatorTest.cpp	`100.00% <ø> (ø)`
src/meta/processors/job/DataBalanceJobExecutor.h	`100.00% <ø> (ø)`
... and 63 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 b20964a...f84da9d. Read the comment docs.

Shylock-Hg · 2022-07-25T02:22:48Z

Why change this?

nevermore3 · 2022-07-25T02:29:50Z

Why change this?

user need to use unwind in nGql, convert a list to a column and pipe to the next statement

tests/tck/features/match/PipeAndVariable.feature

czpmango · 2022-07-25T06:32:35Z

tests/tck/features/match/PipeAndVariable.feature

    When executing query:
      """
      LOOKUP ON player
      WHERE player.name == 'Tim Duncan'
      YIELD player.age as age, id(vertex) as vid
-      | UNWIND $-.vid as a RETURN a
+      | UNWIND $-.vid as a | YIELD $-.a AS a


Will the column age be passed on?

will not be passed to the next statement in nGql

czpmango · 2022-07-25T06:39:17Z

src/graph/executor/query/UnwindExecutor.cpp

@@ -26,7 +26,7 @@ folly::Future<Status> UnwindExecutor::execute() {
    std::vector<Value> vals = extractList(list);
    for (auto &v : vals) {
      Row row;
-      if (!emptyInput) {
+      if (!unwind->fromPipe() && !emptyInput) {


How to support multiple variable?
This may break the variable passing ability of ngql's pipe.

Unwind does not support multiple variables in parser

czpmango · 2022-07-25T06:42:27Z

tests/tck/features/match/PipeAndVariable.feature

-    Then a SyntaxError should be raised at runtime: syntax error near `UNWIND'
+    Then the result should be, in any order, with relax comparison:
+      | a            |
+      | "Tim Duncan" |
    When executing query:
      """
      GET SUBGRAPH 2 STEPS FROM "Tim Duncan" BOTH like YIELD edges as e


Add test case:

LOOKUP ... YIELD age,id(vertex) AS vid | UNWIND $-.vid as a MATCH (v)--(n) ...

czpmango · 2022-07-25T06:56:40Z

This unwind has different semantics from cypher unwind? If so, will you consider redesigning the syntax?
Bcz users may be confused about the difference between the two unwinds. May require more consideration from the perspective of syntax design and ease of use. Any better idea about this?

wey-gu · 2022-08-25T10:39:31Z

The unwind in ngql is soooooooooo very useful and it helped me make impossible things possible today, thanks again @nevermore3

ref: https://discuss.nebula-graph.com.cn/t/topic/9848/6

* fix lookup (#4552) fix Co-authored-by: jimingquan <mingquan.ji@vesoft.com> Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * fix split brain in raft (#4479) Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * fix invalid filter in GetProp make storage crashed (#4568) Co-authored-by: haowen <19355821+wenhaocs@users.noreply.github.com> * fix scan vertex/edge do not handle ttl (#4578) * fix scan vertex/edge do not handle ttl * use ErrorCode to unify community version and end version * Fix #1212. Return FoldConstantExprVisitor, if status_ already failed due to found syantax errors. (#4607) Co-authored-by: jie.wang <38901892+jievince@users.noreply.github.com> * Avoid fatal when expression illegal. (#4618) * Fix concurrent exception related to multi-match statement (#4605) * fix filter executor * Fix concurrency exception of multi-match statements fix iterator fix small delete small delete skip iterator type handle for concurrency small delete fix scan edges small delete small delete fix small delete small change small change fix ut small fix Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * Prune properties(#4523) * fix conflict * extract attribute from properties function (#4604) * extract attribute from properties function * fix error * fix subscript error * add test case * process scanEdges * fix test error * add unwind & check vidType when executing not validate (#4456) * Update AppendVerticesExecutor.cpp fix conflict * Update AppendVerticesExecutor.cpp * Replace obsolete RocksDB API (#4395) Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * Update PrunePropertiesRule.feature * remove useless dc (#4533) * Update PrunePropertiesRule.feature * fix test error Co-authored-by: kyle.cao <kyle.cao@vesoft.com> Co-authored-by: jimingquan <mingquan.ji@vesoft.com> Co-authored-by: liwenhui-soul <38217397+liwenhui-soul@users.noreply.github.com> Co-authored-by: Doodle <13706157+critical27@users.noreply.github.com> Co-authored-by: haowen <19355821+wenhaocs@users.noreply.github.com> Co-authored-by: Cheng Xuntao <7731943+xtcyclist@users.noreply.github.com> Co-authored-by: jie.wang <38901892+jievince@users.noreply.github.com> Co-authored-by: shylock <33566796+Shylock-Hg@users.noreply.github.com> Co-authored-by: Qiaolin Yu <90088090+Qiaolin-Yu@users.noreply.github.com>

add unwind

f8dd6b7

nevermore3 force-pushed the unwind branch from 47751ce to f8dd6b7 Compare July 23, 2022 13:13

check

08ce4dc

nevermore3 force-pushed the unwind branch from 9b53fbc to e2618be Compare July 23, 2022 14:01

check vidType when execute, not validate

ca7a8ea

nevermore3 force-pushed the unwind branch from e2618be to ca7a8ea Compare July 23, 2022 14:03

nevermore3 marked this pull request as ready for review July 23, 2022 14:03

nevermore3 requested a review from dutor as a code owner July 23, 2022 14:03

nevermore3 added 2 commits July 23, 2022 22:20

fix parser error

71f6901

fix error

796131c

nevermore3 changed the title ~~add unwind~~ add unwind & check vidType when executing not validate Jul 23, 2022

nevermore3 requested review from Shylock-Hg, czpmango, jievince and Aiee July 23, 2022 15:20

Sophie-Xie added the ready-for-testing PR: ready for the CI test label Jul 23, 2022

add test case

786b2e9

nevermore3 force-pushed the unwind branch from fa45f49 to 786b2e9 Compare July 23, 2022 15:33

nevermore3 added new feature doc affected PR: improvements or additions to documentation ready for review cherry-pick-v3.2 PR: need cherry-pick to this version labels Jul 23, 2022

fix test error

82118e8

nevermore3 force-pushed the unwind branch from f84da9d to 82118e8 Compare July 24, 2022 06:52

nevermore3 requested a review from codesigner July 25, 2022 02:24

fix removeproject error

e4da9c3

nevermore3 force-pushed the unwind branch from 3bd1d70 to e4da9c3 Compare July 25, 2022 03:59

Shylock-Hg reviewed Jul 25, 2022

View reviewed changes

tests/tck/features/match/PipeAndVariable.feature Outdated Show resolved Hide resolved

Shylock-Hg previously approved these changes Jul 25, 2022

View reviewed changes

address comment

cd08097

nevermore3 dismissed Shylock-Hg’s stale review via cd08097 July 25, 2022 06:02

Shylock-Hg approved these changes Jul 25, 2022

View reviewed changes

jievince approved these changes Jul 25, 2022

View reviewed changes

czpmango reviewed Jul 25, 2022

View reviewed changes

dutor approved these changes Jul 25, 2022

View reviewed changes

dutor merged commit fc434bb into vesoft-inc:master Jul 25, 2022

nevermore3 deleted the unwind branch July 25, 2022 07:10

Sophie-Xie removed the cherry-pick-v3.2 PR: need cherry-pick to this version label Jul 25, 2022

wey-gu mentioned this pull request Jul 30, 2022

Weekly Report 2022-07-29 vesoft-inc/nebula-community#124

Closed

Sophie-Xie pushed a commit that referenced this pull request Sep 8, 2022

add unwind & check vidType when executing not validate (#4456)

57f048c

Sophie-Xie added the affects/v3.2 PR/issue: this bug affects v3.2.x version. label Sep 8, 2022

Sophie-Xie added cherry-pick-v3.2 PR: need cherry-pick to this version and removed affects/v3.2 PR/issue: this bug affects v3.2.x version. labels Sep 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add unwind & check vidType when executing not validate #4456

add unwind & check vidType when executing not validate #4456

nevermore3 commented Jul 23, 2022 •

edited

Loading

codecov-commenter commented Jul 23, 2022

Shylock-Hg commented Jul 25, 2022

nevermore3 commented Jul 25, 2022

czpmango Jul 25, 2022

nevermore3 Jul 25, 2022

czpmango Jul 25, 2022

nevermore3 Jul 25, 2022

czpmango Jul 25, 2022

czpmango commented Jul 25, 2022

wey-gu commented Aug 25, 2022 •

edited

Loading

add unwind & check vidType when executing not validate #4456

add unwind & check vidType when executing not validate #4456

Conversation

nevermore3 commented Jul 23, 2022 • edited Loading

What type of PR is this?

What problem(s) does this PR solve?

Issue(s) number:

Description:

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

Checklist:

Release notes:

codecov-commenter commented Jul 23, 2022

Codecov Report

Shylock-Hg commented Jul 25, 2022

nevermore3 commented Jul 25, 2022

czpmango Jul 25, 2022

Choose a reason for hiding this comment

nevermore3 Jul 25, 2022

Choose a reason for hiding this comment

czpmango Jul 25, 2022

Choose a reason for hiding this comment

nevermore3 Jul 25, 2022

Choose a reason for hiding this comment

czpmango Jul 25, 2022

Choose a reason for hiding this comment

czpmango commented Jul 25, 2022

wey-gu commented Aug 25, 2022 • edited Loading

nevermore3 commented Jul 23, 2022 •

edited

Loading

wey-gu commented Aug 25, 2022 •

edited

Loading