[bugfix] map keys: compare by op:same-key, never coerce across type families

exist-core/src/main/java/org/exist/xquery/functions/map/MapType.java

@@ -66,6 +66,14 @@ public class MapType extends AbstractMapType {
      * then this is set to {@link #MIXED_KEY_TYPES}.
      * <p>
      * Uses integer values from {@link org.exist.xquery.value.Type}.
+     * <p>
+     * This is reported via {@link #getKeyType()} but does NOT take part in key
+     * comparison. Lookups ({@link #get(AtomicValue)} / {@link #contains(AtomicValue)})
+     * delegate directly to the underlying map, whose comparator implements
+     * {@code op:same-key} (see {@link AbstractMapType#sameKey}). Keys are compared
+     * by their op:same-key family, never coerced to {@code keyType} - a key from a
+     * different family that shares a lexical value (e.g. the string {@code "12"}
+     * and the integer {@code 12}) is correctly treated as distinct.
      */
     private int keyType = UNKNOWN_KEY_TYPE;
 
@@ -251,12 +259,7 @@ public void add(final AtomicValue key, final Sequence value) {
     }
 
     @Override
-    public Sequence get(AtomicValue key) {
-        key = convert(key);
-        if (key == null) {
-            return Sequence.EMPTY_SEQUENCE;
-        }
-
+    public Sequence get(final AtomicValue key) {
         return map.get(key, Sequence.EMPTY_SEQUENCE);
     }
 
@@ -267,12 +270,7 @@ public AbstractMapType put(final AtomicValue key, final Sequence value) {
     }
 
     @Override
-    public boolean contains(AtomicValue key) {
-        key = convert(key);
-        if (key == null) {
-            return false;
-        }
-
+    public boolean contains(final AtomicValue key) {
         return map.contains(key);
     }
 
@@ -375,17 +373,6 @@ private void setKeyType(final IMap<AtomicValue, Sequence> newMap) {
         }
     }
 
-    private AtomicValue convert(final AtomicValue key) {
-        if (keyType != UNKNOWN_KEY_TYPE && keyType != MIXED_KEY_TYPES) {
-            try {
-                return key.convertTo(keyType);
-            } catch (final XPathException e) {
-                return null;
-            }
-        }
-        return key;
-    }
-
     @Override
     public int getKeyType() {
         return keyType;

exist-core/src/test/xquery/maps/mapKeySameKey.xqm

@@ -0,0 +1,179 @@
+(:
+ : 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
+ :)
+xquery version "3.1";
+
+(:~
+ : Tests for map key comparison via op:same-key (XQuery 3.1 section 17.1).
+ :
+ : Map keys are compared with op:same-key, which is a TYPE-GROUP comparison:
+ :   - the numeric family (xs:decimal/xs:integer/xs:float/xs:double) interchanges,
+ :   - the string family (xs:string/xs:anyURI/xs:untypedAtomic) interchanges,
+ :   - every other type matches only itself.
+ : There is NO key atomization or casting across groups.
+ :
+ : These tests deliberately use the map{...} CONSTRUCTOR form (which builds a
+ : homogeneously-typed map). The qt3/qt4 suites place their cross-family
+ : distinctness tests on map:entry() (a mixed-key map), so the constructor path
+ : was historically untested - and eXist used to conflate cross-family keys that
+ : shared a lexical value (e.g. map{"12":"x"}(12) wrongly returned "x").
+ :)
+module namespace skt="http://exist-db.org/xquery/test/maps/samekey";
+
+import module namespace test="http://exist-db.org/xquery/xqsuite" at "resource:org/exist/xquery/lib/xqsuite/xqsuite.xql";
+
+(: ----------------------------------------------------------------------- :)
+(: Cross-family distinctness: keys of different op:same-key families that   :)
+(: share a lexical value must NOT match.                                    :)
+(: ----------------------------------------------------------------------- :)
+
+declare
+    %test:name("string key vs integer lookup are distinct (contains)")
+    %test:assertFalse
+function skt:string-key-vs-integer-contains() {
+    map:contains(map { "12": "x" }, 12)
+};
+
+declare
+    %test:name("string key vs integer lookup are distinct (get)")
+    %test:assertEmpty
+function skt:string-key-vs-integer-get() {
+    map:get(map { "12": "x" }, 12)
+};
+
+declare
+    %test:name("string key vs double lookup are distinct (contains)")
+    %test:assertFalse
+function skt:string-key-vs-double-contains() {
+    map:contains(map { "5": 1 }, 5.0e0)
+};
+
+declare
+    %test:name("string key vs xs:date lookup are distinct (contains)")
+    %test:assertFalse
+function skt:string-key-vs-date-contains() {
+    map:contains(map { "2020-01-01": 1 }, xs:date("2020-01-01"))
+};
+
+declare
+    %test:name("xs:date key vs string lookup are distinct (contains)")
+    %test:assertFalse
+function skt:date-key-vs-string-contains() {
+    map:contains(map { xs:date("2020-01-01"): 1 }, "2020-01-01")
+};
+
+declare
+    %test:name("boolean key vs string lookup are distinct (contains)")
+    %test:assertFalse
+function skt:boolean-key-vs-string-contains() {
+    map:contains(map { true(): "x" }, "true")
+};
+
+declare
+    %test:name("integer key vs string lookup are distinct (contains)")
+    %test:assertFalse
+function skt:integer-key-vs-string-contains() {
+    map:contains(map { 12: "x" }, "12")
+};
+
+(: ----------------------------------------------------------------------- :)
+(: Numeric family is by VALUE, not by lossy cast: a non-integral lookup     :)
+(: must not match an integer key (no truncation).                           :)
+(: ----------------------------------------------------------------------- :)
+
+declare
+    %test:name("integer key does not match a non-integral double lookup (contains)")
+    %test:assertFalse
+function skt:integer-key-vs-fractional-double-contains() {
+    map:contains(map { 1: "x" }, 1.5e0)
+};
+
+declare
+    %test:name("integer key does not match a non-integral double lookup (get)")
+    %test:assertEmpty
+function skt:integer-key-vs-fractional-double-get() {
+    map:get(map { 1: "x" }, 1.5e0)
+};
+
+(: ----------------------------------------------------------------------- :)
+(: Within-family positives: members of the SAME op:same-key family that     :)
+(: are value-equal MUST interchange (these must not regress).               :)
+(: ----------------------------------------------------------------------- :)
+
+declare
+    %test:name("numeric family: integer key matches equal double lookup (contains)")
+    %test:assertTrue
+function skt:numeric-integer-key-vs-double-contains() {
+    map:contains(map { 5: 1 }, 5.0e0)
+};
+
+declare
+    %test:name("numeric family: integer key matches equal double lookup (get)")
+    %test:assertEquals(1)
+function skt:numeric-integer-key-vs-double-get() {
+    map:get(map { 5: 1 }, 5.0e0)
+};
+
+declare
+    %test:name("numeric family: decimal key matches equal integer lookup (contains)")
+    %test:assertTrue
+function skt:numeric-decimal-key-vs-integer-contains() {
+    map:contains(map { 5.0: 1 }, 5)
+};
+
+declare
+    %test:name("string family: string key matches equal xs:anyURI lookup (contains)")
+    %test:assertTrue
+function skt:string-key-vs-anyuri-contains() {
+    map:contains(map { "urn:x": 1 }, xs:anyURI("urn:x"))
+};
+
+declare
+    %test:name("string family: string key matches equal xs:untypedAtomic lookup (get)")
+    %test:assertEquals("x")
+function skt:string-key-vs-untypedatomic-get() {
+    map:get(map { "12": "x" }, xs:untypedAtomic("12"))
+};
+
+declare
+    %test:name("string family: anyURI key matches equal string lookup (contains)")
+    %test:assertTrue
+function skt:anyuri-key-vs-string-contains() {
+    map:contains(map { xs:anyURI("urn:x"): 1 }, "urn:x")
+};
+
+(: ----------------------------------------------------------------------- :)
+(: Same-type self-matches still work after the fix.                         :)
+(: ----------------------------------------------------------------------- :)
+
+declare
+    %test:name("string key matches equal string lookup (get)")
+    %test:assertEquals("x")
+function skt:string-key-self-get() {
+    map:get(map { "12": "x" }, "12")
+};
+
+declare
+    %test:name("xs:date key matches equal xs:date lookup (contains)")
+    %test:assertTrue
+function skt:date-key-self-contains() {
+    map:contains(map { xs:date("2020-01-01"): 1 }, xs:date("2020-01-01"))
+};

exist-core/src/test/xquery/xquery3/fnSerializeNewline.xqm

@@ -49,26 +49,31 @@ function ser:exist-insert-final-newline-true() {
 
 declare
     %test:assertTrue
-function ser:exist-insert-final-newline-false-json() {
+function ser:exist-insert-final-newline-true-json() {
     let $doc := map { "a": 1 }
     let $serialized := fn:serialize($doc,
         map {
             "method": "json",
-            "exist:insert-final-newline": false()
+            xs:QName("exist:insert-final-newline"): true()
         }
     )
-    return fn:ends-with($serialized, "}")
+    return fn:ends-with($serialized, "
")
 };
 
+(: An eXist serialization parameter keyed by the prefixed string "exist:..." is
+ : NON-conformant and is now ignored - only the xs:QName key form is honored,
+ : because op:same-key treats a string key and a QName key as distinct. Passing
+ : true() via the string key therefore has no effect, so the default
+ : (insert-final-newline=false) applies and no trailing newline is added. :)
 declare
     %test:assertTrue
-function ser:exist-insert-final-newline-true-json() {
+function ser:exist-insert-final-newline-json-prefixed-string-ignored() {
     let $doc := map { "a": 1 }
     let $serialized := fn:serialize($doc,
         map {
             "method": "json",
             "exist:insert-final-newline": true()
         }
     )
-    return fn:ends-with($serialized, "
")
+    return fn:ends-with($serialized, "}")
 };

exist-core/src/test/xquery/xquery3/serialize.xql

@@ -681,14 +681,16 @@ function ser:exist-output-doctype-QName($value as xs:boolean) {
         map { xs:QName("exist:output-doctype") : $value })
 };
 
+(: An eXist serialization parameter keyed by the prefixed string "exist:..." is
+ : NON-conformant and is now ignored - only the xs:QName key form is honored,
+ : because op:same-key treats a string key and a QName key as distinct. Passing
+ : true() via the string key therefore has no effect, so the default (no doctype)
+ : applies. The xs:QName form is exercised by ser:exist-output-doctype-QName above. :)
 declare
-    %test:args("true")
-    %test:assertXPath("contains($result, '-//OASIS//DTD DITA BookMap//EN') and contains($result, 'bookmap.dtd')")
-    %test:args("false")
     %test:assertXPath("not(contains($result, '-//OASIS//DTD DITA BookMap//EN')) and not(contains($result, 'bookmap.dtd'))")
-function ser:exist-output-doctype-string($value as xs:boolean) {
+function ser:exist-output-doctype-prefixed-string-ignored() {
     serialize(doc($ser:collection || "/test-with-doctype.xml"),
-        map { "exist:output-doctype" : $value })
+        map { "exist:output-doctype" : true() })
 };
 
 declare
@@ -701,14 +703,14 @@ function ser:exist-expand-xinclude-QName($value as xs:boolean) {
         map { xs:QName("exist:expand-xincludes"): $value })
 };
 
+(: Prefixed-string key is ignored (see note above); passing false() has no
+ : effect, so the default (expand-xincludes=true) applies and the include is
+ : expanded to its 'comment' content. :)
 declare
-    %test:args("true")
     %test:assertXPath("contains($result, 'comment')")
-    %test:args("false")
-    %test:assertXPath("contains($result, 'include')")
-function ser:exist-expand-xinclude-string($value as xs:boolean) {
+function ser:exist-expand-xinclude-prefixed-string-ignored() {
     serialize($ser:xi-doc,
-        map { "exist:expand-xincludes": $value })
+        map { "exist:expand-xincludes": false() })
 };
 
 declare
@@ -723,16 +725,13 @@ function ser:exist-add-exist-id-QName($value as xs:string) {
         map { xs:QName("exist:add-exist-id"): $value })
 };
 
+(: Prefixed-string key is ignored (see note above); passing "all" has no effect,
+ : so the default (add-exist-id=none) applies and no exist:id attributes appear. :)
 declare
-    %test:args("all")
-    %test:assertEquals('<?pi?><elem xmlns:exist="http://exist.sourceforge.net/NS/exist" exist:id="2" exist:source="test.xml" a="abc"><!--comment--><b exist:id="2.3">123</b></elem>')
-    %test:args("element")
-    %test:assertEquals('<?pi?><elem xmlns:exist="http://exist.sourceforge.net/NS/exist" exist:id="2" exist:source="test.xml" a="abc"><!--comment--><b>123</b></elem>')
-    %test:args("none")
     %test:assertXPath("not(contains($result, 'exist:id'))")
-function ser:exist-add-exist-id-string($value as xs:string) {
+function ser:exist-add-exist-id-prefixed-string-ignored() {
     serialize(doc($ser:collection || "/test.xml"),
-        map { "exist:add-exist-id": $value })
+        map { "exist:add-exist-id": "all" })
 };
 
 declare
@@ -750,17 +749,16 @@ function ser:exist-jsonp-QName($value as xs:string) {
     )
 };
 
+(: Prefixed-string key is ignored (see note above); passing "functionName" has
+ : no effect, so the default (no JSONP callback) applies and plain JSON results. :)
 declare
-    %test:args("functionName")
-    %test:assertEquals('functionName({"author":["John Doe","Robert Smith"]})')
-    %test:args("anotherName")
-    %test:assertEquals('anotherName({"author":["John Doe","Robert Smith"]})')
-function ser:exist-jsonp-string($value as xs:string) {
+    %test:assertEquals('{"author":["John Doe","Robert Smith"]}')
+function ser:exist-jsonp-prefixed-string-ignored() {
     serialize($ser:in-memory-book,
         map {
             "method": "json",
             "media-type": "application/json",
-            "exist:jsonp": $value
+            "exist:jsonp": "functionName"
         }
     )
 };
@@ -775,14 +773,13 @@ function ser:exist-process-xsl-pi-QName($value as xs:boolean) {
         map { xs:QName("exist:process-xsl-pi"): $value })
 };
 
+(: Prefixed-string key is ignored (see note above); passing false() has no
+ : effect, so the default (process-xsl-pi=true) applies and the PI is processed. :)
 declare
-    %test:args("true")
     %test:assertEquals('processed')
-    %test:args("false")
-    %test:assertXPath("contains($result, 'stylesheet')")
-function ser:exist-process-xsl-pi-string($value as xs:boolean) {
+function ser:exist-process-xsl-pi-prefixed-string-ignored() {
     serialize(doc($ser:collection || "/test-xsl.xml"),
-        map { "exist:process-xsl-pi": $value })
+        map { "exist:process-xsl-pi": false() })
 };
 
 declare