Skip to content

fix: miscellaneous bug fixes and improvements#370

Merged
AviAvni merged 1 commit intomainfrom
misc-fixes
Apr 14, 2026
Merged

fix: miscellaneous bug fixes and improvements#370
AviAvni merged 1 commit intomainfrom
misc-fixes

Conversation

@AviAvni
Copy link
Copy Markdown
Contributor

@AviAvni AviAvni commented Apr 14, 2026

Summary

Independent bug fixes and small improvements extracted from the rdb branch (#359):

  • Optimizer self-reference guard (utilize_node_by_id.rs): Prevent the node-by-ID optimization from firing when the filter's value expression references the same variable as the id() call (self-referencing filter like WHERE id(n) = id(n) + 1)
  • Sorted index properties (procedures.rs): Sort db.indexes() properties and types output by key for deterministic, reproducible results
  • Unwind iterator refactor (unwind.rs): Replace index-based ListExpansion with ValueIter-based IterExpansion to support lazy range iterators and simplify lifetimes
  • Comment out constraints check (graph_utils.py): Disable db.constraints() call in graph_eq until constraints are supported

Not included (deferred to effects PR)

  • temporal.rs zero-arg overloads and .transaction() variants: These require the non_deterministic field additions to the cypher_fn! macro and GraphFn struct, which belong in the effects PR
  • test_multi_writer.py race fix: Already merged to main via Fix flaky test_multi_writer.py #367

Test plan

  • CI passes (cargo check, cargo fmt, cargo clippy)
  • Existing unit tests pass (cargo test -p graph)
  • Flow tests pass (./flow.sh)

Summary by CodeRabbit

  • Performance Improvements

    • Optimized ID filter extraction to reduce unnecessary query planning overhead
    • Replaced lazy expansion mechanism with streaming-based approach for improved memory efficiency
  • Improvements

    • Index properties and types now sorted deterministically for consistent results
  • Tests

    • Temporarily disabled constraints comparison in test utilities pending feature support

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 14, 2026

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

These changes refine graph query optimization and execution: (1) enhance ID filter extraction by validating non-id() operands don't reference the node alias variable, (2) ensure deterministic ordering when enumerating index properties and types, (3) refactor the unwind operator to stream over iterators instead of materializing lists, and (4) disable constraint comparison in graph equality tests pending future support.

Changes

Cohort / File(s) Summary
ID Filter Optimization
graph/src/planner/optimizer/utilize_node_by_id.rs
Added references_var helper to detect variable references within expressions. Updated get_id_filter eligibility logic to reject ID-filter extraction when the non-id() operand contains references to the node alias variable.
Deterministic Index Enumeration
graph/src/runtime/functions/procedures.rs
Modified db.indexes to build "properties" list and "types" map using sorted index field keys rather than relying on map iteration order, ensuring deterministic output.
Unwind Operator Streaming
graph/src/runtime/ops/unwind.rs
Replaced materialized ListExpansion approach with streaming IterExpansion backed by ValueIter. Refactored eval_row to classify iterators and handle distinct cases (empty, single-element, batched). Updated UnwindOp control flow to drain iterators in batches and track expansion state by presence.
Graph Equality Tests
tests/flow/graph_utils.py
Disabled constraints comparison in graph_eq by commenting out CALL db.constraints() block with TODO note, pending constraints feature support.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~65 minutes

Poem

🐰 Oh, what joy when nodes align just right,
No tangled aliases to cause a fright!
We sort and stream with sorted grace,
Each iterator finds its perfect place—
Deterministic hops throughout the space! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'fix: miscellaneous bug fixes and improvements' is overly vague and generic. It uses non-descriptive language ('miscellaneous', 'improvements') that fails to convey the specific, concrete changes in this PR: optimizer guard, sorted indexes, iterator refactor, and constraints comment. Use a more specific title that captures the primary change or key theme, such as 'refactor: iterator-based unwind expansion and related fixes' or 'fix: unwind operator iterator refactor with optimizer and index improvements'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 88.89% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch misc-fixes

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@AviAvni AviAvni merged commit 0924f57 into main Apr 14, 2026
13 of 14 checks passed
@AviAvni AviAvni deleted the misc-fixes branch April 14, 2026 11:58
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 14, 2026

Codecov Report

❌ Patch coverage is 97.91667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 85.47%. Comparing base (c3fb373) to head (654ab3c).
⚠️ Report is 19 commits behind head on main.

Files with missing lines Patch % Lines
graph/src/planner/optimizer/utilize_node_by_id.rs 92.30% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #370      +/-   ##
==========================================
+ Coverage   85.42%   85.47%   +0.05%     
==========================================
  Files         109      109              
  Lines       28000    28013      +13     
==========================================
+ Hits        23918    23945      +27     
+ Misses       4082     4068      -14     
Flag Coverage Δ
fuzz 43.33% <50.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

1 participant