Implement the multi query parts of openCypher. #3519
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CPWstatic
force-pushed
the
impl_match_multi_path_according_to_the_paper
branch
from
9f7da29 to
e5c2101
Compare
CPWstatic
added
ready for review
ready-for-testing
Sophie-Xie
added
doc affected
|
It worked like a charm!!! Thanks!!! |
czpmango
reviewed
Dec 24, 2021
| Background: | ||
| Given a graph with space named "nba" | ||
|
|
||
| Scenario: Multi Path Patterns |
There was a problem hiding this comment.
Add test cases:
MATCH (m:player{name:"Tim Duncan"})-[:like]-(n)--()
WITH m
MATCH (m)--(n)
RETURN count(*) AS scount
MATCH (m:player{name:"Tim Duncan"})-[:like]-(n)--()
WITH m,n
MATCH (m)--(n)
RETURN count(*) AS scount
9f8aa44 to
ebec681
Compare
|
updated: this was based on commit 1 days before and I noticed there is an Fix union commit after this WIP build, maybe that just fixed this one(or?), if so, plz ignore this, I will build from your head later. This result seems to differ from the same query/data in n4j, not sure it's wrong behavior or what I had done wrongly. ref: |
|
CPWstatic
force-pushed
the
impl_match_multi_path_according_to_the_paper
branch
from
204247c to
43febea
Compare
jievince
reviewed
Dec 27, 2021
| size_t startIndex = 0; | ||
| bool startFromEdge = false; | ||
|
|
||
| NG_RETURN_IF_ERROR(findStarts( |
There was a problem hiding this comment.
Could we move the logic of StartVidFinder or IndexSelection to the optimizer in the future?
These logic makes the plannner complicated.
There was a problem hiding this comment.
I'm ok if you have a full solution, talk is always cheap.
CPWstatic
force-pushed
the
impl_match_multi_path_according_to_the_paper
branch
from
65d3f4c to
0cd750d
Compare
I didn't reproduce this one. |
Thanks, my attempts of reproducing failed, too, it should be a false report, sorry for that. |
yixinglu
reviewed
Dec 27, 2021
| std::unordered_set<std::string> nodeAliasesSeen; | ||
| // TODO: Maybe it is better to rebuild the graph and find all connected components. | ||
| auto& pathInfos = matchClauseCtx->paths; | ||
| for (auto iter = pathInfos.begin(); iter < pathInfos.end(); ++iter) { |
There was a problem hiding this comment.
Emmm, seems not that much necessary, the std::vector::iterator supports both < and !=.
| std::unordered_set<std::string> nodeAliasesSeen; | ||
| // TODO: Maybe it is better to rebuild the graph and find all connected components. | ||
| auto& pathInfos = matchClauseCtx->paths; | ||
| for (auto iter = pathInfos.begin(); iter < pathInfos.end(); ++iter) { |
| MATCH (a)-[]-(b) | ||
| RETURN a.name AS n1, b.name AS n2 ORDER BY n1, n2 LIMIT 10 | ||
| """ | ||
| Then a ExecutionError should be raised at runtime: Scan vertices must specify limit number. |
There was a problem hiding this comment.
this error message will be updated in #3549
CPWstatic
force-pushed
the
impl_match_multi_path_according_to_the_paper
branch
from
0cd750d to
ca04bab
Compare
Fixed in the latest commit, please try it. |
|
continue of #3519 (comment) Now with the latest build, that step was fixed, while when I move on to the further extra optional match/ WITH lines, something is wrong here. The result remains OK till this:(below results are correct) # n4j
MATCH (source:Table {key: "hive://gold.test_schema/test_table1"})
OPTIONAL MATCH dpath=(source)-[downstream_len:HAS_DOWNSTREAM*..3]->(downstream_entity:Table)
OPTIONAL MATCH upath=(source)-[upstream_len:HAS_UPSTREAM*..3]->(upstream_entity:Table)
WITH downstream_entity, upstream_entity, downstream_len, upstream_len, upath, dpath
OPTIONAL MATCH (upstream_entity)-[:HAS_BADGE]->(upstream_badge:Badge)
OPTIONAL MATCH (downstream_entity)-[:HAS_BADGE]->(downstream_badge:Badge)
WITH CASE WHEN downstream_badge IS NULL THEN []
ELSE collect(distinct {key:downstream_badge.key,category:downstream_badge.category})
END AS downstream_badges, CASE WHEN upstream_badge IS NULL THEN []
ELSE collect(distinct {key:upstream_badge.key,category:upstream_badge.category})
END AS upstream_badges, upstream_entity, downstream_entity, upstream_len, downstream_len, upath, dpath
OPTIONAL MATCH (downstream_entity:Table)-[downstream_read:READ_BY]->(:User)
WITH upstream_entity, downstream_entity, upstream_len, downstream_len, upath, dpath,
downstream_badges, upstream_badges, sum(downstream_read.read_count) as downstream_read_count
OPTIONAL MATCH (upstream_entity:Table)-[upstream_read:READ_BY]->(:User)
RETURN upstream_entity, downstream_entity, upstream_len, downstream_len,
downstream_badges, upstream_badges, downstream_read_count,
sum(upstream_read.read_count) as upstream_read_count, upath, dpath
╒═════════════════╤══════════════════════════════════════════════════════════════════════╤══════════════╤══════════════════════════════════════════════════════════════════════╤══════════════════════════════════════════════════════════════════════╤═════════════════╤═══════════════════════╤═════════════════════╤═══════╤══════════════════════════════════════════════════════════════════════╕
│"upstream_entity"│"downstream_entity" │"upstream_len"│"downstream_len" │"downstream_badges" │"upstream_badges"│"downstream_read_count"│"upstream_read_count"│"upath"│"dpath" │
╞═════════════════╪══════════════════════════════════════════════════════════════════════╪══════════════╪══════════════════════════════════════════════════════════════════════╪══════════════════════════════════════════════════════════════════════╪═════════════════╪═══════════════════════╪═════════════════════╪═══════╪══════════════════════════════════════════════════════════════════════╡
│null │{"name":"test_view1","publisher_last_updated_epoch_ms":1640401809838,"│null │[{"publisher_last_updated_epoch_ms":1640401809909,"published_tag":"lin│[] │[] │0 │0 │null │[{"name":"test_table1","publisher_last_updated_epoch_ms":1640401810033│
│ │published_tag":"unique_tag","key":"hive://gold.test_schema/test_view1"│ │eage_unique_tag"}] │ │ │ │ │ │,"published_tag":"unique_tag","key":"hive://gold.test_schema/test_tabl│
│ │,"is_view":true} │ │ │ │ │ │ │ │e1","is_view":false},{"publisher_last_updated_epoch_ms":1640401809909,│
│ │ │ │ │ │ │ │ │ │"published_tag":"lineage_unique_tag"},{"name":"test_view1","publisher_│
│ │ │ │ │ │ │ │ │ │last_updated_epoch_ms":1640401809838,"published_tag":"unique_tag","key│
│ │ │ │ │ │ │ │ │ │":"hive://gold.test_schema/test_view1","is_view":true}] │
├─────────────────┼──────────────────────────────────────────────────────────────────────┼──────────────┼──────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────────────────────┼─────────────────┼───────────────────────┼─────────────────────┼───────┼──────────────────────────────────────────────────────────────────────┤
│null │{"name":"test's_table4","publisher_last_updated_epoch_ms":164040180983│null │[{"publisher_last_updated_epoch_ms":1640401809909,"published_tag":"lin│[] │[] │0 │0 │null │[{"name":"test_table1","publisher_last_updated_epoch_ms":1640401810033│
│ │8,"published_tag":"unique_tag","key":"hive://gold.test_schema/test's_t│ │eage_unique_tag"},{"publisher_last_updated_epoch_ms":1640401809910,"pu│ │ │ │ │ │,"published_tag":"unique_tag","key":"hive://gold.test_schema/test_tabl│
│ │able4","is_view":true} │ │blished_tag":"lineage_unique_tag"}] │ │ │ │ │ │e1","is_view":false},{"publisher_last_updated_epoch_ms":1640401809909,│
│ │ │ │ │ │ │ │ │ │"published_tag":"lineage_unique_tag"},{"name":"test_view1","publisher_│
│ │ │ │ │ │ │ │ │ │last_updated_epoch_ms":1640401809838,"published_tag":"unique_tag","key│
│ │ │ │ │ │ │ │ │ │":"hive://gold.test_schema/test_view1","is_view":true},{"name":"test_v│
│ │ │ │ │ │ │ │ │ │iew1","publisher_last_updated_epoch_ms":1640401809838,"published_tag":│
│ │ │ │ │ │ │ │ │ │"unique_tag","key":"hive://gold.test_schema/test_view1","is_view":true│
│ │ │ │ │ │ │ │ │ │},{"publisher_last_updated_epoch_ms":1640401809910,"published_tag":"li│
│ │ │ │ │ │ │ │ │ │neage_unique_tag"},{"name":"test's_table4","publisher_last_updated_epo│
│ │ │ │ │ │ │ │ │ │ch_ms":1640401809838,"published_tag":"unique_tag","key":"hive://gold.t│
│ │ │ │ │ │ │ │ │ │est_schema/test's_table4","is_view":true}] │
├─────────────────┼──────────────────────────────────────────────────────────────────────┼──────────────┼──────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────────────────────┼─────────────────┼───────────────────────┼─────────────────────┼───────┼──────────────────────────────────────────────────────────────────────┤
│null │{"name":"test_table2","publisher_last_updated_epoch_ms":1640401810033,│null │[{"publisher_last_updated_epoch_ms":1640401809909,"published_tag":"lin│[{"category":"table_status","key":"npi"},{"category":"table_status","k│[] │520 │0 │null │[{"name":"test_table1","publisher_last_updated_epoch_ms":1640401810033│
│ │"published_tag":"unique_tag","key":"dynamo://gold.test_schema/test_tab│ │eage_unique_tag"}] │ey":"json"}] │ │ │ │ │,"published_tag":"unique_tag","key":"hive://gold.test_schema/test_tabl│
│ │le2","is_view":false} │ │ │ │ │ │ │ │e1","is_view":false},{"publisher_last_updated_epoch_ms":1640401809909,│
│ │ │ │ │ │ │ │ │ │"published_tag":"lineage_unique_tag"},{"name":"test_table2","publisher│
│ │ │ │ │ │ │ │ │ │_last_updated_epoch_ms":1640401810033,"published_tag":"unique_tag","ke│
│ │ │ │ │ │ │ │ │ │y":"dynamo://gold.test_schema/test_table2","is_view":false}] │
├─────────────────┼──────────────────────────────────────────────────────────────────────┼──────────────┼──────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────────────────────┼─────────────────┼───────────────────────┼─────────────────────┼───────┼──────────────────────────────────────────────────────────────────────┤
│null │{"name":"test_table3","publisher_last_updated_epoch_ms":1640401809838,│null │[{"publisher_last_updated_epoch_ms":1640401809909,"published_tag":"lin│[] │[] │0 │0 │null │[{"name":"test_table1","publisher_last_updated_epoch_ms":1640401810033│
│ │"published_tag":"unique_tag","key":"hive://gold.test_schema/test_table│ │eage_unique_tag"},{"publisher_last_updated_epoch_ms":1640401809910,"pu│ │ │ │ │ │,"published_tag":"unique_tag","key":"hive://gold.test_schema/test_tabl│
│ │3","is_view":true} │ │blished_tag":"lineage_unique_tag"}] │ │ │ │ │ │e1","is_view":false},{"publisher_last_updated_epoch_ms":1640401809909,│
│ │ │ │ │ │ │ │ │ │"published_tag":"lineage_unique_tag"},{"name":"test_table2","publisher│
│ │ │ │ │ │ │ │ │ │_last_updated_epoch_ms":1640401810033,"published_tag":"unique_tag","ke│
│ │ │ │ │ │ │ │ │ │y":"dynamo://gold.test_schema/test_table2","is_view":false},{"name":"t│
│ │ │ │ │ │ │ │ │ │est_table2","publisher_last_updated_epoch_ms":1640401810033,"published│
│ │ │ │ │ │ │ │ │ │_tag":"unique_tag","key":"dynamo://gold.test_schema/test_table2","is_v│
│ │ │ │ │ │ │ │ │ │iew":false},{"publisher_last_updated_epoch_ms":1640401809910,"publishe│
│ │ │ │ │ │ │ │ │ │d_tag":"lineage_unique_tag"},{"name":"test_table3","publisher_last_upd│
│ │ │ │ │ │ │ │ │ │ated_epoch_ms":1640401809838,"published_tag":"unique_tag","key":"hive:│
│ │ │ │ │ │ │ │ │ │//gold.test_schema/test_table3","is_view":true}] │
├─────────────────┼──────────────────────────────────────────────────────────────────────┼──────────────┼──────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────────────────────┼─────────────────┼───────────────────────┼─────────────────────┼───────┼──────────────────────────────────────────────────────────────────────┤
│null │{"name":"test's_table4","publisher_last_updated_epoch_ms":164040180983│null │[{"publisher_last_updated_epoch_ms":1640401809909,"published_tag":"lin│[] │[] │0 │0 │null │[{"name":"test_table1","publisher_last_updated_epoch_ms":1640401810033│
│ │8,"published_tag":"unique_tag","key":"hive://gold.test_schema/test's_t│ │eage_unique_tag"},{"publisher_last_updated_epoch_ms":1640401809910,"pu│ │ │ │ │ │,"published_tag":"unique_tag","key":"hive://gold.test_schema/test_tabl│
│ │able4","is_view":true} │ │blished_tag":"lineage_unique_tag"}] │ │ │ │ │ │e1","is_view":false},{"publisher_last_updated_epoch_ms":1640401809909,│
│ │ │ │ │ │ │ │ │ │"published_tag":"lineage_unique_tag"},{"name":"test_table2","publisher│
│ │ │ │ │ │ │ │ │ │_last_updated_epoch_ms":1640401810033,"published_tag":"unique_tag","ke│
│ │ │ │ │ │ │ │ │ │y":"dynamo://gold.test_schema/test_table2","is_view":false},{"name":"t│
│ │ │ │ │ │ │ │ │ │est_table2","publisher_last_updated_epoch_ms":1640401810033,"published│
│ │ │ │ │ │ │ │ │ │_tag":"unique_tag","key":"dynamo://gold.test_schema/test_table2","is_v│
│ │ │ │ │ │ │ │ │ │iew":false},{"publisher_last_updated_epoch_ms":1640401809910,"publishe│
│ │ │ │ │ │ │ │ │ │d_tag":"lineage_unique_tag"},{"name":"test's_table4","publisher_last_u│
│ │ │ │ │ │ │ │ │ │pdated_epoch_ms":1640401809838,"published_tag":"unique_tag","key":"hiv│
│ │ │ │ │ │ │ │ │ │e://gold.test_schema/test's_table4","is_view":true}] │
└─────────────────┴──────────────────────────────────────────────────────────────────────┴──────────────┴──────────────────────────────────────────────────────────────────────┴──────────────────────────────────────────────────────────────────────┴─────────────────┴───────────────────────┴─────────────────────┴───────┴──────────────────────────────────────────────────────────────────────┘The issue comes in further(incorrect): Where it turned out Please check queries a. and b. on this and check on the cluster I shared to you. a. b.
|
This seem like expression issue, will debug later. |
CPWstatic
force-pushed
the
impl_match_multi_path_according_to_the_paper
branch
from
ca04bab to
4898aa5
Compare
yixinglu
previously approved these changes
Dec 28, 2021
|
See this case: The result seems different with neo4j. |
Shylock-Hg
reviewed
Dec 28, 2021
| if (notVisited) { | ||
| queue.push(dep); | ||
| std::vector<folly::Future<Status>>& futures = futureMap[exe->id()]; | ||
| if (exe->node()->kind() == PlanNode::Kind::kArgument) { |
There was a problem hiding this comment.
Why need special handling?
Emmm, i see, seems we do not ever support this case. I'll forbid this one first, until we have plannode like |
A second thought, we might not need to implement it, as it is equal to: |
CPWstatic
force-pushed
the
impl_match_multi_path_according_to_the_paper
branch
from
d4b1574 to
518aa2a
Compare
yixinglu
approved these changes
Dec 29, 2021
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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/ Design document:
Checklist:
Release notes:
A little incompatible: The returned column names are the same sequence with the declared columns, while it is random in former release.
Now this query returns:
New features:
We have more test cases in pr #3537