handle 64bit longs in document key

firebase · Dec 10, 2024 · f8c91ee · f8c91ee
1 parent dc2f945
commit f8c91ee
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 12 deletions.
diff --git a/packages/firestore/src/local/memory_remote_document_cache.ts b/packages/firestore/src/local/memory_remote_document_cache.ts
@@ -47,6 +47,11 @@ interface MemoryRemoteDocumentCacheEntry {
   size: number;
 }
 
+/**
+ * The smallest value representable by a 64-bit signed integer (long).
+ */
+const MIN_LONG_VALUE = '-9223372036854775808';
+
 type DocumentEntryMap = SortedMap<DocumentKey, MemoryRemoteDocumentCacheEntry>;
 function documentEntryMap(): DocumentEntryMap {
   return new SortedMap<DocumentKey, MemoryRemoteDocumentCacheEntry>(
@@ -171,9 +176,11 @@ class MemoryRemoteDocumentCacheImpl implements MemoryRemoteDocumentCache {
     // Documents are ordered by key, so we can use a prefix scan to narrow down
     // the documents we need to match the query against.
     const collectionPath = query.path;
-    // DocumentId can be numeric ("__id<Long>__") or a plain string. Numeric IDs ordered before strings, sorted numerically.
+    // Document keys are ordered first by numeric value ("__id<Long>__"),
+    // then lexicographically by string value. Start the iterator at the minimum
+    // possible Document key value.
     const prefix = new DocumentKey(
-      collectionPath.child('__id' + Number.MIN_SAFE_INTEGER + '__')
+      collectionPath.child('__id' + MIN_LONG_VALUE + '__')
     );
     const iterator = this.docs.getIteratorFrom(prefix);
     while (iterator.hasNext()) {

diff --git a/packages/firestore/src/model/path.ts b/packages/firestore/src/model/path.ts
@@ -17,6 +17,7 @@
 
 import { debugAssert, fail } from '../util/assert';
 import { Code, FirestoreError } from '../util/error';
+import { Integer } from '@firebase/webchannel-wrapper/bloom-blob';
 
 export const DOCUMENT_KEY_NAME = '__name__';
 
@@ -194,8 +195,8 @@ abstract class BasePath<B extends BasePath<B>> {
       return 1;
     } else if (isLhsNumeric && isRhsNumeric) {
       // both numeric
-      return Math.sign(
-        BasePath.extractNumericId(lhs) - BasePath.extractNumericId(rhs)
+      return BasePath.extractNumericId(lhs).compare(
+        BasePath.extractNumericId(rhs)
       );
     } else {
       // both non-numeric
@@ -214,8 +215,8 @@ abstract class BasePath<B extends BasePath<B>> {
     return segment.startsWith('__id') && segment.endsWith('__');
   }
 
-  private static extractNumericId(segment: string): number {
-    return parseInt(segment.substring(4, segment.length - 2), 10);
+  private static extractNumericId(segment: string): Integer {
+    return Integer.fromString(segment.substring(4, segment.length - 2));
   }
 }
 

diff --git a/packages/firestore/test/integration/api/database.test.ts b/packages/firestore/test/integration/api/database.test.ts
@@ -2259,15 +2259,25 @@ apiDescribe('Database', persistence => {
         __id12__: { a: 1 },
         '__id-2__': { a: 1 },
         '_id1__': { a: 1 },
-        '__id1_': { a: 1 }
+        '__id1_': { a: 1 },
+        // max safe integer +1, +2
+        '__id9007199254740992__': { a: 1 },
+        '__id9007199254740993__': { a: 2 },
+        // smallest long numbers
+        '__id-9223372036854775808__': { a: 3 },
+        '__id-9223372036854775807__': { a: 4 }
       };
 
       return withTestCollection(persistence, testDocs, async collectionRef => {
         const orderedQuery = query(collectionRef, orderBy(documentId()));
         const expectedDocs = [
+          '__id-9223372036854775808__',
+          '__id-9223372036854775807__',
           '__id-2__',
           '__id7__',
           '__id12__',
+          '__id9007199254740992__',
+          '__id9007199254740993__',
           '12',
           '7',
           'A',
@@ -2284,6 +2294,7 @@ apiDescribe('Database', persistence => {
         const unsubscribe = onSnapshot(orderedQuery, storeEvent.storeEvent);
         const watchSnapshot = await storeEvent.awaitEvent();
         expect(toIds(watchSnapshot)).to.deep.equal(expectedDocs);
+
         unsubscribe();
       });
     });
@@ -2299,7 +2310,13 @@ apiDescribe('Database', persistence => {
         __id12__: { a: 1 },
         '__id-2__': { a: 1 },
         '_id1__': { a: 1 },
-        '__id1_': { a: 1 }
+        '__id1_': { a: 1 },
+        // max safe integer +1, +2
+        '__id9007199254740992__': { a: 1 },
+        '__id9007199254740993__': { a: 2 },
+        // smallest long numbers
+        '__id-9223372036854775808__': { a: 3 },
+        '__id-9223372036854775807__': { a: 4 }
       };
 
       return withTestCollection(persistence, testDocs, async collectionRef => {
@@ -2309,7 +2326,15 @@ apiDescribe('Database', persistence => {
           where(documentId(), '>', '__id7__'),
           where(documentId(), '<=', 'Aa')
         );
-        const expectedDocs = ['__id12__', '12', '7', 'A', 'Aa'];
+        const expectedDocs = [
+          '__id12__',
+          '__id9007199254740992__',
+          '__id9007199254740993__',
+          '12',
+          '7',
+          'A',
+          'Aa'
+        ];
 
         const getSnapshot = await getDocsFromServer(filteredQuery);
         expect(toIds(getSnapshot)).to.deep.equal(expectedDocs);
@@ -2335,18 +2360,28 @@ apiDescribe('Database', persistence => {
           __id12__: { a: 1 },
           '__id-2__': { a: 1 },
           '_id1__': { a: 1 },
-          '__id1_': { a: 1 }
+          '__id1_': { a: 1 },
+          // max safe integer +1, +2
+          '__id9007199254740992__': { a: 1 },
+          '__id9007199254740993__': { a: 2 },
+          // smallest long numbers
+          '__id-9223372036854775808__': { a: 3 },
+          '__id-9223372036854775807__': { a: 4 }
         };
 
         return withTestCollection(
-          persistence,
+          persistence.toLruGc(),
           testDocs,
           async collectionRef => {
             const orderedQuery = query(collectionRef, orderBy(documentId()));
             let expectedDocs = [
+              '__id-9223372036854775808__',
+              '__id-9223372036854775807__',
               '__id-2__',
               '__id7__',
               '__id12__',
+              '__id9007199254740992__',
+              '__id9007199254740993__',
               '12',
               '7',
               'A',
@@ -2366,7 +2401,15 @@ apiDescribe('Database', persistence => {
               where(documentId(), '>', '__id7__'),
               where(documentId(), '<=', 'Aa')
             );
-            expectedDocs = ['__id12__', '12', '7', 'A', 'Aa'];
+            expectedDocs = [
+              '__id12__',
+              '__id9007199254740992__',
+              '__id9007199254740993__',
+              '12',
+              '7',
+              'A',
+              'Aa'
+            ];
             await checkOnlineAndOfflineResultsMatch(
               filteredQuery,
               ...expectedDocs