-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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.
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
GetDstBySrc #4531
GetDstBySrc #4531
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
auto props = context_->props_; | ||
DCHECK_EQ(props->size(), 1); | ||
|
||
nebula::List list; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nebula::Row is more clear.
@@ -48,9 +48,6 @@ class MultiTagNode : public IterateNode<VertexID> { | |||
|
|||
// add result of each tag node to tagResult | |||
for (auto* tagNode : tagNodes_) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why remove this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same Q
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They have been check on a higher level node, this is redundant. All the check only exists in output node, just to keep unify with others
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job
Generally, can you reuse |
They are different in memory occupation. See https://confluence.nebula-graph.io/pages/viewpage.action?pageId=47392379 |
The response seems like the same: vs. The only difference is in request. But you can mark only returning kDst in the request. |
We could indeed, there are several slight difference between them:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
What type of PR is this?
What problem(s) does this PR solve?
Issue(s) number:
Subtask of #4525
Description:
Add a new rpc interface called GetDstBySrc, the difference between it and GetNeighbors is that it only return dst of the edge in each row (even the srcId is not returned), see the interface in storage.thrift for details. And the dst has been deduped before returned to graphd to minimize memory cost.
How do you solve it?
Special notes for your reviewer, ex. impact of this fix, design document, etc:
Checklist:
Tests:
Affects:
Release notes:
Please confirm whether to be reflected in release notes and how to describe: