Optimize IN-list anchors for bounded variable-length Cypher paths

[justinblethrow-cloud]

Summary

• allow the bounded variable-length path bridge introduced in Use indexed anchors for bounded variable-length Cypher paths #5357 to accept indexed IN-list target anchors
• start the reversed traversal with IndexSeekStep, reusing the existing multi-value seek and runtime parameter handling
• add regression coverage for both the index-backed plan and existing bound-target reversal without a physical optimizer plan

This preserves the existing bridge safety gates: read-only execution, one bounded and typed bidirectional relationship, a single-property index, and no shape-changing clauses before MATCH.

The indexed-anchor reversal is tracked separately from the existing reversal of ordinary single-hop paths whose target is already bound, so fallback execution does not require a physical optimizer plan.

Motivation

The bridge currently rejects InListValues, so a query such as:

MATCH (place:Area)-[:PART_OF*0..3]->(country:Area)
WHERE country.id IN $countryIds
RETURN country.id, place.id

cannot start from the selective indexed target even though NodeIndexSeek already supports literal and parameterized IN lists. This patch connects that existing operator to the traditional variable-length executor.

Part of #5358.

Verification

./mvnw -pl engine -Dtest=CypherVariableLengthAnchorSelectionTest,CypherMultiHopInListIssue5306Test,OpenCypherVariableLengthPathTest,Issue5362DuplicatePredicateIndexSeekTest,OpenCypherOptimizerVerificationTest,AnchorSelectorTest,CypherOptimizerIntegrationTest,CountEdgesOptimizationTest,StreamingAggregationTest,CypherLabelFilteringTest,OpenCypherOptionalMatchTest test

Result: 128 tests run, 0 failures, 0 errors, 0 skipped.

./mvnw -pl engine -Dtest=com.arcadedb.query.opencypher.tck.OpenCypherTCKSuite test

Result: 3,897 scenarios run, 0 failures, 0 errors, 85 expected skips.

[gemini-code-assist]

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

[mergify]

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

• Queue this pull request

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

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

[lvca]

@claude please review this PR

[claude]

Claude encountered an error after 3m 3s —— View job

---

Reviewing PR #5387

