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.

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

optimize for the last step of go n steps plan #4690

Merged
merged 3 commits into from
Oct 8, 2022

Conversation

jievince
Copy link
Contributor

What type of PR is this?

  • bug
  • feature
  • enhancement

What problem(s) does this PR solve?

Issue(s) number:

Description:

Optimize the last step of go n steps plan.
In the following query, the where clause only uses dst tag props, so we still can replace the last Getneightbors with GetDstbySrc.

GO 3 STEPS FROM "Tony Parker" OVER serve BIDIRECT WHERE $$.team.name != "Lakers" YIELD DISTINCT id($$)

How do you solve it?

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 .....

@jievince jievince added the ready-for-testing PR: ready for the CI test label Sep 30, 2022
@codecov-commenter
Copy link

Codecov Report

Base: 84.69% // Head: 84.72% // Increases project coverage by +0.03% 🎉

Coverage data is based on head (320035e) compared to base (450e58e).
Patch coverage: 92.82% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4690      +/-   ##
==========================================
+ Coverage   84.69%   84.72%   +0.03%     
==========================================
  Files        1358     1358              
  Lines      135674   135833     +159     
==========================================
+ Hits       114911   115091     +180     
+ Misses      20763    20742      -21     
Impacted Files Coverage Δ
src/clients/storage/StorageClient.h 100.00% <ø> (ø)
src/common/graph/Response.h 48.99% <ø> (ø)
src/common/hdfs/HdfsCommandHelper.cpp 0.00% <0.00%> (ø)
src/graph/executor/admin/ShowHostsExecutor.cpp 92.55% <0.00%> (-1.00%) ⬇️
src/graph/executor/algo/BFSShortestPathExecutor.h 100.00% <ø> (ø)
src/graph/executor/algo/BatchShortestPath.cpp 98.29% <ø> (ø)
src/graph/executor/algo/SingleShortestPath.cpp 97.56% <ø> (ø)
src/graph/planner/ngql/GoPlanner.h 100.00% <ø> (ø)
src/graph/planner/ngql/PathPlanner.h 100.00% <ø> (ø)
src/meta/processors/job/BalanceJobExecutor.h 75.00% <ø> (ø)
... and 98 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Shylock-Hg
Copy link
Contributor

Can we do it in optimizer?

@jievince
Copy link
Contributor Author

jievince commented Oct 7, 2022

Can we do it in optimizer?

It's very hard to do it in the optimizer.

nevermore3
nevermore3 previously approved these changes Oct 8, 2022
czpmango
czpmango previously approved these changes Oct 8, 2022
@jievince jievince requested a review from shanlai October 8, 2022 05:36
Comment on lines 96 to 97
// Optimize for some simple go sentence which only need dst id.
// eg. GO 1 TO N STEPS FROM "A" OVER like YIELD DISTINCT like._dst
Copy link
Contributor

Choose a reason for hiding this comment

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

These comments are hijacked.

src/graph/context/ast/QueryAstContext.h Show resolved Hide resolved
src/graph/planner/ngql/GoPlanner.cpp Show resolved Hide resolved
Copy link
Contributor

@dutor dutor left a comment

Choose a reason for hiding this comment

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

LGTM.

@dutor dutor merged commit 444a1f1 into vesoft-inc:master Oct 8, 2022
@jievince jievince deleted the optimize-go-last-step branch October 8, 2022 09:30
@Sophie-Xie Sophie-Xie added the cherry-pick-v3.3 PR: need cherry-pick to this version label Oct 8, 2022
Sophie-Xie pushed a commit that referenced this pull request Oct 8, 2022
* optimize for the last step of go n steps plan

* improve some comments
critical27 pushed a commit that referenced this pull request Oct 12, 2022
* optimize for the last step of go n steps plan (#4690)

* optimize for the last step of go n steps plan

* improve some comments

* change time point calculate in ttl (#4683)

* change time point calculate in ttl

* fix ttl test

* enlarge the ttl sleep interval in the test

* fix ttl in index

* Revert "enlarge the ttl sleep interval in the test"

This reverts commit e37bf51.

* change wall time in mock data

Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>

* fix graph crash (#4698)

* fix_ambiguous_case (#4701)

* fix_ambiguous_case

* change in any order to in order'

* fix map hash method (#4707)

Co-authored-by: jie.wang <38901892+jievince@users.noreply.github.com>
Co-authored-by: pengwei.song <90180021+pengweisong@users.noreply.github.com>
Co-authored-by: jimingquan <mingquan.ji@vesoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick-v3.3 PR: need cherry-pick to this version ready-for-testing PR: ready for the CI test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants