diff --git a/exist-core/src/main/java/org/exist/xquery/PathExpr.java b/exist-core/src/main/java/org/exist/xquery/PathExpr.java index f9d7d34115f..7c34fc0ead4 100644 --- a/exist-core/src/main/java/org/exist/xquery/PathExpr.java +++ b/exist-core/src/main/java/org/exist/xquery/PathExpr.java @@ -259,7 +259,38 @@ public Sequence eval(Sequence contextSequence, final Item contextItem) throws XP !currentContext.isPersistentSet(); //DESIGN : first test the dependency then the result final int exprDeps = expr.getDependencies(); - if (inMemProcessing || + // XPath 3.1 §3.3.5 (Path Operator) requires E2 in E1/E2 to be evaluated + // for each item in E1's result. The "single eval" else-branch below is a + // performance shortcut that only preserves the correct multiplicity when + // E2 returns nodes (the subsequent removeDuplicates() call absorbs any + // missing iterations). When E2 is a context-INdependent step that + // returns atomic values, the shortcut collapses the result — every + // iteration of E1 would produce the same atomic value, so the missing + // iterations matter. See #798. + // + // This is narrowly the "atomic literal RHS" case (`//b/3`, + // `//x/'name'`, etc.). Context-dependent atomic steps already + // declare CONTEXT_ITEM/POSITION/SET and take the existing iterate + // branch — or, in the index-optimised Predicate.selectByNodeSet + // path, are intentionally evaluated once against the full node-set. + // We must not force iteration in those cases. + // + // Restricted to non-first steps: the first step is the path's + // starting point, not a "RHS of /". This also keeps us out of the + // function-argument-wrapper case, where a single-step PathExpr + // wraps an atomic-returning argument (e.g. the literal pattern + // `'^HAM.*'` of matches() inside a predicate) that must NOT be + // iterated over the surrounding context. + 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 || atomicRhsMustIterate || ((Dependency.dependsOn(exprDeps, Dependency.CONTEXT_ITEM) || Dependency.dependsOn(exprDeps, Dependency.CONTEXT_POSITION)) && //A positional predicate will be evaluated one time diff --git a/exist-core/src/test/java/org/exist/xquery/PathExprAtomicRhsTest.java b/exist-core/src/test/java/org/exist/xquery/PathExprAtomicRhsTest.java new file mode 100644 index 00000000000..15a0d5adffe --- /dev/null +++ b/exist-core/src/test/java/org/exist/xquery/PathExprAtomicRhsTest.java @@ -0,0 +1,103 @@ +/* + * eXist-db Open Source Native XML Database + * Copyright (C) 2001 The eXist-db Authors + * + * info@exist-db.org + * http://www.exist-db.org + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ +package org.exist.xquery; + +import org.exist.test.ExistXmldbEmbeddedServer; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Test; +import org.xmldb.api.base.ResourceSet; +import org.xmldb.api.base.XMLDBException; + +import static org.junit.Assert.assertEquals; + +/** + * Regression test for PathExpr per-item iteration over atomic-returning steps. + * + *

XPath 3.1 §3.3.5 mandates that in {@code E1/E2}, E2 is evaluated once + * for each item produced by E1. eXist's PathExpr.eval previously + * short-circuited to a single E2 evaluation when E1 was a persistent node-set + * and E2 did not declare a context dependency — collapsing multiplicity for + * atomic-returning E2 (literals, etc.).

+ * + * @see Issue #798 + */ +public class PathExprAtomicRhsTest { + + @ClassRule + public static final ExistXmldbEmbeddedServer embedded = + new ExistXmldbEmbeddedServer(false, true, true); + + @BeforeClass + public static void store() throws XMLDBException { + embedded.executeQuery( + "xmldb:store('/db', 'pathexpr-issue798.xml', )"); + } + + @AfterClass + public static void cleanup() throws XMLDBException { + try { + embedded.executeQuery("xmldb:remove('/db', 'pathexpr-issue798.xml')"); + } catch (final XMLDBException ignored) { + // best-effort cleanup + } + } + + /** In-memory baseline: //b/3 returns one 3 per b element. */ + @Test + public void inMemoryAtomicRhsIteratesPerItem() throws XMLDBException { + final ResourceSet rs = embedded.executeQuery( + "count(()//b/3)"); + assertEquals("2", rs.getResource(0).getContent()); + } + + /** Persistent input must iterate identically to in-memory. The bug. */ + @Test + public void persistentAtomicRhsIteratesPerItem() throws XMLDBException { + final ResourceSet rs = embedded.executeQuery( + "count(doc('/db/pathexpr-issue798.xml')//b/3)"); + assertEquals("2", rs.getResource(0).getContent()); + } + + /** Both forms must return the same sequence content. */ + @Test + public void inMemoryAndPersistentAgree() throws XMLDBException { + final ResourceSet inMem = embedded.executeQuery( + "string-join((for $x in ()//b/3 return string($x)), ',')"); + final ResourceSet stored = embedded.executeQuery( + "string-join((for $x in doc('/db/pathexpr-issue798.xml')//b/3 return string($x)), ',')"); + assertEquals(inMem.getResource(0).getContent(), stored.getResource(0).getContent()); + assertEquals("3,3", stored.getResource(0).getContent()); + } + + /** + * Sanity: node-axis RHS still de-duplicates as before. Two b elements + * with the same parent: //b/.. should de-dup to one a element. + */ + @Test + public void nodeRhsStillDedupes() throws XMLDBException { + final ResourceSet rs = embedded.executeQuery( + "count(doc('/db/pathexpr-issue798.xml')//b/..)"); + assertEquals("1", rs.getResource(0).getContent()); + } +}