Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix CSI timestamp issue #8090

Merged
merged 9 commits into from
Mar 28, 2024
Merged

Fix CSI timestamp issue #8090

merged 9 commits into from
Mar 28, 2024

Conversation

milaGGL
Copy link
Contributor

@milaGGL milaGGL commented Mar 22, 2024 •
edited
Loading

fix #8031
Problem:
CSI returns incorrect results for query that indexes Timestamp fields, because of the format difference, ie, 2024-01-02T03:04:05Z vs 2024-01-02T03:04:05.00000000Z.

Fix:

  • unify the timestamp format before creating indexeddb index entries.
  • Clear the possibly corrupted index entries

@milaGGL milaGGL self-assigned this Mar 22, 2024
@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Mar 22, 2024 •
edited
Loading

🦋 Changeset detected

Latest commit: 7fb399b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@firebase/firestore Patch
firebase Patch
@firebase/firestore-compat Patch

Not sure what this means? Click here to learn what changesets are.

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

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Mar 25, 2024 •
edited
Loading

Size Report 1

Affected Products

  • @firebase/firestore

    TypeBase (c8a2568)Merge (7d03ca9)Diff
    browser376 kB376 kB+161 B (+0.0%)
    esm5362 kB362 kB+150 B (+0.0%)
    main579 kB580 kB+303 B (+0.1%)
    module376 kB376 kB+161 B (+0.0%)
    react-native376 kB377 kB+162 B (+0.0%)

  • bundle

    TypeBase (c8a2568)Merge (7d03ca9)Diff
    firestore (CSI Auto Indexing Disable and Delete)268 kB268 kB+136 B (+0.1%)
    firestore (CSI Auto Indexing Enable)268 kB268 kB+136 B (+0.1%)
    firestore (Persistence)303 kB303 kB+136 B (+0.0%)
    firestore (Read Write w Persistence)323 kB323 kB+136 B (+0.0%)

  • firebase

    TypeBase (c8a2568)Merge (7d03ca9)Diff
    firebase-compat.js786 kB786 kB+167 B (+0.0%)
    firebase-firestore-compat.js342 kB342 kB+167 B (+0.0%)
    firebase-firestore.js436 kB436 kB+162 B (+0.0%)

Test Logs

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

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Mar 25, 2024 •
edited
Loading

Size Analysis Report 1

This report is too large (98,221 characters) to be displayed here in a GitHub comment. Please use the below link to see the full report on Google Cloud Storage.

Test Logs

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

@milaGGL milaGGL marked this pull request as ready for review March 25, 2024 17:45
@milaGGL milaGGL requested review from a team as code owners March 25, 2024 17:45
wu-hui
wu-hui reviewed Mar 26, 2024
View reviewed changes
Copy link
Contributor

@wu-hui wu-hui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix! Can I also ask you do add some tests to index_manager.test.ts, where we simply reproduce the user's case.

packages/firestore/test/unit/index/firestore_index_value_writer.test.ts Outdated Show resolved Hide resolved
packages/firestore/test/unit/index/firestore_index_value_writer.test.ts Show resolved Hide resolved
@milaGGL milaGGL requested a review from wu-hui March 27, 2024 16:03
wu-hui
wu-hui approved these changes Mar 27, 2024
View reviewed changes
Copy link
Contributor

@wu-hui wu-hui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work!

@wu-hui
Copy link
Contributor

wu-hui commented Mar 27, 2024

Good work!

Can you add an entry to release note?

@milaGGL milaGGL requested a review from a team as a code owner March 27, 2024 18:34
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Mar 27, 2024 •
edited
Loading

Changeset File Check ✅

  • No modified packages are missing from the changeset file.
  • No changeset formatting errors detected.

@milaGGL milaGGL removed the request for review from MarkDuckworth March 27, 2024 19:13
@milaGGL milaGGL merged commit c6ecac8 into master Mar 28, 2024
44 checks passed
@milaGGL milaGGL deleted the mila/fix-CSI-timestamp-issue branch March 28, 2024 17:30
@google-oss-bot google-oss-bot mentioned this pull request Apr 9, 2024
Merged
@firebase firebase locked and limited conversation to collaborators Apr 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@hsubox76 hsubox76 hsubox76 approved these changes

@wu-hui wu-hui wu-hui approved these changes

Assignees

@MarkDuckworth MarkDuckworth

@wu-hui wu-hui

@milaGGL milaGGL

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Equality check for timestamp fields fails when caching is active in a web worker.
5 participants
@milaGGL @google-oss-bot @wu-hui @hsubox76 @MarkDuckworth