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

Add demo graph algorithm as a table function. #3221

Closed
wants to merge 2 commits into from

Conversation

andyfengHKU
Copy link
Contributor

This PR a proof concept to demonstrate how to

  • Add graph class as an abstract layer and pass it as the input of an algorithm function.
  • Register algorithm function as a table function and query it with CALL

Copy link

codecov bot commented Apr 5, 2024

Codecov Report

Attention: Patch coverage is 19.54023% with 70 lines in your changes are missing coverage. Please review.

Project coverage is 92.13%. Comparing base (0b70b02) to head (cec08ae).
Report is 10 commits behind head on master.

❗ Current head cec08ae differs from pull request most recent head dc18b21. Consider uploading reports for the commit dc18b21 to get more accurate results

Files Patch % Lines
src/graph/function.cpp 10.63% 42 Missing ⚠️
src/graph/on_disk_graph.cpp 0.00% 27 Missing ⚠️
src/include/graph/graph.h 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3221      +/-   ##
==========================================
- Coverage   92.42%   92.13%   -0.29%     
==========================================
  Files        1164     1164              
  Lines       44103    44216     +113     
==========================================
- Hits        40762    40739      -23     
- Misses       3341     3477     +136     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

// CallFunction has the assumption that number of output is known before execution.
// This does NOT hold for all graph algorithms.
// So each algorithm need to decide its own shared state to control when to terminate.
struct DemoAlgorithm : public function::CallFunction {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the class that I don't understand. What's "function/function.h vs graph/function.h"? I was expecting to see an algorithm.h class that forms an interface for algorithms, with a generic compute function? I understand Algorithms will be registered as TableFunctions but people implementing new algorithms in the system (us or eventual other developers who write extensions) should not know about the function infrastructure. So it looks like we need a higher-level abstraction over functions to develop algorithms. For example, it's confusing to me that the Graph field is part of DemoAlg's shared state (I am referring to the field in the struct DemoAlgoSharedState : public CallFuncSharedState in function.cpp.

I also need to understand how to pass arguments to functions, e.g., if we needed to pass a start source node to a ShortestPaths algorithm, how do we pass it and how much of that could be simplified. For example, can we abstract away from developers the details of binding expressions or writing any or minimal binding code? Similarly for output expressions of algorithms, I was imagining there is a Algorithm<AlgorithmResult> interface and AlgorithmResult has a factorized table as output, from which the output expressions can be read. Or something along those lines.

@andyfengHKU andyfengHKU force-pushed the graph-function branch 2 times, most recently from 8365e45 to dc18b21 Compare April 11, 2024 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants