From 87460a1c21f1704b13054c5b0ab452d4a953918e Mon Sep 17 00:00:00 2001 From: Joe Wicentowski Date: Fri, 6 Mar 2026 20:26:05 -0500 Subject: [PATCH 1/3] [bugfix] Fix duplicate node elimination for function calls in path expressions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per XPath 3.1 §3.3.1.1, the path operator '/' must eliminate duplicate nodes by identity and return results in document order. This was not happening when the last step in a path expression was a function call (or other non-Step PostfixExpr), because the dedup guard only checked `getLastExpression() instanceof Step`. Replace the heuristic with a `hasSlash` flag set by the grammar tree walker when SLASH, DSLASH, ABSOLUTE_SLASH, or ABSOLUTE_DSLASH tokens are encountered. This correctly identifies genuine path expressions without the performance cost of iterating the steps list, and without risking false positives on PathExprs used as generic expression containers (e.g., variable declarations followed by FLWOR expressions). Closes https://github.com/eXist-db/exist/issues/TBD Co-Authored-By: Claude Opus 4.6 --- .../org/exist/xquery/parser/XQueryTree.g | 8 ++ .../main/java/org/exist/xquery/PathExpr.java | 18 ++- .../org/exist/xquery/PathExprDedupTest.java | 115 ++++++++++++++++++ 3 files changed, 140 insertions(+), 1 deletion(-) create mode 100644 exist-core/src/test/java/org/exist/xquery/PathExprDedupTest.java diff --git a/exist-core/src/main/antlr/org/exist/xquery/parser/XQueryTree.g b/exist-core/src/main/antlr/org/exist/xquery/parser/XQueryTree.g index 20308296806..3b156f59ddf 100644 --- a/exist-core/src/main/antlr/org/exist/xquery/parser/XQueryTree.g +++ b/exist-core/src/main/antlr/org/exist/xquery/parser/XQueryTree.g @@ -2338,6 +2338,7 @@ throws PermissionDeniedException, EXistException, XPathException #( ABSOLUTE_SLASH { + path.setHasSlash(); RootNode root= new RootNode(context); path.add(root); } @@ -2348,6 +2349,7 @@ throws PermissionDeniedException, EXistException, XPathException #( ABSOLUTE_DSLASH { + path.setHasSlash(); RootNode root= new RootNode(context); path.add(root); } @@ -2948,6 +2950,9 @@ throws PermissionDeniedException, EXistException, XPathException | #( SLASH step=expr [path] + { + path.setHasSlash(); + } ( rightStep=expr [path] { @@ -2972,6 +2977,9 @@ throws PermissionDeniedException, EXistException, XPathException | #( DSLASH step=expr [path] + { + path.setHasSlash(); + } ( rightStep=expr [path] { 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 8e096376cc5..32b02288fd2 100644 --- a/exist-core/src/main/java/org/exist/xquery/PathExpr.java +++ b/exist-core/src/main/java/org/exist/xquery/PathExpr.java @@ -53,6 +53,14 @@ public class PathExpr extends AbstractExpression implements CompiledXQuery, protected boolean inPredicate = false; + /** + * Set to true when this PathExpr represents an actual XPath path + * expression with '/' or '//' steps, as opposed to a generic expression + * container. When true, duplicate node elimination is applied per + * XPath 3.1 §3.3.1.1. + */ + private boolean hasSlash = false; + protected Expression parent; public PathExpr(final XQueryContext context) { @@ -298,7 +306,7 @@ public Sequence eval(Sequence contextSequence, final Item contextItem) throws XP !Type.subTypeOf(result.getItemType(), Type.NODE)) { gotAtomicResult = true; } - if (steps.size() > 1 && getLastExpression() instanceof Step) { + if (hasSlash) { // remove duplicate nodes if this is a path // expression with more than one step result.removeDuplicates(); @@ -375,6 +383,14 @@ public Expression getLastExpression() { return steps.isEmpty() ? null : steps.getLast(); } + /** + * Marks this PathExpr as containing a '/' or '//' path operator. + * Called from the grammar tree walker when SLASH or DSLASH is encountered. + */ + public void setHasSlash() { + this.hasSlash = true; + } + /** * Get the length. * diff --git a/exist-core/src/test/java/org/exist/xquery/PathExprDedupTest.java b/exist-core/src/test/java/org/exist/xquery/PathExprDedupTest.java new file mode 100644 index 00000000000..76ec0bda898 --- /dev/null +++ b/exist-core/src/test/java/org/exist/xquery/PathExprDedupTest.java @@ -0,0 +1,115 @@ +/* + * 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.ClassRule; +import org.junit.Test; +import org.xmldb.api.base.ResourceSet; +import org.xmldb.api.base.XMLDBException; +import org.xmldb.api.modules.XQueryService; + +import static org.junit.Assert.assertEquals; + +/** + * Tests for duplicate node elimination in path expressions. + * + * Per XPath 3.1 §3.3.1.1, the path operator '/' must eliminate duplicate + * nodes (by identity) and return results in document order when every + * evaluation of E2 returns nodes. This must apply regardless of whether + * E2 is an axis step, function call, or other PostfixExpr. + * + * @see XPath 3.1 §3.3.1.1 + */ +public class PathExprDedupTest { + + @ClassRule + public static final ExistXmldbEmbeddedServer existEmbeddedServer = + new ExistXmldbEmbeddedServer(false, true, true); + + private String query(final String xquery) throws XMLDBException { + final XQueryService xqs = existEmbeddedServer.getRoot().getService(XQueryService.class); + final ResourceSet result = xqs.query(xquery); + return result.getResource(0).getContent().toString(); + } + + /** + * XQTS K2-Steps-31 (explicit expansion): function call in path where + * multiple context items produce the same result node. The path operator + * must deduplicate results per §3.3.1.1. + */ + @Test + public void functionCallInPathDedup() throws XMLDBException { + final String result = query( + "declare variable $root := ;\n" + + "declare function local:function($arg) { $root[$arg] };\n" + + "count($root/descendant-or-self::node()/local:function(.))"); + assertEquals("1", result); + } + + /** + * Simpler case: child::* / function that always returns the same node. + */ + @Test + public void functionReturnsConstantNodeDedup() throws XMLDBException { + final String result = query( + "declare variable $root := ;\n" + + "declare function local:getroot($x) { $root };\n" + + "count($root/*/local:getroot(.))"); + assertEquals("1", result); + } + + /** + * Ensure for-loop results are NOT incorrectly deduplicated. + * This is the scenario from the 2009 bug fix (SF #2880394). + */ + @Test + public void forLoopPreservesDuplicates() throws XMLDBException { + final String result = query( + "declare variable $root := ;\n" + + "count(for $x in (1, 2, 3) return $root)"); + assertEquals("3", result); + } + + /** + * Global variable with for-loop should preserve all results. + */ + @Test + public void globalVarForLoopPreservesDuplicates() throws XMLDBException { + final String result = query( + "declare variable $data := ;\n" + + "count(for $i in 1 to 5 return $data)"); + assertEquals("5", result); + } + + /** + * Path with axis step followed by function call — dedup should apply. + */ + @Test + public void axisStepThenFunctionCallDedup() throws XMLDBException { + final String result = query( + "declare variable $doc := ;\n" + + "declare function local:parent($n) { $n/.. };\n" + + "count($doc/*/local:parent(.))"); + assertEquals("1", result); + } +} From 01085a08796b6dd96ed96e3df1c111f71eb95776 Mon Sep 17 00:00:00 2001 From: Joe Wicentowski Date: Sat, 7 Mar 2026 16:43:55 -0500 Subject: [PATCH 2/3] [bugfix] Fix NPE in path expression dedup for non-axis-step paths The hasSlash-based duplicate elimination introduced in 3b47500 correctly extends dedup to function calls in path expressions, but triggers a NullPointerException when the path contains constructed nodes from independent document contexts (e.g., standalone attribute or document constructors). Two fixes: 1. PathExpr.eval: Guard removeDuplicates() with a node-type check so it is only called when the result actually contains nodes, not atomic values (which cannot have duplicates in the XPath sense). 2. NodeImpl.compareTo: Handle null document references defensively. DocumentImpl nodes always have document=null (by design in the constructor), so comparing two DocumentImpl instances from different contexts would NPE at document.docId. This is a latent bug exposed by calling removeDuplicates on sequences containing nodes from independent constructor documents. Adds test cases for XQTS K2-Axes-48 and K2-Axes-49 which exercise path expressions ending with integer literals and number() calls over sequences of diverse constructed node types. Co-Authored-By: Claude Opus 4.6 --- .../java/org/exist/dom/memtree/NodeImpl.java | 12 +++++-- .../main/java/org/exist/xquery/PathExpr.java | 3 +- .../org/exist/xquery/PathExprDedupTest.java | 32 +++++++++++++++++++ 3 files changed, 43 insertions(+), 4 deletions(-) diff --git a/exist-core/src/main/java/org/exist/dom/memtree/NodeImpl.java b/exist-core/src/main/java/org/exist/dom/memtree/NodeImpl.java index 47f03d4096b..0e35a2a5360 100644 --- a/exist-core/src/main/java/org/exist/dom/memtree/NodeImpl.java +++ b/exist-core/src/main/java/org/exist/dom/memtree/NodeImpl.java @@ -309,10 +309,16 @@ public int compareTo(final NodeImpl other) { } else { return Constants.SUPERIOR; } - } else if(document.docId < other.document.docId) { - return Constants.INFERIOR; } else { - return Constants.SUPERIOR; + final long thisDocId = document != null ? document.docId : 0; + final long otherDocId = other.document != null ? other.document.docId : 0; + if (thisDocId < otherDocId) { + return Constants.INFERIOR; + } else if (thisDocId > otherDocId) { + return Constants.SUPERIOR; + } else { + return Constants.EQUAL; + } } } 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 32b02288fd2..c0da53cd23d 100644 --- a/exist-core/src/main/java/org/exist/xquery/PathExpr.java +++ b/exist-core/src/main/java/org/exist/xquery/PathExpr.java @@ -306,7 +306,8 @@ public Sequence eval(Sequence contextSequence, final Item contextItem) throws XP !Type.subTypeOf(result.getItemType(), Type.NODE)) { gotAtomicResult = true; } - if (hasSlash) { + if (hasSlash && !result.isEmpty() + && Type.subTypeOf(result.getItemType(), Type.NODE)) { // remove duplicate nodes if this is a path // expression with more than one step result.removeDuplicates(); diff --git a/exist-core/src/test/java/org/exist/xquery/PathExprDedupTest.java b/exist-core/src/test/java/org/exist/xquery/PathExprDedupTest.java index 76ec0bda898..576a02663e5 100644 --- a/exist-core/src/test/java/org/exist/xquery/PathExprDedupTest.java +++ b/exist-core/src/test/java/org/exist/xquery/PathExprDedupTest.java @@ -101,6 +101,38 @@ public void globalVarForLoopPreservesDuplicates() throws XMLDBException { assertEquals("5", result); } + /** + * XQTS K2-Axes-48: path expression ending with integer literal. + * Must not NPE when removeDuplicates is called on atomic results. + */ + @Test + public void pathEndingWithIntegerLiteral() throws XMLDBException { + final XQueryService xqs = existEmbeddedServer.getRoot().getService(XQueryService.class); + final ResourceSet result = xqs.query( + "declare variable $myVar := ;\n" + + "$myVar/(, , , , attribute name {}, document {()})/3"); + assertEquals(6, (int) result.getSize()); + for (int i = 0; i < 6; i++) { + assertEquals("3", result.getResource(i).getContent().toString()); + } + } + + /** + * XQTS K2-Axes-49: path expression ending with number() function. + * Must not NPE when removeDuplicates is called on atomic results. + */ + @Test + public void pathEndingWithNumberFunction() throws XMLDBException { + final XQueryService xqs = existEmbeddedServer.getRoot().getService(XQueryService.class); + final ResourceSet result = xqs.query( + "declare variable $myVar := ;\n" + + "$myVar/(, , , , attribute name {}, document {()})/number()"); + assertEquals(6, (int) result.getSize()); + for (int i = 0; i < 6; i++) { + assertEquals("NaN", result.getResource(i).getContent().toString()); + } + } + /** * Path with axis step followed by function call — dedup should apply. */ From 0ae5b268276f19f30ed40f483d8b73a0969d2b40 Mon Sep 17 00:00:00 2001 From: Joe Wicentowski Date: Sat, 18 Apr 2026 22:13:12 -0400 Subject: [PATCH 3/3] [refactor] Convert XQuery test strings to Java 15 text blocks Co-Authored-By: Claude Opus 4.6 (1M context) --- .../org/exist/xquery/PathExprDedupTest.java | 48 +++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/exist-core/src/test/java/org/exist/xquery/PathExprDedupTest.java b/exist-core/src/test/java/org/exist/xquery/PathExprDedupTest.java index 576a02663e5..0601f08880f 100644 --- a/exist-core/src/test/java/org/exist/xquery/PathExprDedupTest.java +++ b/exist-core/src/test/java/org/exist/xquery/PathExprDedupTest.java @@ -59,10 +59,10 @@ private String query(final String xquery) throws XMLDBException { */ @Test public void functionCallInPathDedup() throws XMLDBException { - final String result = query( - "declare variable $root := ;\n" + - "declare function local:function($arg) { $root[$arg] };\n" + - "count($root/descendant-or-self::node()/local:function(.))"); + final String result = query(""" + declare variable $root := ; + declare function local:function($arg) { $root[$arg] }; + count($root/descendant-or-self::node()/local:function(.))"""); assertEquals("1", result); } @@ -71,10 +71,10 @@ public void functionCallInPathDedup() throws XMLDBException { */ @Test public void functionReturnsConstantNodeDedup() throws XMLDBException { - final String result = query( - "declare variable $root := ;\n" + - "declare function local:getroot($x) { $root };\n" + - "count($root/*/local:getroot(.))"); + final String result = query(""" + declare variable $root := ; + declare function local:getroot($x) { $root }; + count($root/*/local:getroot(.))"""); assertEquals("1", result); } @@ -84,9 +84,9 @@ public void functionReturnsConstantNodeDedup() throws XMLDBException { */ @Test public void forLoopPreservesDuplicates() throws XMLDBException { - final String result = query( - "declare variable $root := ;\n" + - "count(for $x in (1, 2, 3) return $root)"); + final String result = query(""" + declare variable $root := ; + count(for $x in (1, 2, 3) return $root)"""); assertEquals("3", result); } @@ -95,9 +95,9 @@ public void forLoopPreservesDuplicates() throws XMLDBException { */ @Test public void globalVarForLoopPreservesDuplicates() throws XMLDBException { - final String result = query( - "declare variable $data := ;\n" + - "count(for $i in 1 to 5 return $data)"); + final String result = query(""" + declare variable $data := ; + count(for $i in 1 to 5 return $data)"""); assertEquals("5", result); } @@ -108,9 +108,9 @@ public void globalVarForLoopPreservesDuplicates() throws XMLDBException { @Test public void pathEndingWithIntegerLiteral() throws XMLDBException { final XQueryService xqs = existEmbeddedServer.getRoot().getService(XQueryService.class); - final ResourceSet result = xqs.query( - "declare variable $myVar := ;\n" + - "$myVar/(, , , , attribute name {}, document {()})/3"); + final ResourceSet result = xqs.query(""" + declare variable $myVar := ; + $myVar/(, , , , attribute name {}, document {()})/3"""); assertEquals(6, (int) result.getSize()); for (int i = 0; i < 6; i++) { assertEquals("3", result.getResource(i).getContent().toString()); @@ -124,9 +124,9 @@ public void pathEndingWithIntegerLiteral() throws XMLDBException { @Test public void pathEndingWithNumberFunction() throws XMLDBException { final XQueryService xqs = existEmbeddedServer.getRoot().getService(XQueryService.class); - final ResourceSet result = xqs.query( - "declare variable $myVar := ;\n" + - "$myVar/(, , , , attribute name {}, document {()})/number()"); + final ResourceSet result = xqs.query(""" + declare variable $myVar := ; + $myVar/(, , , , attribute name {}, document {()})/number()"""); assertEquals(6, (int) result.getSize()); for (int i = 0; i < 6; i++) { assertEquals("NaN", result.getResource(i).getContent().toString()); @@ -138,10 +138,10 @@ public void pathEndingWithNumberFunction() throws XMLDBException { */ @Test public void axisStepThenFunctionCallDedup() throws XMLDBException { - final String result = query( - "declare variable $doc := ;\n" + - "declare function local:parent($n) { $n/.. };\n" + - "count($doc/*/local:parent(.))"); + final String result = query(""" + declare variable $doc := ; + declare function local:parent($n) { $n/.. }; + count($doc/*/local:parent(.))"""); assertEquals("1", result); } }