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

[master < T1026] Add vertex degree to index statistics #1026

Merged
merged 34 commits into from
Jun 27, 2023

Conversation

Josipmrden
Copy link
Contributor

@Josipmrden Josipmrden commented Jun 20, 2023

Expanding extra information in ANALYZE GRAPH can be crucial for gaining insights on how to execute a particular query. One of such optimizations is counting information on vertex degrees. That way, supernodes don't need to be expanded when doing MERGE, but the expansion is done the other way around, so less edges need to be verified for checking if the edge exists when doing the merge.

[master < Task] PR

  • Check, and update documentation if necessary (did not yet)
  • Provide the full content or a guide for the final git message

To keep docs changelog up to date, one more thing to do:

  • Write a release note here, including added/changed clause
    ANALYZE GRAPH query has been expanded with degree information for all the vertices to enhance MERGE optimizations on supernodes.
  • Tag someone from docs team in the comments

@Josipmrden Josipmrden added this to the mg-v2.9.0 milestone Jun 20, 2023
@Josipmrden Josipmrden self-assigned this Jun 20, 2023
@Josipmrden Josipmrden marked this pull request as draft June 20, 2023 08:53
@gitbuda gitbuda requested a review from gvolfing June 23, 2023 07:40
@Josipmrden Josipmrden requested a review from as51340 June 26, 2023 10:26
@Josipmrden Josipmrden marked this pull request as ready for review June 26, 2023 11:03
Copy link
Contributor

@as51340 as51340 left a comment

Choose a reason for hiding this comment

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

Many things going on in the query planner. I think in general the feature makes a lot of sense. However I also think that we should make this always available to users not only when analyze graph is called since use cases are different, one is for optimizing indices, this is for generating query plan.

src/storage/v2/storage.hpp Outdated Show resolved Hide resolved
src/storage/v2/storage.hpp Outdated Show resolved Hide resolved
src/storage/v2/storage.hpp Outdated Show resolved Hide resolved
src/storage/v2/indices.cpp Show resolved Hide resolved
src/query/interpreter.cpp Outdated Show resolved Hide resolved
tests/e2e/analyze_graph/optimize_indexes.py Outdated Show resolved Hide resolved
tests/e2e/analyze_graph/common.py Show resolved Hide resolved
tests/unit/query_cost_estimator.cpp Show resolved Hide resolved
tests/e2e/analyze_graph/optimize_indexes.py Show resolved Hide resolved
tests/e2e/analyze_graph/optimize_indexes.py Show resolved Hide resolved
@Josipmrden Josipmrden changed the title [master < ] Add vertex degree to index statistics [master < T1026] Add vertex degree to index statistics Jun 27, 2023
@Josipmrden Josipmrden requested review from antoniofilipovic and as51340 and removed request for gvolfing June 27, 2023 11:32
@Josipmrden
Copy link
Contributor Author

@vpavicic (haven't still update the docs, will do that today/tomorrow)

Copy link
Collaborator

@antoniofilipovic antoniofilipovic left a comment

Choose a reason for hiding this comment

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

Looks good to me

Copy link
Contributor

@as51340 as51340 left a comment

Choose a reason for hiding this comment

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

Good job, I have few code comments, include them if you have time otherwise feel free to ignore, it won't break in either way

src/query/interpreter.cpp Outdated Show resolved Hide resolved
src/query/plan/cost_estimator.hpp Show resolved Hide resolved
src/query/interpreter.cpp Show resolved Hide resolved
src/query/interpreter.cpp Show resolved Hide resolved
src/query/interpreter.cpp Show resolved Hide resolved
src/query/interpreter.cpp Outdated Show resolved Hide resolved
src/query/interpreter.cpp Outdated Show resolved Hide resolved
src/query/interpreter.cpp Show resolved Hide resolved
src/query/interpreter.cpp Show resolved Hide resolved
src/query/interpreter.cpp Show resolved Hide resolved
@Josipmrden Josipmrden merged commit 84721f7 into master Jun 27, 2023
6 checks passed
@vpavicic
Copy link
Contributor

RN: The ANALYZE GRAPH; query now includes information about the degree of all
nodes to enhance the MERGE optimizations on supernodes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants