Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable scan in match. #5416

Merged
merged 15 commits into from
Mar 22, 2023
Merged

Enable scan in match. #5416

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
4fd489e
Enable scan in match.
Shylock-Hg Mar 20, 2023
9d47e02
Merge branch 'master' into feature/scan
Shylock-Hg Mar 20, 2023
8ec7665
Tune test.
Shylock-Hg Mar 20, 2023
83d314d
Merge branch 'master' into feature/scan
Shylock-Hg Mar 21, 2023
29341ee
Revert "Tune test."
Shylock-Hg Mar 21, 2023
7485dcf
Disable mold.
Shylock-Hg Mar 21, 2023
25f5357
Merge branch 'feature/scan' of github.com:Shylock-Hg/nebula into feat…
Shylock-Hg Mar 21, 2023
35cf0c6
Skip.
Shylock-Hg Mar 21, 2023
d5cc093
Merge branch 'master' into feature/scan
Shylock-Hg Mar 21, 2023
c7f322a
Debug.
Shylock-Hg Mar 21, 2023
b206462
Merge branch 'feature/scan' of github.com:Shylock-Hg/nebula into feat…
Shylock-Hg Mar 21, 2023
afba22c
Merge branch 'master' into feature/scan
Shylock-Hg Mar 21, 2023
383a0b8
Revert "Debug."
Shylock-Hg Mar 22, 2023
a144475
Revert "Disable mold."
Shylock-Hg Mar 22, 2023
6c1f1ec
Merge branch 'master' into feature/scan
Sophie-Xie Mar 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/graph/executor/query/JoinExecutor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,12 @@ Status JoinExecutor::checkBiInputDataSets() {

DCHECK_LE(colSize_, lhsColSize + rhsColSize);
if (colSize_ < lhsColSize + rhsColSize) {
rhsOutputColIdxs_ = std::vector<size_t>();
for (size_t i = 0; i < rhsColSize; ++i) {
auto it = std::find(lhsColNames.begin(), lhsColNames.end(), rhsColNames[i]);
if (it == lhsColNames.end()) {
rhsOutputColIdxs_.push_back(i);
// not duplicate
rhsOutputColIdxs_.value().push_back(i);
}
}
}
Expand Down Expand Up @@ -107,8 +109,8 @@ Row JoinExecutor::newRow(Row left, Row right) const {
row.values.insert(row.values.end(),
std::make_move_iterator(left.values.begin()),
std::make_move_iterator(left.values.end()));
if (!rhsOutputColIdxs_.empty()) {
for (auto idx : rhsOutputColIdxs_) {
if (rhsOutputColIdxs_.has_value()) {
for (auto idx : rhsOutputColIdxs_.value()) {
row.values.emplace_back(std::move(right.values[idx]));
}
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/graph/executor/query/JoinExecutor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class JoinExecutor : public Executor {
size_t colSize_{0};
// If the join is natural join, rhsOutputColIdxs_ will be used to record the output column index
// of the right. If not, rhsOutputColIdxs_ will be empty.
std::vector<size_t> rhsOutputColIdxs_;
std::optional<std::vector<size_t>> rhsOutputColIdxs_;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why to change this field to optional? The original implementation seems OK.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When |a| join |a| will lead to error.

std::unordered_map<Value, std::vector<const Row*>> hashTable_;
std::unordered_map<List, std::vector<const Row*>> listHashTable_;
};
Expand Down
5 changes: 0 additions & 5 deletions src/graph/executor/query/ScanEdgesExecutor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,6 @@ folly::Future<Status> ScanEdgesExecutor::scanEdges() {
SCOPED_TIMER(&execTime_);
StorageClient *client = qctx()->getStorageClient();
auto *se = asNode<ScanEdges>(node());
if (se->limit() < 0) {
return Status::Error(
"Scan vertices or edges need to specify a limit number, "
"or limit number can not push down.");
}

time::Duration scanEdgesTime;
StorageClient::CommonRequestParam param(se->space(),
Expand Down
5 changes: 0 additions & 5 deletions src/graph/executor/query/ScanVerticesExecutor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,6 @@ folly::Future<Status> ScanVerticesExecutor::scanVertices() {
SCOPED_TIMER(&execTime_);

auto *sv = asNode<ScanVertices>(node());
if (sv->limit() < 0) {
return Status::Error(
"Scan vertices or edges need to specify a limit number,"
" or limit number can not push down.");
}
StorageClient *storageClient = qctx()->getStorageClient();

time::Duration scanVertexTime;
Expand Down
2 changes: 1 addition & 1 deletion tests/tck/features/bugfix/GetEdgesTransformLimitRule.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ Feature: Push Limit down scan edges rule
MATCH p=()-[e]->()
RETURN p LIMIT 3
"""
Then a ExecutionError should be raised at runtime: Scan vertices or edges need to specify a limit number, or limit number can not push down.
Then the execution should be successful
And the execution plan should be:
| id | name | dependencies | operator info |
| 15 | Project | 13 | |
Expand Down
6 changes: 5 additions & 1 deletion tests/tck/features/bugfix/LackFilterGetEdges.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@ Feature: Test lack filter of get edges transform
where e.likeness > 78 or uuid() > 100
return rank(e) limit 3
"""
Then a ExecutionError should be raised at runtime: Scan vertices or edges need to specify a limit number, or limit number can not push down.
Then the result should be, in any order:
| rank(e) |
| 0 |
| 0 |
| 0 |
And the execution plan should be:
| id | name | dependencies | operator info |
| 24 | Project | 20 | |
Expand Down
261 changes: 259 additions & 2 deletions tests/tck/features/match/AllShortestPaths.feature
View file
Open in desktop

Large diffs are not rendered by default.

512 changes: 501 additions & 11 deletions tests/tck/features/match/Base.IntVid.feature
View file
Open in desktop

Large diffs are not rendered by default.

1,090 changes: 1,077 additions & 13 deletions tests/tck/features/match/Base.feature
View file
Open in desktop

Large diffs are not rendered by default.

48 changes: 41 additions & 7 deletions tests/tck/features/match/MatchById.IntVid.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1010,13 +1010,6 @@ Feature: Integer Vid Match By Id
| "Tim Duncan" | "Tony Parker" |
| "Tim Duncan" | "Tony Parker" |
| "Tim Duncan" | "Tony Parker" |
When executing query:
"""
MATCH (a)--(b)
WHERE id(a) == hash('Tim Duncan') OR id(b) == hash('Tony Parker')
RETURN id(a) as src, id(b) as dst
"""
Then a ExecutionError should be raised at runtime: Scan vertices or edges need to specify a limit number, or limit number can not push down.
When executing query:
"""
MATCH (v1)-[:like]->(v2:player)-[:serve]->(v3)
Expand Down Expand Up @@ -1088,3 +1081,44 @@ Feature: Integer Vid Match By Id
| "Manu Ginobili" |
| "Kyle Anderson" |
| "LaMarcus Aldridge" |

Scenario: Can't infer
When executing query:
"""
MATCH (a)--(b)
WHERE id(a) == hash('Tim Duncan') OR id(b) == hash('Tony Parker')
RETURN id(a) as src, id(b) as dst
"""
Then the result should be, in any order:
| src | dst |
| 1106406639891543428 | -7579316172763586624 |
| -1782445125509592239 | -7579316172763586624 |
| -1782445125509592239 | -7579316172763586624 |
| -1782445125509592239 | -7579316172763586624 |
| -7391649757245641883 | -7579316172763586624 |
| 7193291116733635180 | -7579316172763586624 |
| -8206304902611825447 | -7579316172763586624 |
| 5209979940224249985 | -7579316172763586624 |
| -8310021930715358072 | -7579316172763586624 |
| 3394245602834314645 | -7579316172763586624 |
| 3394245602834314645 | -7579316172763586624 |
| 5662213458193308137 | -1782445125509592239 |
| 5662213458193308137 | 3394245602834314645 |
| 5662213458193308137 | -4246510323023722591 |
| 5662213458193308137 | -7579316172763586624 |
| 5662213458193308137 | 3394245602834314645 |
| 5662213458193308137 | -7579316172763586624 |
| 5662213458193308137 | 3394245602834314645 |
| 5662213458193308137 | -7579316172763586624 |
| 5662213458193308137 | 7193291116733635180 |
| 5662213458193308137 | -7034133662712739796 |
| 5662213458193308137 | -1782445125509592239 |
| 5662213458193308137 | -7391649757245641883 |
| 5662213458193308137 | -2886538900410297738 |
| 5662213458193308137 | 5209979940224249985 |
| 5662213458193308137 | -8160811731890648949 |
| 5662213458193308137 | 3394245602834314645 |
| 5662213458193308137 | -4246510323023722591 |
| 5662213458193308137 | -7579316172763586624 |
| 5662213458193308137 | -8206304902611825447 |
| 3394245602834314645 | -7579316172763586624 |
72 changes: 55 additions & 17 deletions tests/tck/features/match/MatchById.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1044,23 +1044,6 @@ Feature: Match By Id
| "Tim Duncan" | "Tony Parker" |
| "Tim Duncan" | "Tony Parker" |
| "Tim Duncan" | "Tony Parker" |
When executing query:
"""
MATCH (a)--(b)
WHERE id(a) == 'Tim Duncan' OR id(b) == 'Tony Parker'
RETURN id(a) as src, id(b) as dst
"""
Then a ExecutionError should be raised at runtime: Scan vertices or edges need to specify a limit number, or limit number can not push down.
When executing query:
"""
MATCH (n) MATCH (n) WHERE id(n) == 'James Harden' RETURN n
"""
Then a ExecutionError should be raised at runtime: Scan vertices or edges need to specify a limit number, or limit number can not push down.
When executing query:
"""
OPTIONAL MATCH (n) MATCH (n) WHERE id(n) == 'James Harden' RETURN n
"""
Then a ExecutionError should be raised at runtime: Scan vertices or edges need to specify a limit number, or limit number can not push down.
When executing query:
"""
OPTIONAL MATCH (n) OPTIONAL MATCH (n) WHERE id(n) == 'James Harden' RETURN n
Expand Down Expand Up @@ -1137,3 +1120,58 @@ Feature: Match By Id
| "Manu Ginobili" |
| "Kyle Anderson" |
| "LaMarcus Aldridge" |

Scenario: Can't seek by id
When executing query:
"""
MATCH (a)--(b)
WHERE id(a) == 'Tim Duncan' OR id(b) == 'Tony Parker'
RETURN id(a) as src, id(b) as dst
"""
Then the result should be, in any order:
| src | dst |
| "Boris Diaw" | "Tony Parker" |
| "Marco Belinelli" | "Tony Parker" |
| "Dejounte Murray" | "Tony Parker" |
| "Kyle Anderson" | "Tony Parker" |
| "Hornets" | "Tony Parker" |
| "LaMarcus Aldridge" | "Tony Parker" |
| "LaMarcus Aldridge" | "Tony Parker" |
| "LaMarcus Aldridge" | "Tony Parker" |
| "Manu Ginobili" | "Tony Parker" |
| "Manu Ginobili" | "Tony Parker" |
| "Manu Ginobili" | "Tony Parker" |
| "Tim Duncan" | "Danny Green" |
| "Tim Duncan" | "LaMarcus Aldridge" |
| "Tim Duncan" | "Manu Ginobili" |
| "Tim Duncan" | "Tony Parker" |
| "Tim Duncan" | "Manu Ginobili" |
| "Tim Duncan" | "Tony Parker" |
| "Tim Duncan" | "Manu Ginobili" |
| "Tim Duncan" | "Tony Parker" |
| "Tim Duncan" | "Spurs" |
| "Tim Duncan" | "Aron Baynes" |
| "Tim Duncan" | "Boris Diaw" |
| "Tim Duncan" | "Danny Green" |
| "Tim Duncan" | "Dejounte Murray" |
| "Tim Duncan" | "LaMarcus Aldridge" |
| "Tim Duncan" | "Manu Ginobili" |
| "Tim Duncan" | "Marco Belinelli" |
| "Tim Duncan" | "Shaquille O'Neal" |
| "Tim Duncan" | "Tiago Splitter" |
| "Tim Duncan" | "Tony Parker" |
| "Spurs" | "Tony Parker" |
When executing query:
"""
OPTIONAL MATCH (n) MATCH (n) WHERE id(n) == 'James Harden' RETURN n
"""
Then the result should be, in any order:
| n |
| ("James Harden":player{age:29,name:"James Harden"}) |
When executing query:
"""
MATCH (n) MATCH (n) WHERE id(n) == 'James Harden' RETURN n
"""
Then the result should be, in any order:
| n |
| ("James Harden":player{age:29,name:"James Harden"}) |
29 changes: 26 additions & 3 deletions tests/tck/features/match/MultiQueryParts.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ Feature: Multi Query Parts
MATCH (m)-[]-(n), (a)-[]-(c) WHERE id(m)=="Tim Duncan"
RETURN m,n,a,c
"""
Then a ExecutionError should be raised at runtime: Scan vertices or edges need to specify a limit number, or limit number can not push down.
# thouands rows, so don't put here
Then the execution should be successful

Scenario: Multi Match
When executing query:
Expand Down Expand Up @@ -236,7 +237,18 @@ Feature: Multi Query Parts
OPTIONAL MATCH (a)<-[]-(b)
RETURN m.player.name AS n1, n.player.name AS n2, a.player.name AS n3 ORDER BY n1, n2, n3 LIMIT 10
"""
Then a ExecutionError should be raised at runtime: Scan vertices or edges need to specify a limit number, or limit number can not push down.
Then the result should be, in order:
| n1 | n2 | n3 |
| "Tim Duncan" | "Aron Baynes" | "Amar'e Stoudemire" |
| "Tim Duncan" | "Aron Baynes" | "Ben Simmons" |
| "Tim Duncan" | "Aron Baynes" | "Carmelo Anthony" |
| "Tim Duncan" | "Aron Baynes" | "Carmelo Anthony" |
| "Tim Duncan" | "Aron Baynes" | "Chris Paul" |
| "Tim Duncan" | "Aron Baynes" | "Chris Paul" |
| "Tim Duncan" | "Aron Baynes" | "Chris Paul" |
| "Tim Duncan" | "Aron Baynes" | "Chris Paul" |
| "Tim Duncan" | "Aron Baynes" | "Danny Green" |
| "Tim Duncan" | "Aron Baynes" | "Danny Green" |

Scenario: Multi Query Parts
When executing query:
Expand Down Expand Up @@ -288,7 +300,18 @@ Feature: Multi Query Parts
MATCH (a)-[]-(b)
RETURN a.player.name AS n1, b.player.name AS n2 ORDER BY n1, n2 LIMIT 10
"""
Then a ExecutionError should be raised at runtime: Scan vertices or edges need to specify a limit number, or limit number can not push down.
Then the result should be, in order:
| n1 | n2 |
| "Amar'e Stoudemire" | "Steve Nash" |
| "Amar'e Stoudemire" | "Steve Nash" |
| "Amar'e Stoudemire" | "Steve Nash" |
| "Amar'e Stoudemire" | "Steve Nash" |
| "Amar'e Stoudemire" | "Steve Nash" |
| "Amar'e Stoudemire" | "Steve Nash" |
| "Amar'e Stoudemire" | "Steve Nash" |
| "Amar'e Stoudemire" | "Steve Nash" |
| "Amar'e Stoudemire" | "Steve Nash" |
| "Amar'e Stoudemire" | "Steve Nash" |

Scenario: Some Errors
When executing query:
Expand Down
4 changes: 3 additions & 1 deletion tests/tck/features/match/Path.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,9 @@ Feature: Matching paths
match p = (v:Label_0)
return count(p)
"""
Then a ExecutionError should be raised at runtime: Scan vertices or edges need to specify a limit number, or limit number can not push down.
Then the result should be, in any order:
| count(p) |
| 60 |

@skip #bug to fix: https://github.com/vesoft-inc/nebula/issues/5185
Scenario: conflicting type
Expand Down
Loading