Skip to content

Fix Firestore failing to return empty results from the local cache #5897

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from

Conversation

dconeybe
Copy link
Contributor

Fix a bug where Firestore fails to return results from the local cache if the result set was empty.

This bug is due to the following logic in event_manager.ts:

// Raise data from cache if we have any documents or we are offline
return !snap.docs.isEmpty() || onlineState === OnlineState.Offline;
where it

which assumes that if the result set from the local cache is empty that there is no cached result; however, if the result set of the query is indeed empty then it should be returning that empty result set from the cache.

This fix improves the logic to use the presence of a resume token for the query to indicate that the empty result set is cached data and should be raised to the client.

Fixed #5873

@dconeybe dconeybe self-assigned this Jan 17, 2022
@changeset-bot
Copy link

changeset-bot bot commented Jan 17, 2022

⚠️ No Changeset found

Latest commit: dfb45ad

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@dconeybe dconeybe force-pushed the dconeybe/CacheEmptyQueryResults branch from de3c0da to dfb45ad Compare September 19, 2022 20:03
@google-oss-bot
Copy link
Contributor

Size Analysis Report 1

Affected Products

  • @firebase/firestore

    • enableMultiTabIndexedDbPersistence

      Size

      TypeBase (e35db6f)Merge (3ef2847)Diff
      size206 kB207 kB+349 B (+0.2%)
      size-with-ext-deps266 kB267 kB+349 B (+0.1%)
    • getDoc

      Size

      TypeBase (e35db6f)Merge (3ef2847)Diff
      size132 kB133 kB+362 B (+0.3%)
      size-with-ext-deps192 kB192 kB+362 B (+0.2%)
    • getDocFromServer

      Size

      TypeBase (e35db6f)Merge (3ef2847)Diff
      size132 kB133 kB+362 B (+0.3%)
      size-with-ext-deps192 kB192 kB+362 B (+0.2%)
    • getDocs

      Size

      TypeBase (e35db6f)Merge (3ef2847)Diff
      size134 kB134 kB+362 B (+0.3%)
      size-with-ext-deps193 kB194 kB+362 B (+0.2%)
    • getDocsFromCache

      Size

      TypeBase (e35db6f)Merge (3ef2847)Diff
      size96.3 kB96.6 kB+309 B (+0.3%)
      size-with-ext-deps155 kB156 kB+309 B (+0.2%)
    • getDocsFromServer

      Size

      TypeBase (e35db6f)Merge (3ef2847)Diff
      size134 kB134 kB+362 B (+0.3%)
      size-with-ext-deps193 kB194 kB+362 B (+0.2%)
    • onSnapshot

      Size

      TypeBase (e35db6f)Merge (3ef2847)Diff
      size134 kB135 kB+362 B (+0.3%)
      size-with-ext-deps194 kB194 kB+362 B (+0.2%)
    • onSnapshotsInSync

      Size

      TypeBase (e35db6f)Merge (3ef2847)Diff
      size124 kB124 kB+314 B (+0.3%)
      size-with-ext-deps183 kB184 kB+314 B (+0.2%)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/wqHd6oBDMp.html

@dconeybe
Copy link
Contributor Author

dconeybe commented Oct 7, 2022

Closing since this PR was obsoleted by #6624

@dconeybe dconeybe closed this Oct 7, 2022
@dconeybe dconeybe deleted the dconeybe/CacheEmptyQueryResults branch October 7, 2022 19:40
@firebase firebase locked and limited conversation to collaborators Nov 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Collection snapshot queries with no documents don't store anything in cache. Always requires server snapshot.
2 participants