feat(graph): add WITH clause with query chaining support #86
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
ChunxuTang
left a comment
ChunxuTang
left a comment
There was a problem hiding this comment.
Thanks for the work! Just left some minor comments, mostly related to your comments in the code.
rust/lance-graph/src/parser.rs
Outdated
| None => (input, vec![], None), | ||
| }; | ||
|
|
||
| let (input, ret_clause) = return_clause(input)?; |
There was a problem hiding this comment.
ret_clause -> return_clause?
rust/lance-graph/src/semantic.rs
Outdated
| } | ||
| } | ||
|
|
||
| // Phase 2.5: Validate WITH clause if present |
There was a problem hiding this comment.
Could you remove the Phase 2.5 and Phase 2.6 from the comments? We actually don't have Phase 2.1 - 2.4.
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn test_with_post_where_filter() { |
There was a problem hiding this comment.
Could you merge this test and the previous test test_with_order_by_limit? Looks like this test also covers ORDER_BY, so maybe just need to add LIMIT.
rust/lance-graph/src/ast.rs
Outdated
| /// the input for subsequent clauses. Variables not in the WITH projection | ||
| /// are no longer in scope after the WITH. |
There was a problem hiding this comment.
Variables not in the WITH projection are no longer in scope after the WITH.
I don't think this PR enforces this scope restriction. Maybe remove this comment for now and leave a TODO for this type of enforcement.
If you have interests, you can send a follow-up PR for this impl.
dcfocus
force-pushed
the
dac/with_clause
branch
2 times, most recently
from
6c523e2 to
a5cc40a
Compare
Adds WITH clause for intermediate projections, aggregations, and query chaining in Cypher queries. Supported syntax: - WITH projection: WITH p.name AS name - WITH aggregation: WITH city, count(*) AS total - WITH ORDER BY/LIMIT: WITH p ORDER BY p.age DESC LIMIT 10 - Post-WITH WHERE: WITH ... WHERE total > 1 - Post-WITH MATCH: WITH ... MATCH (p2:Person) ... Changes: - Add WithClause to AST with items, order_by, limit fields - Add with_clause, post_with_match_clauses, post_with_where_clause to CypherQuery - Parse WITH clause and post-WITH MATCH/WHERE in parser - Add semantic analysis for WITH scope - Add plan_with_clause in logical planner - Chain post-WITH MATCH using plan_match_clause_with_base - Add 5 comprehensive tests for WITH functionality Note: WITH p (passing whole node) then MATCH (p)-[]->(f) requires explicit property projection. Use WITH p.id AS id ... instead.
dcfocus
force-pushed
the
dac/with_clause
branch
from
a5cc40a to
6d2cb69
Compare
| /// MATCH clauses after WITH (optional) - query chaining | ||
| pub post_with_match_clauses: Vec<MatchClause>, | ||
| /// WHERE clause after WITH (optional) - filters the WITH results | ||
| pub post_with_where_clause: Option<WhereClause>, |
There was a problem hiding this comment.
what if there is more than one "with" in a query?
4 participants
Adds WITH clause for intermediate projections, aggregations, and query chaining in Cypher queries.
Supported syntax:
Changes:
Note: WITH p (passing whole node) then MATCH (p)-[]->(f) requires explicit property projection. Use WITH p.id AS id ... instead.