• Gather context (diff, related code from Use indexed anchors for bounded variable-length Cypher paths #5357 bridge)
• Understand the request (code review only)
• Review CypherExecutionPlan.java changes
• Review new regression test
• Post review feedback

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.72%. Comparing base (df514e1) to head (bdc6b57).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #5387      +/-   ##
============================================
- Coverage     66.33%   65.72%   -0.62%     
- Complexity        0     1044    +1044     
============================================
  Files          1723     1723              
  Lines        141865   141876      +11     
  Branches      30357    30359       +2     
============================================
- Hits          94113    93244     -869     
- Misses        35017    36034    +1017     
+ Partials      12735    12598     -137     

☔ View full report in Codecov by Harness.
📢 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.

[lvca]

@claude please review this PR

[claude]

Claude finished @lvca's task in 10m 22s —— View job

---

Reviewing PR #5387

• Gather context (diff, related code from Use indexed anchors for bounded variable-length Cypher paths #5357 bridge)
• Understand the request (code review only)
• Review CypherExecutionPlan.java changes
• Review new regression test
• Post review feedback

Note: This environment has no network/build access for me in this run, so I couldn't execute mvn/tests — this is a static-analysis review only. The reasoning below is based on tracing the code paths, please verify with the repro query.

Summary

The change is well-scoped: it reuses NodeIndexSeek's existing multi-value seek machinery via a new IndexSeekStep, and the guard method correctly restricts the bridge to read-only, single-MATCH, single-relationship, bounded, typed, bidirectional patterns. The second commit's fix (decoupling reversedFromIndexedAnchor from the generic bound-target reversed flag) is correct and necessary — without it, the bound-target reversal path at CypherExecutionPlan.java:1577-1595 would also hit the IndexSeekStep branch and crash/misbehave since physicalPlan.getAnchor() doesn't correspond to that case.

Correctness issue: inline properties/labels on the anchor node are silently dropped

CypherExecutionPlan.java:1607-1613:

if (reversedFromIndexedAnchor && physicalPlan.getAnchor().getPropertyValue() instanceof InListValues) {
  final var anchor = physicalPlan.getAnchor();
  sourceStep = new IndexSeekStep(anchor.getVariable(), anchor.getIndex().getTypeName(),
      anchor.getPropertyName(), anchor.getPropertyValue(), anchor.getIndex().getIndexName(),
      anchor.getEstimatedCost(), anchor.getEstimatedCardinality(), context);
} else
  sourceStep = new MatchNodeStep(sourceVar, sourceNode, context, sourceIdFilter, sourcePushdown);

IndexSeekStep/NodeIndexSeek (engine/.../executor/operators/NodeIndexSeek.java:73-148) only checks the single indexed property against the IN-list values. Unlike MatchNodeStep, it never evaluates NodePattern.getProperties() (inline {...} property map) or extra/dynamic labels on that node (MatchNodeStep.java:943-948).

Previously, shouldReverseVariableLengthPathFromIndexedAnchor (CypherExecutionPlan.java:1798-1820) excluded InListValues entirely, so every reversed-anchor case still went through MatchNodeStep, which validates the full NodePattern. This PR adds a new branch that bypasses that validation for the IN-list case specifically, and the guard method wasn't updated to compensate — it still has no check on pathPattern.getLastNode()'s inline properties/labels.

Also, the final FilterPropertiesStep (CypherExecutionPlan.java:1761-1774) only re-evaluates matchClause.getWhereClause(); it never inspects NodePattern inline properties, so nothing downstream catches the gap either.

Repro:

MATCH (place:Area)-[:PART_OF*0..3]->(country:Area {region: 'EU'})
WHERE country.id IN $countryIds
RETURN country, place

If AnchorSelector picks country.id as the IN-list anchor (plausible when region isn't indexed — see AnchorSelector.java:126-204, which merges inline properties into allPredicates but falls through to the IN-list branch when no index covers them), the inline region: 'EU' filter is silently ignored, and the query returns rows for countries matching the id list regardless of region.

Suggested fix: in shouldReverseVariableLengthPathFromIndexedAnchor, also require that pathPattern.getLastNode() has no explicit inline properties and no extra/dynamic labels beyond the indexed type before taking the IndexSeekStep path — otherwise keep falling back to MatchNodeStep. Worth a regression test in CypherVariableLengthAnchorSelectionTest.java combining an inline property with the IN-list WHERE filter on the anchor node.

Fix this →

Minor

• CypherExecutionPlan.java:1603-1605 — sourceIdFilter/sourcePushdown are computed unconditionally even on the IndexSeekStep branch where they're unused. Harmless (cheap AST walk), but could be moved into the else branch for clarity.

Not an issue

• The regression tests added are good and pass per the PR description; the "2 rows" / index-name assertions match IndexSeekStep.prettyPrint.
• IndexSeekStep/NodeIndexSeek correctly dedupes RIDs across IN-list values and resolves parameters at runtime.

[justinblethrow-cloud]

Thanks, addressed in ca21bc5.

I confirmed that both reported constraint shapes already fall back to traditional execution under the current optimizer eligibility checks: inline node properties and multi-label relationship endpoints are rejected before a physical plan is built. I nevertheless added the safety check at the indexed IN-list bridge itself so future optimizer expansion cannot make the shortcut bypass those constraints.

The shortcut now requires an anchor with no inline/parameter properties, no dynamic labels, and exactly one static label matching the indexed type. Added focused regressions for an inline property and an additional label; both verify correct results and fallback plan selection.

Verification: ./mvnw -pl engine -Dtest=CypherVariableLengthAnchorSelectionTest test — 8 tests passed, 0 failures/errors.

[lvca]

Really great PR, merging it! Thanks!