#3998 fix(cypher): MERGE with unbound label-only endpoint creates fresh node#4005
#3998 fix(cypher): MERGE with unbound label-only endpoint creates fresh node#4005
Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
🟢 Coverage 91.06% diff coverage · -7.80% coverage variation
Metric Results Coverage variation ✅ -7.80% coverage variation Diff coverage ✅ 91.06% diff coverage Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (4ad4748) 120048 88335 73.58% Head commit (4077cc6) 151288 (+31240) 99515 (+11180) 65.78% (-7.80%) 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 (#4005) 123 112 91.06% 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 refactors the MERGE path execution logic to resolve issue #3998, ensuring that unbound endpoint variables are resolved via edge traversal rather than independent node lookups. The changes introduce a recursive DFS walker for path matching and a dedicated path creation method. Feedback includes addressing a missing direction check in undirected relationship traversals when the anchor is unbound, handling potential conflicts between forced vertices and existing variable bindings, and optimizing node label matching by using instanceOf instead of retrieving all labels.
| Vertex vertex = forcedVertex; | ||
| if (vertex == null && nodePattern.getVariable() != null) { | ||
| final Object bound = currentResult.getProperty(nodePattern.getVariable()); | ||
| if (bound instanceof Vertex v) | ||
| vertex = v; | ||
| } |
There was a problem hiding this comment.
The vertex resolution logic should explicitly handle potential conflicts between a forcedVertex (arriving from an edge traversal) and an existing variable binding. If both are present, they must be identical for the path to match. If only the binding is present, it should be used as the anchor for the current step.
Vertex vertex = forcedVertex;
if (nodePattern.getVariable() != null) {
final Object bound = currentResult.getProperty(nodePattern.getVariable());
if (bound instanceof Vertex v) {
if (vertex != null && !vertex.equals(v))
return;
vertex = v;
}
}There was a problem hiding this comment.
Fixed. Changed if (vertex == null && nodePattern.getVariable() != null) to unconditionally check the bound variable. When forcedVertex is non-null and differs from the bound value, the method returns immediately. The terminal-node check already handled the 2-node case; this closes the gap for intermediate nodes in 3+ hop paths. Added preBoundIntermediateNodeIsRespectedDuringPathTraversal which sets up alice->dave->carol and a standalone bob, then MATCH (alice),(bob) MERGE (alice)-[:KNOWS]->(bob)-[:KNOWS]->(carol) - verifying the alice->bob edge is created rather than the dave path being silently accepted.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4005 +/- ##
==========================================
+ Coverage 64.59% 64.79% +0.19%
==========================================
Files 1597 1597
Lines 120048 120049 +1
Branches 25551 25551
==========================================
+ Hits 77545 77785 +240
+ Misses 31743 31492 -251
- Partials 10760 10772 +12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…sh node instead of reusing an existing one Fixes #3998. MergeStep.mergePathAll() was calling findNode() independently for unbound endpoints, which returned any existing node of that label. It then looked for an edge between the pre-resolved nodes, found none, and created the edge to the wrong existing node. The fix replaces the pre-resolution approach with a traversal-based algorithm: traverse edges from bound anchor nodes via DFS to find complete matching paths. Unbound endpoints are matched only after being reached via a qualifying edge. If no complete path exists, createNewPath() creates fresh nodes for every unbound position. Removed now-unused findNode(), findAllEdges(), findEdge(). Five regression tests added in OpenCypherMergeTest$MergeUnboundLabelEndpointRegression. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
bf81905 to
f567f7c
Compare
…ct check, label instanceOf - Fix unbound-anchor edge scan for Direction.BOTH: was treating BOTH as OUT-only, so undirected MERGE patterns never matched edges in the reverse orientation - Add forcedVertex vs pre-bound variable conflict check in traverseFromNode for intermediate nodes in multi-hop paths (terminal node already had this check) - Replace Labels.getLabels(v).containsAll(required) with Labels.hasLabel(v,label) loop in matchesNodePattern: avoids list allocation and correctly handles type inheritance (consistent with iterateType(type,true) semantics elsewhere) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…reates fresh node (ArcadeData#4005)
…skip ci] Bumps [org.postgresql:postgresql](https://github.com/pgjdbc/pgjdbc) from 42.7.10 to 42.7.11. Release notes *Sourced from [org.postgresql:postgresql's releases](https://github.com/pgjdbc/pgjdbc/releases).* > v42.7.11 > -------- > > Security > -------- > > * fix: Limit SCRAM PBKDF2 iterations accepted from the server. > pgjdbc was vulnerable to a client-side denial of service in SCRAM-SHA-256 authentication, where a malicious or compromised PostgreSQL server could specify an extremely large PBKDF2 iteration count, causing the client to consume unbounded CPU and potentially exhaust connection pools. The fix introduces a new scramMaxIterations connection property (defaulting to 100,000) to cap iteration counts before computation begins. > See the [Security Advisory](GHSA-98qh-xjc8-98pq) for more detail. > The following [CVE-2026-42198](https://nvd.nist.gov/vuln/detail/CVE-2026-42198) has been issued. > > Changes > ------- > > * fix: Add sources and javadocs to shaded published lib generation [`@sehrope`](https://github.com/sehrope) ([#4043](https://redirect.github.com/pgjdbc/pgjdbc/issues/4043)) > * update Changelog and website for release of 42.7.11 [`@davecramer`](https://github.com/davecramer) ([#4042](https://redirect.github.com/pgjdbc/pgjdbc/issues/4042)) > * Fix scram fix location in changelog and update published artifact developer list [`@sehrope`](https://github.com/sehrope) ([#4041](https://redirect.github.com/pgjdbc/pgjdbc/issues/4041)) > * Restrict test with scram\_iterations to v16+ and release notes [`@sehrope`](https://github.com/sehrope) ([#4040](https://redirect.github.com/pgjdbc/pgjdbc/issues/4040)) > * chore(deps): update ubuntu:24.04 docker digest to 84e77de [`@renovate-bot`](https://github.com/renovate-bot) ([#4017](https://redirect.github.com/pgjdbc/pgjdbc/issues/4017)) > * test: add tests for QueryExecutor#getTransactionState [`@vlsi`](https://github.com/vlsi) ([#4006](https://redirect.github.com/pgjdbc/pgjdbc/issues/4006)) > * chore(deps): update actions/create-github-app-token action to v2.2.2 [`@renovate-bot`](https://github.com/renovate-bot) ([#3983](https://redirect.github.com/pgjdbc/pgjdbc/issues/3983)) > * fix: fix flaky CopyBothResponseTest by using WAL flush LSN [`@vlsi`](https://github.com/vlsi) ([#3979](https://redirect.github.com/pgjdbc/pgjdbc/issues/3979)) > * fix: fix flaky replication restart tests by waiting for confirmed\_flush\_lsn [`@vlsi`](https://github.com/vlsi) ([#3975](https://redirect.github.com/pgjdbc/pgjdbc/issues/3975)) > * test: fix flaky LogicalReplicationStatusTest by polling pg\_stat\_replication [`@vlsi`](https://github.com/vlsi) ([#3974](https://redirect.github.com/pgjdbc/pgjdbc/issues/3974)) > * chore: replace Appveyor with ikalnytskyi/action-setup-postgres [`@vlsi`](https://github.com/vlsi) ([#3966](https://redirect.github.com/pgjdbc/pgjdbc/issues/3966)) > * test: move test table creation from [`@BeforeEach`](https://github.com/BeforeEach) to [`@BeforeAll`](https://github.com/BeforeAll) [`@vlsi`](https://github.com/vlsi) ([#3967](https://redirect.github.com/pgjdbc/pgjdbc/issues/3967)) > * Return jsonb as PGObject fixes Issue [#3926](https://redirect.github.com/pgjdbc/pgjdbc/issues/3926) [`@davecramer`](https://github.com/davecramer) ([#3956](https://redirect.github.com/pgjdbc/pgjdbc/issues/3956)) > * Update docker scripts [`@davecramer`](https://github.com/davecramer) ([#3958](https://redirect.github.com/pgjdbc/pgjdbc/issues/3958)) > * implement require\_auth, this is pretty much how libpq does this. [`@davecramer`](https://github.com/davecramer) ([#3895](https://redirect.github.com/pgjdbc/pgjdbc/issues/3895)) > * docs: add SCRAM authentication test setup section to TESTING.md [`@emmaeng700`](https://github.com/emmaeng700) ([#3945](https://redirect.github.com/pgjdbc/pgjdbc/issues/3945)) > * Add RequireServerVersion annotation for tests [`@sehrope`](https://github.com/sehrope) ([#3939](https://redirect.github.com/pgjdbc/pgjdbc/issues/3939)) > > 🐛 Bug Fixes > ----------- > > * fix: ensure extended protocol messages end with Sync message [`@vlsi`](https://github.com/vlsi) ([#3728](https://redirect.github.com/pgjdbc/pgjdbc/issues/3728)) > * fix: enable cursor-based fetching in extended protocol when transaction started via SQL command [`@vlsi`](https://github.com/vlsi) ([#3996](https://redirect.github.com/pgjdbc/pgjdbc/issues/3996)) > * fix: retry with SSL on IOException when sslMode=ALLOW [`@vlsi`](https://github.com/vlsi) ([#3973](https://redirect.github.com/pgjdbc/pgjdbc/issues/3973)) > * fix: allow fallback to non-SSL connection when sslMode=prefer and sslResponseTimeout kicks in [`@vlsi`](https://github.com/vlsi) ([#3968](https://redirect.github.com/pgjdbc/pgjdbc/issues/3968)) > * fix: catch SecurityException from setContextClassLoader on ForkJoinPool workers [`@vlsi`](https://github.com/vlsi) ([#3962](https://redirect.github.com/pgjdbc/pgjdbc/issues/3962)) > * fix: use compareTo for LogSequenceNumber comparison [`@vlsi`](https://github.com/vlsi) ([#3961](https://redirect.github.com/pgjdbc/pgjdbc/issues/3961)) > * fix: release COPY lock on IOException to prevent connection hang ([#3957](https://redirect.github.com/pgjdbc/pgjdbc/issues/3957)) [`@vlsi`](https://github.com/vlsi) ([#3960](https://redirect.github.com/pgjdbc/pgjdbc/issues/3960)) > > 🧰 Maintenance > ------------- > > * style: replace [`@exception`](https://github.com/exception) with [`@throws`](https://github.com/throws) in getBoolean javadoc [`@vlsi`](https://github.com/vlsi) ([#4035](https://redirect.github.com/pgjdbc/pgjdbc/issues/4035)) > * chore: use `@vlsi/github-actions-random-matrix` npm package [`@vlsi`](https://github.com/vlsi) ([#4008](https://redirect.github.com/pgjdbc/pgjdbc/issues/4008)) > * chore: use tag names for pinning github actions, pin ikalnytskyi/action-setup-postgres [`@vlsi`](https://github.com/vlsi) ([#4007](https://redirect.github.com/pgjdbc/pgjdbc/issues/4007)) > * chore: bump errorprone to 2.48.0 [`@vlsi`](https://github.com/vlsi) ([#4005](https://redirect.github.com/pgjdbc/pgjdbc/issues/4005)) > * test: add [`@DisableLogger`](https://github.com/DisableLogger) annotation to suppress expected log warnings in tests [`@vlsi`](https://github.com/vlsi) ([#3971](https://redirect.github.com/pgjdbc/pgjdbc/issues/3971)) > * chore: suppress deprecations in test code to reduce build verbosity [`@vlsi`](https://github.com/vlsi) ([#3972](https://redirect.github.com/pgjdbc/pgjdbc/issues/3972)) > * chore: replace log warning in ConnectionFactory.closeStream with Throwable.addSuppressed [`@vlsi`](https://github.com/vlsi) ([#3970](https://redirect.github.com/pgjdbc/pgjdbc/issues/3970)) > * chore: use greedy pairwise coverage for CI matrix generation [`@vlsi`](https://github.com/vlsi) ([#3965](https://redirect.github.com/pgjdbc/pgjdbc/issues/3965)) > * chore: use full version tags in GitHub Actions comments [`@vlsi`](https://github.com/vlsi) ([#3963](https://redirect.github.com/pgjdbc/pgjdbc/issues/3963)) > > ⬆️ Dependencies > --------------- ... (truncated) Changelog *Sourced from [org.postgresql:postgresql's changelog](https://github.com/pgjdbc/pgjdbc/blob/master/CHANGELOG.md).* > [42.7.11] (2026-04-28) > ---------------------- > > ### Security > > * fix: Limit SCRAM PBKDF2 iterations accepted from the server. > pgjdbc was vulnerable to a client-side denial of service in SCRAM-SHA-256 authentication, where a malicious or compromised PostgreSQL server could specify an extremely large PBKDF2 iteration count, causing the client to consume unbounded CPU and potentially exhaust connection pools. The fix introduces a new scramMaxIterations connection property (defaulting to 100,000) to cap iteration counts before computation begins. > See the [Security Advisory](GHSA-98qh-xjc8-98pq) for more detail. > The following [CVE-2026-42198](https://nvd.nist.gov/vuln/detail/CVE-2026-42198) has been issued. > > ### Added > > * feat: implement require\_auth connection property, aligning with libpq behavior [PR [#3895](https://redirect.github.com/pgjdbc/pgjdbc/issues/3895)]([pgjdbc/pgjdbc#3895](https://redirect.github.com/pgjdbc/pgjdbc/pull/3895)) > > ### Changed > > * chore: replace Appveyor CI with ikalnytskyi/action-setup-postgres [PR [#3966](https://redirect.github.com/pgjdbc/pgjdbc/issues/3966)]([pgjdbc/pgjdbc#3966](https://redirect.github.com/pgjdbc/pgjdbc/pull/3966)) > * chore: upgrade Gradle to v9 [PR [#3978](https://redirect.github.com/pgjdbc/pgjdbc/issues/3978)]([pgjdbc/pgjdbc#3978](https://redirect.github.com/pgjdbc/pgjdbc/pull/3978)) > > ### Fixed > > * fix: ensure extended protocol messages end with Sync message [PR [#3728](https://redirect.github.com/pgjdbc/pgjdbc/issues/3728)]([pgjdbc/pgjdbc#3728](https://redirect.github.com/pgjdbc/pgjdbc/pull/3728)) > * fix: enable cursor-based fetching in extended protocol when transaction started via SQL command [PR [#3996](https://redirect.github.com/pgjdbc/pgjdbc/issues/3996)]([pgjdbc/pgjdbc#3996](https://redirect.github.com/pgjdbc/pgjdbc/pull/3996)) > * fix: retry with SSL on IOException when sslMode=ALLOW [PR [#3973](https://redirect.github.com/pgjdbc/pgjdbc/issues/3973)]([pgjdbc/pgjdbc#3973](https://redirect.github.com/pgjdbc/pgjdbc/pull/3973)) > * fix: make sure the driver honours connectTimeout when retrying the connection [PR [#3968](https://redirect.github.com/pgjdbc/pgjdbc/issues/3968)]([pgjdbc/pgjdbc#3968](https://redirect.github.com/pgjdbc/pgjdbc/pull/3968)) > * fix: allow fallback to non-SSL connection when sslMode=prefer and sslResponseTimeout kicks in [PR [#3968](https://redirect.github.com/pgjdbc/pgjdbc/issues/3968)]([pgjdbc/pgjdbc#3968](https://redirect.github.com/pgjdbc/pgjdbc/pull/3968)) > * fix: catch SecurityException from setContextClassLoader on ForkJoinPool workers [PR [#3962](https://redirect.github.com/pgjdbc/pgjdbc/issues/3962)]([pgjdbc/pgjdbc#3962](https://redirect.github.com/pgjdbc/pgjdbc/pull/3962)) > * fix: use compareTo for LogSequenceNumber comparison to handle unsigned values correctly [PR [#3961](https://redirect.github.com/pgjdbc/pgjdbc/issues/3961)]([pgjdbc/pgjdbc#3961](https://redirect.github.com/pgjdbc/pgjdbc/pull/3961)) > * fix: release COPY lock on IOException to prevent connection hang [PR [#3957](https://redirect.github.com/pgjdbc/pgjdbc/issues/3957)]([pgjdbc/pgjdbc#3957](https://redirect.github.com/pgjdbc/pgjdbc/pull/3957)) > * fix: return jsonb as PGObject instead of String [PR [#3956](https://redirect.github.com/pgjdbc/pgjdbc/issues/3956)]([pgjdbc/pgjdbc#3956](https://redirect.github.com/pgjdbc/pgjdbc/pull/3956)) > * fix: align SSL key file permission check with libpq [PR [#3952](https://redirect.github.com/pgjdbc/pgjdbc/issues/3952)]([pgjdbc/pgjdbc#3952](https://redirect.github.com/pgjdbc/pgjdbc/pull/3952)) > * fix: guard connection closed flag with a reentrant lock to protect against concurrent close [PR [#3905](https://redirect.github.com/pgjdbc/pgjdbc/issues/3905)]([pgjdbc/pgjdbc#3905](https://redirect.github.com/pgjdbc/pgjdbc/pull/3905)) Commits * [`78e261f`](pgjdbc/pgjdbc@78e261f) fix: Add sources and javadocs to shaded published lib generation * [`1e09fa0`](pgjdbc/pgjdbc@1e09fa0) update Changelog and website for release of 42.7.11 ([#4042](https://redirect.github.com/pgjdbc/pgjdbc/issues/4042)) * [`d479fa5`](pgjdbc/pgjdbc@d479fa5) Fix scram fix location in changelog and update published artifact developer l... * [`b04fc46`](pgjdbc/pgjdbc@b04fc46) docs: Add scram max iters fix to changelog * [`cf54822`](pgjdbc/pgjdbc@cf54822) test: Disable scram test on older version without scram\_iterations GUC * [`7dbcc79`](pgjdbc/pgjdbc@7dbcc79) test: Add SCRAM max iteration tests * [`c9d41d1`](pgjdbc/pgjdbc@c9d41d1) fix: Limit SCRAM PBKDF2 iterations accepted from the server * [`a340cb2`](pgjdbc/pgjdbc@a340cb2) style: replace [`@exception`](https://github.com/exception) with [`@throws`](https://github.com/throws) in getBoolean javadoc * [`77837f8`](pgjdbc/pgjdbc@77837f8) fix(deps): update dependency org.openrewrite.rewrite:org.openrewrite.rewrite.... * [`23af03b`](pgjdbc/pgjdbc@23af03b) chore(deps): update actions/checkout action to v6 * Additional commits viewable in [compare view](pgjdbc/pgjdbc@REL42.7.10...REL42.7.11) [](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
Fixes #3998.
MERGE (p)-[r:WORKS_AT {since: 2020}]->(c:Company)withcunbound incorrectly reused an existingCompanynode instead of creating a new one, contradicting Neo4j semantics.Root cause:
MergeStep.mergePathAll()calledfindNode()independently for each unbound endpoint, which returned the first existing node of that label. It then searched for an edge between the pre-resolved anchor and that node, found none, and created the edge to the wrong existing node.Fix: Replaced the pre-resolution approach with a traversal-based algorithm:
findAllMatchingPaths()starts from any pre-bound anchor node and uses DFS (traverseFromNode+traverseEdgesFromVertex) to find complete matching paths by edge traversal.createNewPath()creates fresh nodes for every unbound position when no complete path is found.findNode(),findAllEdges(),findEdge().Test plan
unboundLabelOnlyEndpointCreatesNewNodeWhenSameLabelExists- main bug case: existing Company must not be reusedunboundLabelOnlyEndpointCreatesNodeWhenNoSameLabelExists- control: no prior node, creates oneexplicitlyBoundEndpointStillReusesExistingNode- control: bound via MATCH, must reuseunboundLabelOnlyEndpointCreatesNewNodeWithMultipleExistingSameLabel- two existing Companies, creates a thirdsecondMergeReusesCreatedPathAndDoesNotCreateAnotherNode- idempotencyOpenCypherMergeActionsTestandMatchIdThenMergeRelationshipRegression)OpenCypher*test suite passes (pre-existing TCK Match4 failure is unrelated)🤖 Generated with Claude Code
EOF
)