#4006 fix(cypher): VLP segments must not re-traverse a relationship bound in a prior MATCH

[robfrank]

$(cat <<'EOF'

Summary

Fixes #4006.

OpenCypher path isomorphism applies within a single path, not a single MATCH clause. When a relationship variable r is bound in one MATCH and explicitly co-participates in a path pattern with variable-length segments in a subsequent MATCH, those segments must not re-traverse r.

Before: MATCH ()-[r:EDGE]-() MATCH p = (n)-[*0..1]-()-[r]-()-[*0..1]-(m) RETURN count(p) returned 52
After: returns 32 (correct per TCK Match4 [7])

Root Cause

ExpandPathStep.hasEdgeConflict() skipped conflict checking for all variables listed in previousStepVariables (a snapshot of variables bound by prior MATCH clauses). This was too broad: a named relationship that is also an explicit node in the current path is a same-path co-participant and must still be excluded from variable-length traversal.

Fix

Added computePrevVarsForVlp(pathPattern, vlpHopIndex, boundVariables) in CypherExecutionPlan. It copies boundVariables then removes any named relationship variables appearing in other hops of the same path pattern. The resulting set is passed as previousStepVariables to ExpandPathStep, so co-participants remain subject to edge conflict checking.

Applied at both VLP step creation sites (primary optimizer path and legacy path).

Test Plan

• New regression test Issue4006BoundRelVarPathIsomorphismTest - 2 methods
  • vlpSegmentsDoNotReuseExplicitlyNamedBoundRelationship - verifies count = 32
  • unboundedVlpIsNotBlockedByUnrelatedPreviouslyBoundRel - regression guard for Relationship variable may be lost after a later variable-length match #3999
• All related traversal tests pass (45 tests)
• Full Cypher suite passes (5891 tests, 0 failures)

🤖 Generated with Claude Code
EOF

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Coverage 90.00% diff coverage · -8.66% coverage variation

Metric	Results
Coverage variation	✅ -8.66% coverage variation
Diff coverage	✅ 90.00% diff coverage

View coverage diff in Codacy

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (7779412)	120040	88683	73.88%
Head commit (2faf3f4)	151287 (+31247)	98668 (+9985)	65.22% (-8.66%)

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 (#4008)	10	9	90.00%

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.}

[gemini-code-assist]

Code Review

This pull request addresses OpenCypher path isomorphism issues by introducing the computePrevVarsForVlp method to manage relationship variable exclusions during variable-length path expansion, along with corresponding regression tests. Feedback suggests that the implementation should be broadened to consider all path patterns within a MATCH clause to fully comply with relationship isomorphism rules, and specific code suggestions were provided to update the method signature and its call sites.

[gemini-code-assist]

The current implementation of computePrevVarsForVlp only considers relationship variables within the same PathPattern. However, OpenCypher relationship isomorphism applies to the entire MATCH clause (the whole pattern). If a relationship variable was bound in a prior MATCH and is explicitly referenced in a different comma-separated path pattern within the current MATCH clause, it should still be subject to conflict checking for the variable-length segments.

Consider updating this method to iterate over all path patterns in the current MatchClause to ensure full compliance with relationship isomorphism rules when a bound variable is reused anywhere in the current MATCH scope.

  /**
   * Computes the set of "previously bound" variables to pass to {@link ExpandPathStep} for a
   * variable-length hop at {@code vlpHopIndex} within {@code currentPathPattern} of the
   * given {@code matchClause}.
   * <p>
   * OpenCypher relationship isomorphism applies within a single pattern (MATCH clause).
   * A relationship variable bound by a prior MATCH that is also explicitly named in the current
   * MATCH clause is a co-participant and must be checked for edge conflicts even though
   * it was introduced before this MATCH. We therefore remove those co-participants from the
   * exclusion set before handing it to ExpandPathStep.
   */
  private static Set<String> computePrevVarsForVlp(final MatchClause matchClause, final PathPattern currentPathPattern, final int vlpHopIndex,
      final Set<String> boundVariables) {
    final Set<String> prevVars = new HashSet<>(boundVariables);
    for (final PathPattern pathPattern : matchClause.getPathPatterns()) {
      for (int j = 0; j < pathPattern.getRelationshipCount(); j++) {
        if (pathPattern == currentPathPattern && j == vlpHopIndex)
          continue;
        final RelationshipPattern rel = pathPattern.getRelationship(j);
        final String var = rel.getVariable();
        if (var != null && !var.isEmpty())
          prevVars.remove(var);
      }
    }
    return prevVars;
  }

[robfrank]

The suggestion extends the fix beyond what the OpenCypher spec requires.

Path isomorphism applies within a single path pattern, not across comma-separated patterns within a MATCH clause. The issue description itself states this explicitly:

"OpenCypher's path isomorphism rule, which applies within a single path, not within a single MATCH clause."

TCK Match4 [7] (the canonical test for this fix) uses a single path pattern:

MATCH p = (n)-[*0..1]-()-[r]-()-[*0..1]-(m)

The rule is: if r is an explicit hop in the same path pattern as the VLP, the VLP must not re-traverse it. If r appears in a different comma-separated sub-pattern, they are independent patterns and the spec does not forbid the VLP from traversing the same edge.

Broadening to the full MatchClause would impose a cross-path uniqueness constraint that the spec does not mandate, and would incorrectly restrict valid queries. Keeping the scope to the current PathPattern is the correct and spec-compliant implementation.

[gemini-code-assist]

Update the call to computePrevVarsForVlp to pass the matchClause if the suggested change to the method signature is adopted.

Suggested change

	true, effectiveTargetNode, pathPattern.getEffectivePathMode(), computePrevVarsForVlp(pathPattern, i, boundVariables), context);
	true, effectiveTargetNode, pathPattern.getEffectivePathMode(), computePrevVarsForVlp(matchClause, pathPattern, i, boundVariables), context);

[gemini-code-assist]

Update the call to computePrevVarsForVlp to pass the matchClause if the suggested change to the method signature is adopted.

Suggested change

	targetNode, pathPattern.getEffectivePathMode(), computePrevVarsForVlp(pathPattern, i, legacyBoundVariables), context);
	targetNode, pathPattern.getEffectivePathMode(), computePrevVarsForVlp(matchClause, pathPattern, i, legacyBoundVariables), context);

[codecov]

Codecov Report

❌ Patch coverage is 80.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.18%. Comparing base (7779412) to head (2faf3f4).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
...query/opencypher/executor/CypherExecutionPlan.java	80.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4008      +/-   ##
==========================================
- Coverage   64.89%   64.18%   -0.72%     
==========================================
  Files        1597     1597              
  Lines      120040   120048       +8     
  Branches    25548    25551       +3     
==========================================
- Hits        77903    77054     -849     
- Misses      31387    32338     +951     
+ Partials    10750    10656      -94     

☔ 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.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.