Skip to content

[bugfix] RootNode: declare CONTEXT_ITEM dependency#6409

Merged
line-o merged 2 commits into
eXist-db:developfrom
joewiz:bugfix/rootnode-context-item-dependency
May 30, 2026
Merged

[bugfix] RootNode: declare CONTEXT_ITEM dependency#6409
line-o merged 2 commits into
eXist-db:developfrom
joewiz:bugfix/rootnode-context-item-dependency

Conversation

@joewiz
Copy link
Copy Markdown
Member

@joewiz joewiz commented May 29, 2026

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

Summary

A one-line dependency-declaration fix in RootNode (the lone-slash root step /), plus a regression test. Without this, the optimizer's predicate hoister treats predicates whose only context-dependent leaf is / as context-independent and evaluates them once outside the iteration. With no context item to resolve against, RootNode.eval falls through to the static-context default branch (getStaticallyKnownDocuments()), which returns every XML resource the broker can read. Under admin that includes content from /db/system/security/, which then atomizes to multiple untypedAtomics and breaks arithmetic in the predicate.

Surfaced while preparing the exist-xqts-runner "assertion fixes only" PR (eXist-db/exist-xqts-runner#61), where commit 5 changes the runner's embedded server connection from getBroker() to authenticate("admin", "") — that change is a prerequisite for the in-flight EXPath File built-in PR (#6257). The runner change unblocks #6257; this fix removes the seven XQTS-HEAD regressions admin-context introduced.

The fix

+    @Override
+    public int getDependencies() {
+        // Declare CONTEXT_ITEM so the optimizer does not hoist predicates
+        // containing only / out of their iteration context.
+        return Dependency.CONTEXT_ITEM | Dependency.CONTEXT_SET;
+    }

RootNode extends Step extends AbstractExpression, which returns Dependency.DEFAULT_DEPENDENCIES = CONTEXT_SET. The default isn't enough: the optimizer's predicate hoister looks at CONTEXT_ITEM specifically to decide whether a sub-expression can be evaluated once outside the iteration. / evaluation depends on the context item to resolve the owner document, so the override is the correct dependency declaration.

RootNode.eval is unchanged. Top-level / in queries with no fn:doc() still falls through to the static-context default in its no-context cases, so the long-standing "absolute path without fn:doc" eXist convention is preserved for legitimate uses (e.g. //foo at the top of a query).

What it fixes

Per a measured before/after on XQTS HEAD against current develop, with two builds of exist-xqts-runner that differ only in broker authentication (guest vs admin):

Test set Test case Without fix (admin) With fix (admin)
prod-PathExpr PathExpr-1 XPTY0004 "Too many operands at the right of *" pass
prod-PathExpr PathExpr-2 XPTY0004 pass
prod-PathExpr PathExpr-15 XPTY0004 pass
prod-StepExpr Steps-leading-lone-slash-15 XPTY0004 pass
prod-StepExpr Steps-leading-lone-slash-17 XPTY0004 pass
prod-StepExpr Steps-leading-lone-slash-1a XPTY0004 pass

All six are "leading lone slash" XPath syntax-constraint tests from Nicolae Brinza's 2009 contributions, of the shape fn:count(.[5 * /]) over a single-element document. The expected behavior per the spec, with the bid element as context item, is: / resolves to its owner document (single item), arithmetic produces a single numeric value, the positional predicate has no match, count is zero.

These tests pass under guest only because the static-context fallback returns the empty sequence there (guest can't read system collections). The fix removes the dependency on that accidental coincidence.

The diagnosis trail

Instrumenting OpNumeric.eval with one System.err.println of the operands made the issue obvious:

Admin: op=* user=admin lcount=1 rcount=3 ctxSeq=null ctxItem=null
       rseq[0] type=xs:untypedAtomic val=/authentication/login
       rseq[1] type=xs:untypedAtomic val={RIPEMD160}<hash>
       rseq[2] type=xs:untypedAtomic val={RIPEMD160}<hash>

Guest: op=* user=guest lcount=1 rcount=0 ctxSeq=null ctxItem=null

ctxItem=null in both cases is the smoking gun: the predicate isn't being iterated per-item. The 3-item rseq under admin is admin.xml atomized through the static-doc fallback. The cause is the missing CONTEXT_ITEM dependency on RootNode.

The "all documents in scope under admin" behavior of RootNode.eval is a vendor-specific static-context default that the XPath 3.1 spec permits (§2.1.1 — static context can define a default context item). What's not permitted is the optimizer mistaking a context-dependent expression for a context-independent one; that's the actual bug.

Test plan

  • New JUnit regression test RootNodeContextItemDependencyTest covers three positive cases that all pass with the fix and (2 of 3) error without it. The test uses an in-memory <bid>23</bid> document bound via let $ctx := document {...}/bid and exercises fn:count($ctx[5 * /]), fn:count($ctx[(/) * 5]), and fn:count($ctx[fn:count(/) eq 1]).
  • mvn test -pl exist-core — 6,592 tests, 0 failures, 0 errors, 106 skipped, 3:39 wall.
  • XQTS HEAD with the runner's admin-auth change applied: net effect is +6 passes (the six lone-slash tests above flipping from fail to pass), 0 regressions.

Related

RootNode (the lone-slash root step '/') inherited the default
Dependency.CONTEXT_SET from AbstractExpression and never declared
CONTEXT_ITEM. The predicate-hoisting path of the optimizer uses
CONTEXT_ITEM to decide whether a sub-expression can be evaluated once
outside the iteration. Without that bit, predicates whose only
context-dependent leaf is '/' get hoisted out and evaluated with no
context item.

When that happens, RootNode.eval falls through to the static-context
default branch — getStaticallyKnownDocuments() with a null
staticDocumentPaths, which returns every XML resource the broker can
see. Under guest that filters down to the empty set; under admin the
sequence includes the admin user's own account record from
/db/system/security/exist/accounts/. An expression like
fn:count(.[5 * /]) then throws XPTY0004 ("Too many operands at the
right of *") because the multi-item result of '/' atomizes to multiple
untypedAtomic values.

Surfaces in W3C qt3tests prod-PathExpr/PathExpr-1, -2, -15 and
prod-StepExpr/Steps-leading-lone-slash-15, -17, -1a when measured
under an admin broker — they pass under guest only because the
fallback returns the empty sequence there.

Adding CONTEXT_ITEM keeps the per-iteration context flowing through to
the predicate. RootNode.eval is unchanged; top-level '/' still falls
through to the static-context default in its no-context cases, so the
long-standing 'absolute path without fn:doc' convention is preserved
for legitimate uses.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Comment thread exist-core/src/test/java/org/exist/xquery/RootNodeContextItemDependencyTest.java Outdated
Comment thread exist-core/src/test/java/org/exist/xquery/RootNodeContextItemDependencyTest.java Outdated
Comment thread exist-core/src/test/java/org/exist/xquery/RootNodeContextItemDependencyTest.java Outdated
PMD JUnit naming convention requires [a-z][a-zA-Z0-9]* — no underscores.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@joewiz
Copy link
Copy Markdown
Member Author

joewiz commented May 29, 2026

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

@reinhapa Thanks Patrick — renamed all three test methods to camelCase in 7eeb631.

@line-o line-o requested review from a team and reinhapa May 29, 2026 18:39
@line-o line-o added this to Wave 2 and v7.0.0 May 30, 2026
@github-project-automation github-project-automation Bot moved this to Todo in Wave 2 May 30, 2026
@line-o line-o added the xquery issue is related to xquery implementation label May 30, 2026
@line-o line-o moved this from Todo to In progress in Wave 2 May 30, 2026
@line-o line-o merged commit 7fb7dd9 into eXist-db:develop May 30, 2026
9 checks passed
@github-project-automation github-project-automation Bot moved this to Done in v7.0.0 May 30, 2026
@github-project-automation github-project-automation Bot moved this from In progress to Done in Wave 2 May 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

xquery issue is related to xquery implementation

Projects

Status: Done
Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants