switch from documentType to document.createTime

dconeybe · dconeybe · commit a4bcfa5b0876 · 2025-09-11T21:37:00.000-04:00
diff --git a/packages/firestore/src/local/indexeddb_remote_document_cache.ts b/packages/firestore/src/local/indexeddb_remote_document_cache.ts
@@ -36,12 +36,7 @@ import { SortedSet } from '../util/sorted_set';
 
 import { IndexManager } from './index_manager';
 import { dbDocumentSize } from './indexeddb_mutation_batch_impl';
-import {
-  DbRemoteDocument,
-  DbRemoteDocumentType,
-  DbRemoteDocumentGlobal,
-  DbRemoteDocumentCollectionIndexKey
-} from './indexeddb_schema';
+import { DbRemoteDocument, DbRemoteDocumentGlobal } from './indexeddb_schema';
 import {
   DbRemoteDocumentCollectionGroupIndex,
   DbRemoteDocumentDocumentKeyIndex,
@@ -50,6 +45,9 @@ import {
   DbRemoteDocumentKey,
   DbRemoteDocumentStore,
   DbRemoteDocumentCollectionIndex,
+  DbRemoteDocumentCollectionIndexKey,
+  MAX_CREATE_TIME,
+  MIN_CREATE_TIME,
   DbTimestampKey
 } from './indexeddb_sentinels';
 import { getStore } from './indexeddb_transaction';
@@ -291,7 +289,7 @@ class IndexedDbRemoteDocumentCacheImpl implements IndexedDbRemoteDocumentCache {
   ): PersistencePromise<MutableDocumentMap> {
     const collection = query.path;
     const startKey: DbRemoteDocumentCollectionIndexKey = [
-      DbRemoteDocumentType.FoundDocument,
+      MIN_CREATE_TIME,
       collection.popLast().toArray(),
       collection.lastSegment(),
       toDbTimestampKey(offset.readTime),
@@ -300,7 +298,7 @@ class IndexedDbRemoteDocumentCacheImpl implements IndexedDbRemoteDocumentCache {
         : offset.documentKey.path.lastSegment()
     ];
     const endKey: DbRemoteDocumentCollectionIndexKey = [
-      DbRemoteDocumentType.FoundDocument,
+      MAX_CREATE_TIME,
       collection.popLast().toArray(),
       collection.lastSegment(),
       [Number.MAX_SAFE_INTEGER, Number.MAX_SAFE_INTEGER],
diff --git a/packages/firestore/src/local/indexeddb_schema.ts b/packages/firestore/src/local/indexeddb_schema.ts
@@ -186,26 +186,6 @@ export interface DbUnknownDocument {
   version: DbTimestamp;
 }
 
-/**
- * The "type" of a document stored in a `DbRemoteDocument`.
- */
-export enum DbRemoteDocumentType {
-  /**
-   * The `noDocument` property of the `DbRemoteDocument` is set.
-   */
-  NoDocument = 1,
-
-  /**
-   * The `document` property of the `DbRemoteDocument` is set.
-   */
-  FoundDocument = 2,
-
-  /**
-   * The `unknownDocument` property of the `DbRemoteDocument` is set.
-   */
-  UnknownDocument = 3
-}
-
 /**
  * An object to be stored in the 'remoteDocuments' store in IndexedDb.
  * It represents either:
@@ -258,31 +238,8 @@ export interface DbRemoteDocument {
    * the write's commit version as their document version.
    */
   hasCommittedMutations: boolean;
-  /**
-   * The "type" of document stored in this object.
-   *
-   * The value of this property _must_ be consistent with the semantics of the
-   * `unknownDocument`, `noDocument`, and `document` properties. This property
-   * is provided as an optimization to allow skipping entries in result sets
-   * whose documents are not of the desired "type".
-   *
-   * This property was added in a schema migration at version 19. Documents
-   * written prior to this version has this field set to 0 (zero).
-   */
-  documentType: DbRemoteDocumentType;
 }
 
-/**
- * A key in the `DbRemoteDocumentCollectionIndex` index.
- */
-export type DbRemoteDocumentCollectionIndexKey = [
-  /** document type */ DbRemoteDocumentType,
-  /** path to collection */ string[],
-  /** collection group */ string,
-  /** read time */ DbTimestampKey,
-  /** document ID */ string
-];
-
 /**
  * Contains a single entry that has metadata about the remote document cache.
  */
diff --git a/packages/firestore/src/local/indexeddb_schema_converter.ts b/packages/firestore/src/local/indexeddb_schema_converter.ts
@@ -44,7 +44,6 @@ import {
   DbMutationQueue,
   DbRemoteDocument,
   DbRemoteDocumentGlobal,
-  DbRemoteDocumentType,
   DbTarget,
   DbTargetDocument,
   DbTargetGlobal,
@@ -300,29 +299,7 @@ export class SchemaConverter implements SimpleDbSchemaConverter {
 
     if (fromVersion < 19 && toVersion >= 19) {
       p = p.next(() => {
-        {
-          const startTime = performance.now();
-          console.log(
-            'zzyzx createRemoteDocumentStoreDocumentTypeIndex starting'
-          );
-          createRemoteDocumentStoreDocumentTypeIndex(txn);
-          console.log(
-            'zzyzx createRemoteDocumentStoreDocumentTypeIndex completed: elapsedTime:',
-            performance.now() - startTime
-          );
-        }
-        {
-          const startTime = performance.now();
-          console.log('zzyzx backfillRemoteDocumentStoreDocumentType starting');
-          return backfillRemoteDocumentStoreDocumentType(
-            simpleDbTransaction
-          ).next(() => {
-            console.log(
-              'zzyzx backfillRemoteDocumentStoreDocumentType completed: elapsedTime:',
-              performance.now() - startTime
-            );
-          });
-        }
+        createRemoteDocumentCollectionIndex(txn);
       });
     }
 
@@ -525,8 +502,6 @@ export class SchemaConverter implements SimpleDbSchemaConverter {
 
         const path = extractKey(legacyDocument).path.toArray();
 
-        // Note: omit the "documentType" property because it will be populated
-        // by schema migration 19.
         const dbRemoteDocument = {
           prefixPath: path.slice(0, path.length - 2),
           collectionGroup: path[path.length - 2],
@@ -536,7 +511,7 @@ export class SchemaConverter implements SimpleDbSchemaConverter {
           noDocument: legacyDocument.noDocument,
           document: legacyDocument.document,
           hasCommittedMutations: !!legacyDocument.hasCommittedMutations
-        } satisfies Omit<DbRemoteDocument, 'documentType'> as DbRemoteDocument;
+        };
         writes.push(remoteDocumentStore.put(dbRemoteDocument));
       })
       .next(() => PersistencePromise.waitFor(writes));
@@ -830,30 +805,11 @@ function extractKey(remoteDoc: DbRemoteDocumentLegacy): DocumentKey {
   }
 }
 
-function createRemoteDocumentStoreDocumentTypeIndex(txn: IDBTransaction): void {
+function createRemoteDocumentCollectionIndex(txn: IDBTransaction): void {
   txn
     .objectStore(DbRemoteDocumentStore)
     .createIndex(
       DbRemoteDocumentCollectionIndex,
-      DbRemoteDocumentCollectionIndexPath,
-      { unique: false }
+      DbRemoteDocumentCollectionIndexPath
     );
 }
-
-function backfillRemoteDocumentStoreDocumentType(
-  txn: SimpleDbTransaction
-): PersistencePromise<void> {
-  const remoteDocumentStore = txn.store<DbRemoteDocumentKey, DbRemoteDocument>(
-    DbRemoteDocumentStore
-  );
-
-  return remoteDocumentStore.iterate((key, doc) => {
-    doc.documentType = doc.noDocument
-      ? DbRemoteDocumentType.NoDocument
-      : doc.unknownDocument
-      ? DbRemoteDocumentType.UnknownDocument
-      : DbRemoteDocumentType.FoundDocument;
-
-    return remoteDocumentStore.put(doc);
-  });
-}
diff --git a/packages/firestore/src/local/indexeddb_sentinels.ts b/packages/firestore/src/local/indexeddb_sentinels.ts
@@ -173,25 +173,58 @@ export const DbRemoteDocumentCollectionGroupIndexPath = [
 ];
 
 /**
- * An index that provides access to documents by collection, read time, and
- * document type.
+ * An index that provides access to documents by the primary key, but filtering
+ * out all but "known" documents.
  *
  * This index is used as an optimization in getDocumentsMatchingQuery() to
  * avoid visiting "tombstone" entries which are impossible to be part of the
- * query result anyway.
+ * query result anyway. For background information and motivation, see
+ * https://github.com/firebase/firebase-android-sdk/issues/7295.
+ *
+ * This "filtering" is achieved as a side effect of including a property from
+ * the "document" property in the index's key. Since the "document" property
+ * is _only_ present for "found" documents, indexing one of its keys has the
+ * convenient side effect of excluding all entries that lack a "document"
+ * property. The "createTime" sub-property was chosen for several reasons:
+ *
+ * 1. Its length is strictly bounded. This provides a predictable O(1) size of
+ *    the index entries relative to the size of the primary key. This is because
+ *    the "createTime" values are RFC3339 strings (e.g. "2023-09-24T15:30:00Z").
+ * 2. It is easy to pick a "smallest" and "largest" value for "createTime",
+ *    which is critical when creating bounded IndexedDB key ranges. Namely,
+ *    "" (the empty string) is smaller than any value and "\uffff" is larger
+ *    than any value, guaranteed. This cannot be said about arbitrary strings.
  *
- * See https://github.com/firebase/firebase-android-sdk/issues/7295.
  */
 export const DbRemoteDocumentCollectionIndex = 'collectionIndex';
 
 export const DbRemoteDocumentCollectionIndexPath = [
-  'documentType',
+  'document.createTime',
   'prefixPath',
   'collectionGroup',
   'readTime',
   'documentId'
 ];
 
+export const MIN_CREATE_TIME = '' as const;
+export const MAX_CREATE_TIME = '\uFFFF' as const;
+
+/**
+ * A key in the `DbRemoteDocumentCollectionIndex` index.
+ *
+ * Note that the "document.createTime" entry is restricted to the min and max
+ * values as they are only intended to be used for _filtering_ entries that
+ * contain the "document" property and are not intended for general-purpose
+ * filtering.
+ */
+export type DbRemoteDocumentCollectionIndexKey = [
+  /** document.createTime */ typeof MIN_CREATE_TIME | typeof MAX_CREATE_TIME,
+  /** path to collection */ string[],
+  /** collection group */ string,
+  /** read time */ DbTimestampKey,
+  /** document ID */ string
+];
+
 export const DbRemoteDocumentGlobalStore = 'remoteDocumentGlobal';
 
 export const DbRemoteDocumentGlobalKey = 'remoteDocumentGlobalKey';
diff --git a/packages/firestore/src/local/local_serializer.ts b/packages/firestore/src/local/local_serializer.ts
@@ -66,7 +66,6 @@ import {
   DbNamedQuery,
   DbQuery,
   DbRemoteDocument,
-  DbRemoteDocumentType,
   DbTarget,
   DbTimestamp
 } from './indexeddb_schema';
@@ -118,48 +117,30 @@ export function toDbRemoteDocument(
   document: MutableDocument
 ): DbRemoteDocument {
   const key = document.key;
-  return {
+  const remoteDoc: DbRemoteDocument = {
     prefixPath: key.getCollectionPath().popLast().toArray(),
     collectionGroup: key.collectionGroup,
     documentId: key.path.lastSegment(),
     readTime: toDbTimestampKey(document.readTime),
-    hasCommittedMutations: document.hasCommittedMutations,
-    ...toDbRemoteDocumentDocumentSpecificComponents(localSerializer, document)
+    hasCommittedMutations: document.hasCommittedMutations
   };
-}
 
-function toDbRemoteDocumentDocumentSpecificComponents(
-  localSerializer: LocalSerializer,
-  document: MutableDocument
-): Pick<
-  DbRemoteDocument,
-  'documentType' | 'document' | 'noDocument' | 'unknownDocument'
-> {
   if (document.isFoundDocument()) {
-    return {
-      documentType: DbRemoteDocumentType.FoundDocument,
-      document: toDocument(localSerializer.remoteSerializer, document)
+    remoteDoc.document = toDocument(localSerializer.remoteSerializer, document);
+  } else if (document.isNoDocument()) {
+    remoteDoc.noDocument = {
+      path: key.path.toArray(),
+      readTime: toDbTimestamp(document.version)
     };
-  }
-  if (document.isNoDocument()) {
-    return {
-      documentType: DbRemoteDocumentType.NoDocument,
-      noDocument: {
-        path: document.key.path.toArray(),
-        readTime: toDbTimestamp(document.version)
-      }
-    };
-  }
-  if (document.isUnknownDocument()) {
-    return {
-      documentType: DbRemoteDocumentType.UnknownDocument,
-      unknownDocument: {
-        path: document.key.path.toArray(),
-        version: toDbTimestamp(document.version)
-      }
+  } else if (document.isUnknownDocument()) {
+    remoteDoc.unknownDocument = {
+      path: key.path.toArray(),
+      version: toDbTimestamp(document.version)
     };
+  } else {
+    return fail(0xe230, 'Unexpected Document', { document });
   }
-  fail(0xe230, 'Unexpected Document', { document });
+  return remoteDoc;
 }
 
 export function toDbTimestampKey(
