#3997 test(cypher): regression tests for collect(r)+VLP row-drop bug#4010
Conversation
Adds regression tests to OpenCypherVariableLengthPathTest covering the scenarios reported in issue #3997: when collect(r) is carried via WITH and a later MATCH uses a variable-length path, no rows should be dropped. The root fix was already merged in commit d58bf7b (issue #3999) which introduced the previousStepVariables mechanism in ExpandPathStep. That mechanism correctly excludes WITH-projected variables like rels (from collect(r)) from the same-MATCH edge-uniqueness conflict check. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Coverage variation | ✅ -8.53% coverage variation |
| Diff coverage | ✅ ∅ diff coverage |
Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (63ec984) 120069 88599 73.79% Head commit (5f23c65) 151308 (+31239) 98747 (+10148) 65.26% (-8.53%) Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>
Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#4010) 0 0 ∅ (not applicable) Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%
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.
There was a problem hiding this comment.
Code Review
This pull request introduces three new test cases to OpenCypherVariableLengthPathTest.java to address Issue #3997, specifically testing the interaction between collect() and variable-length paths in OpenCypher queries. The review feedback suggests wrapping the database.query() calls in try-with-resources blocks to ensure that the ResultSet is properly closed and database resources are released.
| database.query("opencypher", | ||
| """ | ||
| MATCH (a:Person {name: 'Alice'}) | ||
| OPTIONAL MATCH (a)-[r:KNOWS]->(b:Person) | ||
| WITH a, collect(DISTINCT r) AS rels | ||
| MATCH path = (a)-[:KNOWS*1..2]->(c:Person) | ||
| RETURN length(path) AS len, size(rels) AS relationCount | ||
| ORDER BY len""").forEachRemaining(rows::add); |
There was a problem hiding this comment.
The ResultSet should be closed to prevent potential resource leaks. This also applies to the other new test cases in this PR.
| database.query("opencypher", | |
| """ | |
| MATCH (a:Person {name: 'Alice'}) | |
| OPTIONAL MATCH (a)-[r:KNOWS]->(b:Person) | |
| WITH a, collect(DISTINCT r) AS rels | |
| MATCH path = (a)-[:KNOWS*1..2]->(c:Person) | |
| RETURN length(path) AS len, size(rels) AS relationCount | |
| ORDER BY len""").forEachRemaining(rows::add); | |
| try (final ResultSet resultSet = database.query("opencypher", | |
| """ | |
| MATCH (a:Person {name: 'Alice'}) | |
| OPTIONAL MATCH (a)-[r:KNOWS]->(b:Person) | |
| WITH a, collect(DISTINCT r) AS rels | |
| MATCH path = (a)-[:KNOWS*1..2]->(c:Person) | |
| RETURN length(path) AS len, size(rels) AS relationCount | |
| ORDER BY len""")) { | |
| resultSet.forEachRemaining(rows::add); | |
| } |
There was a problem hiding this comment.
Fixed in 5f23c65 - all three new test methods now wrap the ResultSet in try-with-resources.
…ssion tests Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4010 +/- ##
==========================================
- Coverage 64.78% 64.22% -0.57%
==========================================
Files 1597 1597
Lines 120069 120069
Branches 25557 25557
==========================================
- Hits 77790 77115 -675
- Misses 31500 32280 +780
+ Partials 10779 10674 -105 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…o [skip ci] Bumps [marked](https://github.com/markedjs/marked) from 18.0.5 to 18.0.6. Release notes *Sourced from [marked's releases](https://github.com/markedjs/marked/releases).* > v18.0.6 > ------- > > [18.0.6](markedjs/marked@v18.0.5...v18.0.6) (2026-07-09) > ----------------------------------------------------------------------------------- > > ### Bug Fixes > > * Avoid O(n^2) backtracking in inline link href regex ([#4013](https://redirect.github.com/markedjs/marked/issues/4013)) ([a009808](markedjs/marked@a009808)) > * Fix ordered lists after blockquotes ([#4003](https://redirect.github.com/markedjs/marked/issues/4003)) ([33928d0](markedjs/marked@33928d0)) > * keep trailing text on HTML block close line for PI, declarations, and CDATA ([#3991](https://redirect.github.com/markedjs/marked/issues/3991)) ([bbb84c8](markedjs/marked@bbb84c8)) Commits * [`39bd884`](markedjs/marked@39bd884) chore(release): 18.0.6 [skip ci] * [`6bce57d`](markedjs/marked@6bce57d) chore(deps-dev): bump `@semantic-release/github` from 12.0.8 to 12.0.9 ([#4009](https://redirect.github.com/markedjs/marked/issues/4009)) * [`63a4bba`](markedjs/marked@63a4bba) chore(deps-dev): bump markdown-it from 14.2.0 to 14.3.0 ([#4010](https://redirect.github.com/markedjs/marked/issues/4010)) * [`a009808`](markedjs/marked@a009808) fix: Avoid O(n^2) backtracking in inline link href regex ([#4013](https://redirect.github.com/markedjs/marked/issues/4013)) * [`33928d0`](markedjs/marked@33928d0) fix: Fix ordered lists after blockquotes ([#4003](https://redirect.github.com/markedjs/marked/issues/4003)) * [`bbb84c8`](markedjs/marked@bbb84c8) fix: keep trailing text on HTML block close line for PI, declarations, and CD... * [`d2af54e`](markedjs/marked@d2af54e) chore(deps-dev): bump eslint from 10.5.0 to 10.6.0 ([#4005](https://redirect.github.com/markedjs/marked/issues/4005)) * [`f403898`](markedjs/marked@f403898) chore(deps-dev): bump `@arethetypeswrong/cli` from 0.18.3 to 0.18.4 ([#4006](https://redirect.github.com/markedjs/marked/issues/4006)) * [`f8f4112`](markedjs/marked@f8f4112) chore(deps): bump actions/checkout from 6 to 7 ([#4000](https://redirect.github.com/markedjs/marked/issues/4000)) * [`5ddfd8a`](markedjs/marked@5ddfd8a) chore: Update repository metadata URLs ([#3998](https://redirect.github.com/markedjs/marked/issues/3998)) * Additional commits viewable in [compare view](markedjs/marked@v18.0.5...v18.0.6) [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- Dependabot commands and options You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
$(cat <<'EOF'
Summary
Closes #3997.
Adds regression tests for the scenario where
collect(r)is carried viaWITHand a laterMATCHuses a variable-length path (VLP) - previously causing all rows to be dropped.collectRelThenVlpReturnsRows- basiccollect(r) AS rels+ VLP returns 2 rows (len=1 and len=2)collectDistinctRelThenVlpReturnsRows- OPTIONAL MATCH +collect(DISTINCT r)+ VLP (the "stronger variant" from the issue) returns 2 rowscollectScalarThenVlpReturnsRows- control case:collect(type(r))continues to workRoot cause and fix
The bug was caused by
ExpandPathStep.hasEdgeConflict()treatingList<Edge>values (fromcollect(r)) as same-MATCH bound relationships, blocking all VLP paths when those edges appeared in the traversal.The fix was already merged in commit
d58bf7b1c(issue #3999), which introduced thepreviousStepVariablesmechanism inExpandPathStep. AfterWITH a, collect(r) AS rels,boundVariables = {a, rels}. When the second MATCH VLP runs,previousStepVariables = {a, rels}, sorelsis correctly skipped in the conflict check.Test plan
collectRelThenVlpReturnsRowspasses - primary bug scenariocollectDistinctRelThenVlpReturnsRowspasses - OPTIONAL MATCH + collect(DISTINCT r) variantcollectScalarThenVlpReturnsRowspasses - control case (scalar collect)OpenCypherVariableLengthPathTesttests pass (3 pre-existing + 3 new = 6 total)Issue3999RelVarLostAfterVarLengthMatchTesttests pass (6 total)🤖 Generated with Claude Code
EOF
)