[bugfix] PathExpr: iterate atomic, context-independent RHS per-item (closes #798)

[joewiz]

[This response was co-authored with Claude Code. -Joe]

Summary

XPath 3.1 §3.3.5 (Path Operator) requires that in E1/E2, the right-hand step E2 is evaluated once for each item produced by E1 — even when E2 doesn't reference the context. eXist's PathExpr.eval short-circuited that iteration for context-independent atomic-returning right-hand sides over persistent inputs, collapsing the multiplicity.

The bug has been open since 2015 (#798); @line-o confirmed it still reproduces on develop earlier today.

Closes #798.

Reproducer (from #798, restated by @line-o in the latest comment)

let $data := <a><b/><b/></a>
let $doc := xmldb:store('/db', 'test.xml', $data)
return [
    $data//b/3,
    doc('/db/test.xml')//b/3
]

Before this PR: [(3, 3), 3] — in-memory iterates per item (2 results), persistent collapses (1 result).
After this PR: [(3, 3), (3, 3)] — both forms iterate, multiplicity preserved.

Root cause

PathExpr.eval (~line 261) chooses between per-item iteration of E2 and a single evaluation. In-memory inputs always iterate (inMemProcessing flag). Persistent inputs only iterate when E2 declares a CONTEXT_ITEM or CONTEXT_POSITION dependency.

The single-eval shortcut is sound when E2 returns nodes: the post-step result.removeDuplicates() at line ~313 absorbs the missing multiplicity, since each iteration would produce the same node-set. It's not sound when E2 returns atomics — there's no de-duplication step, the literal value is the same every iteration, and the missing iterations are exactly what carries the multiplicity required by §3.3.5.

Fix

Three guards bolted on to the iterate/single-eval condition. Each one was needed because a broader form broke real callers — the test suite drove the design.

+ // XPath 3.1 §3.3.5: E2 in E1/E2 must evaluate per item of E1. The else-branch
+ // shortcut is only sound for node-returning E2 (where removeDuplicates() absorbs
+ // missing iterations). For atomic-returning, context-independent E2 we must
+ // force iteration to preserve multiplicity. See #798.
+ final boolean stepReturnsNonNode = !Type.subTypeOf(expr.returnsType(), Type.NODE);
+ final boolean stepIsContextIndependent =
+         !Dependency.dependsOn(exprDeps, Dependency.CONTEXT_ITEM)
+         && !Dependency.dependsOn(exprDeps, Dependency.CONTEXT_POSITION)
+         && !Dependency.dependsOn(exprDeps, Dependency.CONTEXT_SET);
+ final boolean atomicRhsMustIterate = stepReturnsNonNode
+         && stepIsContextIndependent
+         && stepIdx > 0
+         && currentContext != null && currentContext.hasMany();
- if (inMemProcessing ||
+ if (inMemProcessing || atomicRhsMustIterate ||

Guard	Why it's needed
stepReturnsNonNode	Node-axis RHS keeps the persistent fast-path; only atomic RHS needs forced iteration.
stepIsContextIndependent (no CONTEXT_ITEM / CONTEXT_POSITION / CONTEXT_SET)	Context-dependent atomic steps already take the existing iterate branch, or — in the index-optimised Predicate.selectByNodeSet path — are intentionally single-evaluated against the full node-set. Forcing iteration there broke regex/date-predicate / value-index optimisations in mvn test.
stepIdx > 0	Only applies to RHS positions. Without this guard, a bare atomic expression wrapped in a single-step PathExpr (e.g. the literal pattern arg of matches(SPEAKER, '^HAM.*')) would be iterated over the surrounding node-set and produce a many-item arg where one was expected.

Test plan

New regression test (PathExprAtomicRhsTest, 4 cases):

• inMemoryAtomicRhsIteratesPerItem — baseline ((<a><b/><b/></a>)//b/3 → 2 items)
• persistentAtomicRhsIteratesPerItem — the bug (doc('…')//b/3 → 2 items, was 1)
• inMemoryAndPersistentAgree — sequence equality (both "3,3")
• nodeRhsStillDedupes — sanity that //b/.. still dedups to 1 (no regression in node-axis fast-path)

All 4 pass. The first 3 fail on develop.

xquery.xquery3.XQuery3Tests: 1030/1030 pass (1 pre-existing skip).

Full mvn test -pl exist-core on this branch: 3 failures — all unrelated infrastructure flakes confirmed by inspecting each stack:

• GetXMLResourceNoLockTest — Failed to bind to 0.0.0.0:8088 (another Docker container is holding the port on this machine)
• RecoverBinary2Test.storeAndRead — Collection /db/test/test2 should exist after store(), a long-running flaky storage test
• EvalWebSocketEndpointTest.cancellation — 30s WebSocket cancellation timeout
  None of the three touch PathExpr or path semantics; the same three fail without this fix on the same machine.

Full XQTS HEAD before/after (exist-xqts-runner --xqts-version HEAD, i.e. the live qt3tests master / final 3.1 Rec + errata — not the older --xqts-version 3.1 archive). Per-test comparison on the 26,014 tests both runs actually measured (excluding tests dropped by batch-runner timeout):

              develop    fix    delta
pass           23405    23405      +0
fail            1427     1427      +0
error            128      128      +0
skip            1054     1054      +0

Zero per-test transitions. The aggregate headlines differ (23,437 → 23,666 pass) only because different test sets hit the batched-runner timeout each run — that's the same measurement noise we've seen in earlier XQTS comparisons.

Related

• Closes different path result from in-memory vs stored #798
• Spec: XPath 3.1 §3.3.5 (Path Operator: "For each item in the result of evaluating E1, E2 is evaluated …")
• Distinct from [bugfix] RootNode: declare CONTEXT_ITEM dependency #6409 (RootNode CONTEXT_ITEM dependency, which was about a node-axis falsely claiming context-independence). This one is about the optimizer's persistent fast-path being wrong for atomic-RHS regardless of dependency.

[line-o]

Thank you!

[duncdrum]

I like the Design instruction 😉