Index keys: normalize to double only where the conversion is exact

nitrite/src/main/java/org/dizitart/no2/common/DBValue.java

@@ -26,6 +26,8 @@
 import java.io.ObjectInputStream;
 import java.io.ObjectOutputStream;
 import java.io.Serializable;
+import java.math.BigDecimal;
+import java.math.BigInteger;
 
 /**
  * @author Anindya Chatterjee
@@ -59,14 +61,48 @@ public int compareTo(DBValue o) {
     }
 
     private static Comparable<?> normalizeNumber(Comparable<?> value) {
-        // Normalize all numeric types to Double for consistent serialization
+        // Normalize numeric types to Double for consistent serialization
         // This ensures Integer(5) and Double(5.0) are treated the same in indexes
         if (value instanceof Number && !(value instanceof Double)) {
-            return ((Number) value).doubleValue();
+            double normalized = ((Number) value).doubleValue();
+            // ...but only where a double can hold the value exactly. Beyond 2^53 it cannot,
+            // and folding there maps distinct numbers onto one index key: consecutive longs
+            // around 8.7e17 are 128 apart as doubles, so ids closer than that become the same
+            // key, which makes a unique index reject a new id and a non-unique one return rows
+            // belonging to a different id.
+            if (isExactAsDouble((Number) value, normalized)) {
+                return normalized;
+            }
         }
         return value;
     }
 
+    private static boolean isExactAsDouble(Number value, double normalized) {
+        if (value instanceof Integer || value instanceof Short
+            || value instanceof Byte || value instanceof Float) {
+            // every value of these types survives the widening unchanged
+            return true;
+        }
+
+        if (Double.isNaN(normalized) || Double.isInfinite(normalized)) {
+            return false;
+        }
+
+        // new BigDecimal(double) is the exact value of the double, so this compares the
+        // number against what the conversion actually produced
+        BigDecimal converted = new BigDecimal(normalized);
+        if (value instanceof Long) {
+            return converted.compareTo(BigDecimal.valueOf(value.longValue())) == 0;
+        }
+        if (value instanceof BigInteger) {
+            return converted.compareTo(new BigDecimal((BigInteger) value)) == 0;
+        }
+        if (value instanceof BigDecimal) {
+            return converted.compareTo((BigDecimal) value) == 0;
+        }
+        return false;
+    }
+
     private void writeObject(ObjectOutputStream stream) throws IOException {
         stream.writeObject(value);
     }

nitrite/src/test/java/org/dizitart/no2/common/DBValueTest.java

@@ -0,0 +1,83 @@
+/*
+ * Copyright (c) 2017-2021 Nitrite author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.dizitart.no2.common;
+
+import org.junit.Test;
+
+import java.math.BigInteger;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+
+public class DBValueTest {
+
+    @Test
+    public void testSmallNumbersAreNormalizedToDouble() {
+        // cross-type equality for values a double holds exactly, including in stores that
+        // compare the encoded key rather than going through compareTo
+        assertEquals(new DBValue(5.0), new DBValue(5));
+        assertEquals(new DBValue(5.0), new DBValue(5L));
+        assertEquals(new DBValue(5.0), new DBValue((short) 5));
+        assertEquals(new DBValue(5.0), new DBValue((byte) 5));
+        assertEquals(new DBValue(5.0), new DBValue(BigInteger.valueOf(5)));
+    }
+
+    @Test
+    public void testLargeLongsKeepTheirValue() {
+        long id = 870000000000000123L;   // beyond 2^53, doubles are 128 apart here
+        assertEquals(id, new DBValue(id).getValue());
+    }
+
+    @Test
+    public void testLongsCloserThanDoublePrecisionStayDistinct() {
+        long id = 870000000000000123L;
+        assertNotEquals(new DBValue(id), new DBValue(id + 1));
+        assertNotEquals(0, new DBValue(id).compareTo(new DBValue(id + 1)));
+    }
+
+    @Test
+    public void testLargeBigIntegerKeepsItsValue() {
+        // odd and far beyond 2^53, so no double holds it exactly
+        BigInteger value = BigInteger.ONE.shiftLeft(70).add(BigInteger.ONE);
+        assertEquals(value, new DBValue(value).getValue());
+        assertNotEquals(new DBValue(value), new DBValue(value.add(BigInteger.valueOf(2))));
+    }
+
+    @Test
+    public void testLongAtTheEdgeOfExactRange() {
+        long exact = 1L << 53;          // the largest power of two a double still steps by one
+        assertEquals(2.0 * (1L << 52), new DBValue(exact).getValue());
+        // one above it is not representable, so it has to keep its own value
+        assertEquals(exact + 1, new DBValue(exact + 1).getValue());
+    }
+
+    @Test
+    public void testExactlyRepresentableLargeValuesStillNormalize() {
+        // 2^63 is a power of two, so the conversion loses nothing and folding is safe
+        BigInteger powerOfTwo = BigInteger.ONE.shiftLeft(63);
+        assertEquals(Math.pow(2, 63), new DBValue(powerOfTwo).getValue());
+    }
+
+    @Test
+    public void testNumbersStillCompareAcrossTypes() {
+        // compareTo goes through Comparables/Numbers, so this holds whatever the stored form is
+        long id = 870000000000000123L;
+        assertEquals(0, new DBValue(id).compareTo(new DBValue(BigInteger.valueOf(id))));
+        assertEquals(0, new DBValue(5).compareTo(new DBValue(5.0)));
+    }
+}

nitrite/src/test/java/org/dizitart/no2/integration/collection/CollectionLargeIdIndexTest.java

@@ -0,0 +1,62 @@
+/*
+ * Copyright (c) 2017-2021 Nitrite author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.dizitart.no2.integration.collection;
+
+import org.dizitart.no2.collection.Document;
+import org.dizitart.no2.index.IndexType;
+import org.junit.Test;
+
+import static org.dizitart.no2.collection.Document.createDocument;
+import static org.dizitart.no2.filters.FluentFilter.where;
+import static org.dizitart.no2.index.IndexOptions.indexOptions;
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Ids above 2^53 - snowflake ids, TSIDs and the like - are further apart than a double can
+ * step, so an index that keyed them as doubles could not tell them apart.
+ */
+public class CollectionLargeIdIndexTest extends BaseCollectionTest {
+
+    // two ids 1 apart; the nearest doubles around here are 128 apart
+    private static final long FIRST_ID = 870000000000000123L;
+    private static final long SECOND_ID = FIRST_ID + 1;
+
+    @Test
+    public void testUniqueIndexAcceptsIdsCloserThanDoublePrecision() {
+        collection.remove(org.dizitart.no2.filters.Filter.ALL);
+        collection.createIndex(indexOptions(IndexType.UNIQUE), "entityId");
+
+        collection.insert(createDocument("entityId", FIRST_ID));
+        collection.insert(createDocument("entityId", SECOND_ID));
+
+        assertEquals(2, collection.find().size());
+    }
+
+    @Test
+    public void testIndexedLookupReturnsOnlyTheMatchingId() {
+        collection.remove(org.dizitart.no2.filters.Filter.ALL);
+        collection.createIndex(indexOptions(IndexType.NON_UNIQUE), "entityId");
+
+        collection.insert(createDocument("entityId", FIRST_ID));
+        collection.insert(createDocument("entityId", SECOND_ID));
+
+        Document found = collection.find(where("entityId").eq(FIRST_ID)).firstOrNull();
+        assertEquals(1, collection.find(where("entityId").eq(FIRST_ID)).size());
+        assertEquals(FIRST_ID, (long) found.get("entityId", Long.class));
+    }
+}