fix(#10512, #10513, #10515): batch-boundary sensitive writes after a read of more than 100 rows - #10549

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

_{Comment @coderabbitai help to get the list of available commands.}

This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again.

Review

Overall this looks solid. The three fixes are small and targeted, they reuse existing mechanisms (withEagerBarrier, matchClausesNeedEagerDelete, collectPatternVariables), and the tests cover each issue and its neighbours. I found no security concerns: there are no new dependencies, no new I/O and no change to auth paths.

What I checked

• TraversalPath.withRecord is copy-on-write and returns this by identity when nothing changed. That keeps shared paths safe and makes the common case cheap. The private constructor it calls exists. Repeated visits of the same node are handled by scanning from firstStale.
• RowAliases.propagateUpdate only walks the row's top-level values. Rows without a path variable pay one instanceof check per property, which is a small cost.
• collectPatternVariables was refactored to take a List<PathPattern>, and it now also adds path variables. It is shared with the MATCH case, so path variables are now counted as bound after a MATCH too. That looks right, but it is a behaviour change worth keeping in mind for the reversal logic.
• The SUBQUERY barrier is gated on subqueryBodyDeletes and on the same conditions as the outer DELETE, so plain scans keep streaming. The explain test pins this.

Minor suggestions (non-blocking)

1. propagateUpdate replaces the stale path in the current row only. The AggregationStep comment says another row's path is not touched. That is fine, but a short test that two rows sharing the same TraversalPath instance don't affect each other would document the copy-on-write contract end to end.
2. In subqueryBodyDeletes, a body with no getClausesInOrder() falls back to body.hasDelete(). A test with a UNION body, or a nested subquery containing a delete, would exercise the recursive branches. At the moment only the direct DETACH DELETE body is covered.
3. In TraversalPathWithRecordTest, the field alignment (private Vertex a;) is slightly inconsistent with the rest of the code base. This is cosmetic.
4. The new regression test class runs ~100-row queries and a tetrahedron with variable-length paths up to depth 5. It looks fast, but if it shows up in the CI timings, consider @Tag("slow").

I did not run the build or tests locally. Nothing blocks the merge from my side.

🤖 Generated with Claude Code

Up to standards ✅

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

Review

Overall this is a clean, well-scoped fix for the three batch-boundary issues (#10512, #10513, #10515). I found no security concerns: no new dependencies, no I/O or parsing surface, and the changes are confined to the Cypher planner and row/path handling.

What looks good

• A semantically inert OPTIONAL MATCH changes correlated delete results #10512: the subqueryBodyDeletes check is thorough (DELETE, FOREACH, nested subquery, UNION branches). It reuses the existing withEagerBarrier and matchClausesNeedEagerDelete conditions, so plain scans keep streaming. The EXPLAIN-based tests pin both the positive and negative cases.
• Path endpoint properties disagree with the same bound node after REMOVE #10513: TraversalPath.withRecord is copy-on-write and returns this by identity when nothing changed. The cost is limited to rows that carry a path, and the Javadoc explains why lists and maps are deliberately not walked.
• MERGE-created relationship is missing from the following OPTIONAL MATCH on row 101 #10515: factoring collectPatternVariables over List<PathPattern> and including path variables is a nice reuse. Tests cover MERGE, CREATE and MATCH variants.
• Tests follow the assertThat style, and the TraversalPath unit tests cover the edge cases (duplicates, no-op, immutability).

Minor suggestions (non-blocking)

1. In propagateUpdate, the loop now calls withRecord for every path in every row of every SET. It is cheap, but for long variable-length paths (up to *1..N) it is O(path length) per row per write. You could add a cheap pre-check (e.g. skip when the replacement is not a Vertex/Edge, which withRecord already does) - fine as is, just worth keeping in mind for benchmarks.
2. withRecord only replaces members by RID. If the same path is shared across rows and the written record differs per row, that is fine because the path is never mutated, but a short comment in propagateUpdate saying so would help future readers.
3. The comment in AggregationStep now has a very long line; consider re-wrapping.
4. Issue10512...Test has some long query-string lines; fine, but consider text blocks for readability.

LGTM pending CI.

Codecov Report

❌ Patch coverage is 84.74576% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.76%. Comparing base (8b93bb7) to head (ca903f1).
⚠️ Report is 2 commits behind head on main.

@@             Coverage Diff              @@
##               main   #10549      +/-   ##
============================================
- Coverage     78.77%   78.76%   -0.01%     
- Complexity     4736     4737       +1     
============================================
  Files          2284     2284              
  Lines        247758   247815      +57     
  Branches      53945    53968      +23     
============================================
+ Hits         195168   195194      +26     
- Misses        32565    32595      +30     
- Partials      20025    20026       +1     

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