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

Jump to bottom

[master < T187-MAGE] All Shortest Paths Algorithm #409

Merged
merged 34 commits into from Sep 6, 2022
Merged

[master < T187-MAGE] All Shortest Paths Algorithm #409

merged 34 commits into from Sep 6, 2022

Conversation

jmatak
Copy link
Contributor

@jmatak jmatak commented Jun 15, 2022 •
edited by antaljanosbenjamin

Implementation of all shortest paths. Things to consider: how to apply filtering of nodes when traversing paths, and how to include weighted paths along the way.

[master < Epic] PR

  • Check, and update documentation if necessary
  • Update changelog
  • Write E2E tests
  • Compare the benchmarking results between the master branch and the Epic branch

[master < Task] PR

  • Check, and update documentation if necessary
  • Update changelog

Docs PR

@jmatak jmatak self-assigned this Jun 15, 2022
gitbuda
gitbuda reviewed Jul 10, 2022
View reviewed changes
licenses/BSL.txt Outdated Show resolved Hide resolved
gitbuda
gitbuda reviewed Jul 10, 2022
View reviewed changes
CMakeLists.txt Outdated Show resolved Hide resolved
jmatak and others added 17 commits July 13, 2022 15:18
@jmatak
Update total weight
da2a813
@brunos252
added total_weight return
374b8b3
@brunos252
bugfix paths ending in start node, by creating edge list in reverse (…
2c0d8ee 
…compared to expansion)
@jmatak
Update ASP with bi-directional paths
2b4c41f
@jmatak
Fix behaviour of parent container for Dijkstra algorithm
b764e26
@brunos252
remove unnecessary std::move from appending current_edge to previous_…
c6d7ddf 
…, resolving tests fail
@brunos252
unit tests and fixture
4806115
@brunos252
bugfix: add depth state to previous_, reverse edge order if self_.is_…
aaeea7a 
…reverse is set
@jmatak
Update the shortest path with upper bound
3fa3164
@brunos252
rename next_edges_, code comments
96cf85a
@brunos252
unit tests from wshortest, added sorting of results
4e08c94
@brunos252
unit test for all paths
df2f184
@brunos252
unit tests for multiEdge, multiple same length paths
92df8dd
@brunos252
Merge branch 'master' into T187-MAGE_all-shortest-paths
6b97cfd
@brunos252
add mgbench allshortest query
66eb803
@brunos252
add gql_behave test, copy of wshortest with addition of testing multi…
8e5363a 
…ple paths of same length
@jmatak
Revert changes on licence and version
fbf571c
jmatak
jmatak commented Aug 15, 2022
View reviewed changes
src/query/plan/operator.cpp Outdated Show resolved Hide resolved
jmatak
jmatak commented Aug 15, 2022
View reviewed changes
Copy link
Contributor Author

@jmatak jmatak left a comment

Choose a reason for hiding this comment

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

There are some things to comment out in the code itself. Otherwise, tests are written and the full feature is implemented. Moving PR to Ready for Review and starting documentation work.

BorisTasevski
BorisTasevski suggested changes Aug 24, 2022
View reviewed changes
src/query/frontend/ast/cypher_main_visitor.cpp Outdated Show resolved Hide resolved
src/query/frontend/ast/cypher_main_visitor.cpp Outdated Show resolved Hide resolved
src/query/plan/operator.cpp Outdated Show resolved Hide resolved
antaljanosbenjamin
antaljanosbenjamin suggested changes Aug 25, 2022
View reviewed changes
src/query/frontend/ast/cypher_main_visitor.cpp Outdated Show resolved Hide resolved
src/query/plan/operator.cpp Show resolved Hide resolved
src/query/plan/operator.cpp Outdated Show resolved Hide resolved
antaljanosbenjamin
antaljanosbenjamin approved these changes Aug 31, 2022
View reviewed changes
Copy link
Contributor

@antaljanosbenjamin antaljanosbenjamin left a comment

Choose a reason for hiding this comment

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

LGTM

@antaljanosbenjamin antaljanosbenjamin merged commit 9e8fb25 into master Sep 6, 2022
@antaljanosbenjamin antaljanosbenjamin deleted the T187-MAGE_all-shortest-paths branch September 6, 2022 14:21
@brunos252 brunos252 added the v2.4 label Sep 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gitbuda gitbuda gitbuda left review comments

@antaljanosbenjamin antaljanosbenjamin antaljanosbenjamin approved these changes

@Josipmrden Josipmrden Josipmrden approved these changes

@BorisTasevski BorisTasevski BorisTasevski approved these changes

@antepusic antepusic antepusic approved these changes

@niko4299 niko4299 Awaiting requested review from niko4299

@antoniofilipovic antoniofilipovic Awaiting requested review from antoniofilipovic

@kostasrim kostasrim Awaiting requested review from kostasrim

Assignees

@jmatak jmatak

@brunos252 brunos252

Labels
v2.4
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@jmatak @gitbuda @antaljanosbenjamin @Josipmrden @BorisTasevski @antepusic @brunos252 @jbajic