Skip to content

Conversation

@xav-db
Copy link
Member

@xav-db xav-db commented Oct 29, 2025

Summary

This PR integrates the fix from #667 into the arena-implementation branch. It adds an .id() method to the ID struct to resolve compilation errors when using AddE with borrowed IDs in FOR loops.

Changes

  • Added ID.id() method in helix-db/src/utils/id.rs returning u128 directly
  • Added test case in hql-tests/tests/add_e_borrowed_ids/ validating the fix

Technical Details

The helixc code generator consistently generates .id() calls to access ID values. Before this change, calling .id() on &ID (borrowed ID) would fail. This fix leverages Rust's auto-deref so both ID.id() and &ID.id() work seamlessly.

Testing

✅ cargo check --workspace passes
✅ All 27 ID struct unit tests pass
✅ New test case compiles successfully

Co-Authored-By: ishaksebsib

Greptile Overview

Updated On: 2025-10-29 11:46:27 UTC

Greptile Summary

This PR fixes a compilation error where AddE queries with borrowed ID references in FOR loops failed because &ID didn't have an .id() method. The solution adds an .id() method to the ID struct that returns u128, leveraging Rust's auto-deref so both ID.id() and &ID.id() work seamlessly.

Key Changes:

  • Added pub fn id(&self) -> u128 method to ID struct in helix-db/src/utils/id.rs:28-30
  • Method mirrors existing .inner() method but provides the interface expected by code generator
  • Code generator in helix-db/src/helixc/analyzer/utils.rs:100 generates {name}.id() calls for identifier references
  • New test case validates the fix works for AddE operations with borrowed IDs from FOR loop iterations

Technical Context:
The helixc code generator's gen_id_access_or_param function consistently generates .id() calls to access ID values from identifiers. When iterating over arrays in FOR loops (like FOR {from_id, to_id} IN edges), the variables are borrowed references (&ID). Before this change, calling .id() on &ID would fail to compile. By adding the .id() method, Rust's auto-deref mechanism automatically handles both owned and borrowed cases.

Important Files Changed

File Analysis

Filename Score Overview
helix-db/src/utils/id.rs 5/5 Added .id() method to ID struct returning u128, enabling both owned and borrowed IDs to work with code generator
hql-tests/tests/add_e_borrowed_ids/queries.hx 5/5 Test query demonstrating FOR loop with borrowed IDs in AddE operation
hql-tests/tests/add_e_borrowed_ids/queries.rs 5/5 Generated code showing .id() method successfully called on borrowed ID references from FOR loop iteration

Sequence Diagram

sequenceDiagram
    participant User as HQL Query
    participant Parser as HQL Parser
    participant Analyzer as Semantic Analyzer
    participant CodeGen as Code Generator
    participant ID as ID Struct
    
    User->>Parser: AddE query with FOR loop
    Note over User: FOR {from_id, to_id} IN edges<br/>AddE::From(from_id)::To(to_id)
    
    Parser->>Analyzer: Parse AddE with identifiers
    Analyzer->>Analyzer: Validate identifiers in scope
    Note over Analyzer: from_id: &ID (borrowed from iteration)
    
    Analyzer->>CodeGen: gen_id_access_or_param(from_id)
    Note over CodeGen: Generates: from_id.id()
    
    CodeGen->>ID: Call .id() on &ID
    Note over ID: Before: &ID had no .id() method<br/>Compiler error: "no method named id"
    
    ID-->>CodeGen: After: auto-deref to ID.id()
    Note over ID: Rust auto-deref: &ID -> ID<br/>Returns u128 from self.0
    
    CodeGen-->>Analyzer: GeneratedValue with u128
    Analyzer-->>User: Compiled query code
Loading

This integrates the fix from PR #667 (#667) which adds an .id() method to the ID struct to resolve compilation errors when using AddE with borrowed IDs in FOR loops.

Changes:
- Added ID.id() method returning u128 in helix-db/src/utils/id.rs
- Added test case for AddE with borrowed IDs from FOR loop destructuring

The helixc code generator consistently uses .id() to access ID values. Before this change, calling .id() on &ID (borrowed ID) would fail. The new method leverages Rust's auto-deref so both ID.id() and &ID.id() work seamlessly.

This fixes queries like:
  FOR {from_id, to_id, ...} IN edges {
    AddE<MyEdge>::From(from_id)::To(to_id)
  }

Co-Authored-By: ishaksebsib <[email protected]>
@xav-db xav-db merged commit efa3e17 into arena-implementation Oct 29, 2025
@xav-db xav-db deleted the pr667-integration branch October 29, 2025 11:44
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

6 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

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.

2 participants