feature: add support for variable re-use in MATCH clause

[aheev]

This PR adds support for re-using variables in a MATCH clause with caveats below:

• re-use is supported only on nodes, not rels
• variables which arre not previously labelled can't be re-used
• variable used on a node(label) can't be re-used on a different node(label)

Used label id filter on the nodes to achieve re-use

Added unit and system tests

Closes: #111

[aheev]

@beinan @ChunxuTang could you PTAL? Curious why are we passing query in plan rather than constructor?

[codecov-commenter]

Codecov Report

❌ Patch coverage is 94.11765% with 12 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
crates/lance-graph/src/logical_plan.rs	94.08%	12 Missing ⚠️

📢 Thoughts on this report? Let us know!

[beinan]

Great contribution! just left some minor comments. @ChunxuTang do you want to take a look also?

[beinan]

Curious why are we passing query in plan rather than constructor?

Because the planner is stateful across a single plan run, not across multiple queries. plan(&CypherQuery) keeps the planner reusable and makes it clear that the query is per‑call input. The constructor is used for long‑lived config/state (like GraphConfig or counters), while the query is a transient input. If we put the query in the constructor, we’d either (a) need a new planner per query anyway, or (b) store the query inside and prevent reusing the planner for another query. Passing it to plan is the more flexible pattern and matches how SemanticAnalyzer/DataFusionPlanner are used.

[ChunxuTang]

@beinan Per discussion in this thread #111 (comment), I think it's better to reject self-reference queries and encourage users to rewrite the query to fit better for columnar storage. In the longer term, it's better to have a query rewritter component to automatically rewrite self-reference queries.

[aheev]

Curious why are we passing query in plan rather than constructor?

Because the planner is stateful across a single plan run, not across multiple queries. plan(&CypherQuery) keeps the planner reusable and makes it clear that the query is per‑call input. The constructor is used for long‑lived config/state (like GraphConfig or counters), while the query is a transient input. If we put the query in the constructor, we’d either (a) need a new planner per query anyway, or (b) store the query inside and prevent reusing the planner for another query. Passing it to plan is the more flexible pattern and matches how SemanticAnalyzer/DataFusionPlanner are used.

Could you provide an example where LogicalPlanner/SemanticAnalyzer is used on multiple queries? Also, variables property on these objects would lead to issues if ran on multiple queries right?

[aheev]

@beinan Per discussion in this thread #111 (comment), I think it's better to reject self-reference queries and encourage users to rewrite the query to fit better for columnar storage. In the longer term, it's better to have a query rewritter component to automatically rewrite self-reference queries.

should I refactor this PR to reject self-referencing queries and output a user-friendly error msg?

[aheev]

@beinan Per discussion in this thread #111 (comment), I think it's better to reject self-reference queries and encourage users to rewrite the query to fit better for columnar storage. In the longer term, it's better to have a query rewritter component to automatically rewrite self-reference queries.

should I refactor this PR to reject self-referencing queries and output a user-friendly error msg?

@ChunxuTang gentle ping

[ChunxuTang]

@beinan Per discussion in this thread #111 (comment), I think it's better to reject self-reference queries and encourage users to rewrite the query to fit better for columnar storage. In the longer term, it's better to have a query rewritter component to automatically rewrite self-reference queries.

should I refactor this PR to reject self-referencing queries and output a user-friendly error msg?

@ChunxuTang gentle ping

@aheev How about closing this PR and creating a new PR to reject self-referencing queries with user-friendly error msgs? What do you think?

[aheev]

@beinan Per discussion in this thread #111 (comment), I think it's better to reject self-reference queries and encourage users to rewrite the query to fit better for columnar storage. In the longer term, it's better to have a query rewritter component to automatically rewrite self-reference queries.

should I refactor this PR to reject self-referencing queries and output a user-friendly error msg?

@ChunxuTang gentle ping

@aheev How about closing this PR and creating a new PR to reject self-referencing queries with user-friendly error msgs? What do you think?

Tt's actually working now. Thanks to your PR

test:

// Person Dataset:
| id | name    |
|----|---------|
| 1  | Bob     |
| 2  | Alice   |
| 3  | Charlie |

// Follows Dataset 
| from_id | to_id |
|---------|-------|
| 1       | 2     |
| 2       | 3     |
| 3       | 1     |
| 3       | 3     |

Query: MATCH (p1:Person)-[:FOLLOWS]->(p1) RETURN p1

Result Data: RecordBatch { schema: Schema { fields: [Field { name: "p1.id", data_type: Int64, nullable: false, dict_id: 0, dict_is_ordered: false, metadata: {} }, Field { name: "p1.name", data_type: Utf8, nullable: false, dict_id: 0, dict_is_ordered: false, metadata: {} }], metadata: {} }, columns: [PrimitiveArray<Int64>
[
  3,
], StringArray
[
  "Charlie",
]], row_count: 1 }