Simplify code and improve test coverage #7512

milaGGL · 2023-08-01T18:45:33Z

simplify the code to get inequality fields
add unit tests for implicit ordering on inequality fields
remove unnecessary validation tests on multiple inequality

TODO: next PR

modify the test for aggregate query
add negative test cases

changeset-bot · 2023-08-01T18:45:37Z

⚠️ No Changeset found

Latest commit: bdcfeb6

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

google-oss-bot · 2023-08-01T19:11:28Z

Size Report ¹

Affected Products

@firebase/firestore
Type Base (63698a7) Merge (381a1ac) Diff
browser 369 kB 369 kB -140 B (-0.0%)
esm5 355 kB 355 kB -218 B (-0.1%)
main 568 kB 567 kB -172 B (-0.0%)
module 369 kB 369 kB -140 B (-0.0%)
react-native 369 kB 369 kB -140 B (-0.0%)
@firebase/firestore-lite
Type Base (63698a7) Merge (381a1ac) Diff
browser 109 kB 109 kB -140 B (-0.1%)
esm5 106 kB 106 kB -218 B (-0.2%)
main 149 kB 149 kB -172 B (-0.1%)
module 109 kB 109 kB -140 B (-0.1%)
react-native 109 kB 109 kB -140 B (-0.1%)

`bundle`

Type	Base (`63698a7`)	Merge (`381a1ac`)	Diff
firestore (Persistence)	301 kB	301 kB	-140 B (-0.0%)
firestore (Query Cursors)	238 kB	238 kB	-140 B (-0.1%)
firestore (Query)	236 kB	235 kB	-140 B (-0.1%)
firestore (Read data once)	224 kB	224 kB	-140 B (-0.1%)
firestore (Realtime updates)	226 kB	226 kB	-140 B (-0.1%)
firestore (Transaction)	203 kB	202 kB	-140 B (-0.1%)
firestore (Write data)	202 kB	202 kB	-140 B (-0.1%)
firestore-lite (Query Cursors)	89.0 kB	88.8 kB	-140 B (-0.2%)
firestore-lite (Query)	85.1 kB	85.0 kB	-140 B (-0.2%)

`firebase`

Type	Base (`63698a7`)	Merge (`381a1ac`)	Diff
firebase-compat.js	773 kB	773 kB	-136 B (-0.0%)
firebase-firestore-compat.js	339 kB	339 kB	-136 B (-0.0%)
firebase-firestore-lite.js	116 kB	116 kB	-140 B (-0.1%)
firebase-firestore.js	429 kB	429 kB	-140 B (-0.0%)

Test Logs

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

milaGGL · 2023-08-01T19:13:15Z

packages/firestore/test/unit/core/query.test.ts

@@ -777,6 +778,82 @@ describe('Query', () => {
    );
  });

+  it("generates the correct implicit order by's for multiple inequality", () => {


double checked with backend result: https://cider-v.corp.google.com/?ws=milamamat/CS-QueryToConstraintSpecConverterTest-2023-07-31_173943&sid=7af76dfd-ea54-4bdc-8a62-7e5d4550dd35

👍 Thanks

google-oss-bot · 2023-08-01T19:23:01Z

Size Analysis Report ¹

Affected Products

@firebase/firestore
- QueryCompositeFilterConstraint
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 20.0 kB 19.9 kB -87 B (-0.4%)
  size-with-ext-deps 89.6 kB 89.5 kB -87 B (-0.1%)
- QueryEndAtConstraint
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 43.3 kB 43.3 kB +7 B (+0.0%)
  size-with-ext-deps 113 kB 113 kB +7 B (+0.0%)
- QueryFieldFilterConstraint
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 41.3 kB 41.2 kB -60 B (-0.1%)
  size-with-ext-deps 111 kB 111 kB -60 B (-0.1%)
- QueryStartAtConstraint
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 43.3 kB 43.3 kB +7 B (+0.0%)
  size-with-ext-deps 113 kB 113 kB +7 B (+0.0%)
- addDoc
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 122 kB 122 kB -140 B (-0.1%)
  size-with-ext-deps 192 kB 192 kB -140 B (-0.1%)
- aggregateQuerySnapshotEqual
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 40.7 kB 40.6 kB -140 B (-0.3%)
  size-with-ext-deps 111 kB 111 kB -140 B (-0.1%)
- and
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 42.7 kB 42.6 kB -147 B (-0.3%)
  size-with-ext-deps 112 kB 112 kB -147 B (-0.1%)
- deleteDoc
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 113 kB 113 kB -140 B (-0.1%)
  size-with-ext-deps 183 kB 183 kB -140 B (-0.1%)
- disableNetwork
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 101 kB 101 kB -140 B (-0.1%)
  size-with-ext-deps 171 kB 171 kB -140 B (-0.1%)
- enableIndexedDbPersistence
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 184 kB 184 kB -140 B (-0.1%)
  size-with-ext-deps 255 kB 255 kB -140 B (-0.1%)
- enableMultiTabIndexedDbPersistence
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 221 kB 220 kB -140 B (-0.1%)
  size-with-ext-deps 292 kB 292 kB -140 B (-0.0%)
- enableNetwork
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 101 kB 101 kB -140 B (-0.1%)
  size-with-ext-deps 171 kB 171 kB -140 B (-0.1%)
- endAt
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 43.3 kB 43.3 kB +7 B (+0.0%)
  size-with-ext-deps 113 kB 113 kB +7 B (+0.0%)
- endBefore
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 43.3 kB 43.3 kB +7 B (+0.0%)
  size-with-ext-deps 113 kB 113 kB +7 B (+0.0%)
- executeWrite
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 112 kB 112 kB -140 B (-0.1%)
  size-with-ext-deps 182 kB 182 kB -140 B (-0.1%)
- getAggregateFromServer
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 108 kB 108 kB -140 B (-0.1%)
  size-with-ext-deps 179 kB 178 kB -140 B (-0.1%)
- getCountFromServer
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 108 kB 108 kB -140 B (-0.1%)
  size-with-ext-deps 179 kB 179 kB -140 B (-0.1%)
- getDoc
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 143 kB 143 kB -140 B (-0.1%)
  size-with-ext-deps 213 kB 213 kB -140 B (-0.1%)
- getDocFromCache
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 93.5 kB 93.3 kB -140 B (-0.1%)
  size-with-ext-deps 163 kB 163 kB -140 B (-0.1%)
- getDocFromServer
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 143 kB 143 kB -140 B (-0.1%)
  size-with-ext-deps 213 kB 213 kB -140 B (-0.1%)
- getDocs
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 145 kB 145 kB -140 B (-0.1%)
  size-with-ext-deps 215 kB 215 kB -140 B (-0.1%)
- getDocsFromCache
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 101 kB 101 kB -140 B (-0.1%)
  size-with-ext-deps 171 kB 171 kB -140 B (-0.1%)
- getDocsFromServer
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 144 kB 144 kB -140 B (-0.1%)
  size-with-ext-deps 215 kB 215 kB -140 B (-0.1%)
- loadBundle
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 111 kB 111 kB -140 B (-0.1%)
  size-with-ext-deps 181 kB 181 kB -140 B (-0.1%)
- memoryEagerGarbageCollector
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 84.0 kB 83.9 kB -140 B (-0.2%)
  size-with-ext-deps 154 kB 154 kB -140 B (-0.1%)
- memoryLocalCache
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 98.3 kB 98.2 kB -140 B (-0.1%)
  size-with-ext-deps 169 kB 168 kB -140 B (-0.1%)
- memoryLruGarbageCollector
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 90.5 kB 90.4 kB -140 B (-0.2%)
  size-with-ext-deps 160 kB 160 kB -140 B (-0.1%)
- namedQuery
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 86.9 kB 86.8 kB -140 B (-0.2%)
  size-with-ext-deps 157 kB 157 kB -140 B (-0.1%)
- onSnapshot
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 145 kB 145 kB -140 B (-0.1%)
  size-with-ext-deps 216 kB 215 kB -140 B (-0.1%)
- onSnapshotsInSync
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 135 kB 135 kB -140 B (-0.1%)
  size-with-ext-deps 205 kB 205 kB -140 B (-0.1%)
- or
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 42.7 kB 42.6 kB -147 B (-0.3%)
  size-with-ext-deps 112 kB 112 kB -147 B (-0.1%)
- persistentLocalCache
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 181 kB 181 kB -140 B (-0.1%)
  size-with-ext-deps 253 kB 252 kB -140 B (-0.1%)
- persistentMultipleTabManager
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 217 kB 217 kB -140 B (-0.1%)
  size-with-ext-deps 289 kB 289 kB -140 B (-0.0%)
- persistentSingleTabManager
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 181 kB 180 kB -140 B (-0.1%)
  size-with-ext-deps 252 kB 252 kB -140 B (-0.1%)
- query
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 43.0 kB 42.8 kB -147 B (-0.3%)
  size-with-ext-deps 113 kB 113 kB -147 B (-0.1%)
- queryEqual
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 40.6 kB 40.5 kB -140 B (-0.3%)
  size-with-ext-deps 110 kB 110 kB -140 B (-0.1%)
- runTransaction
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 122 kB 122 kB -140 B (-0.1%)
  size-with-ext-deps 192 kB 192 kB -140 B (-0.1%)
- setDoc
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 121 kB 121 kB -140 B (-0.1%)
  size-with-ext-deps 191 kB 191 kB -140 B (-0.1%)
- setIndexConfiguration
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 89.6 kB 89.4 kB -140 B (-0.2%)
  size-with-ext-deps 159 kB 159 kB -140 B (-0.1%)
- snapshotEqual
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 46.3 kB 46.1 kB -140 B (-0.3%)
  size-with-ext-deps 116 kB 116 kB -140 B (-0.1%)
- startAfter
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 43.3 kB 43.3 kB +7 B (+0.0%)
  size-with-ext-deps 113 kB 113 kB +7 B (+0.0%)
- startAt
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 43.3 kB 43.3 kB +7 B (+0.0%)
  size-with-ext-deps 113 kB 113 kB +7 B (+0.0%)
- updateDoc
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 122 kB 121 kB -140 B (-0.1%)
  size-with-ext-deps 192 kB 192 kB -140 B (-0.1%)
- waitForPendingWrites
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 102 kB 102 kB -140 B (-0.1%)
  size-with-ext-deps 172 kB 172 kB -140 B (-0.1%)
- where
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 42.1 kB 42.0 kB -60 B (-0.1%)
  size-with-ext-deps 112 kB 112 kB -60 B (-0.1%)
- writeBatch
  Size
  Type Base (63698a7) Merge (381a1ac) Diff
  size 124 kB 123 kB -140 B (-0.1%)
  size-with-ext-deps 194 kB 194 kB -140 B (-0.1%)

Test Logs

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

…ests

update tests

c827ca1

milaGGL changed the base branch from master to mila/Multiple-Inequality-support August 1, 2023 18:45

milaGGL commented Aug 1, 2023

View reviewed changes

milaGGL added 2 commits August 1, 2023 15:44

Update query.test.ts

21b2919

improvise the code

d57f1ab

milaGGL changed the title ~~Mila/polish up tests~~ simplify code and add test coverage Aug 1, 2023

milaGGL marked this pull request as ready for review August 1, 2023 21:09

milaGGL requested review from a team as code owners August 1, 2023 21:09

milaGGL requested a review from ehsannas August 1, 2023 21:09

Merge branch 'mila/Multiple-Inequality-support' into mila/polish-up-t…

c237dce

…ests

This was referenced Aug 2, 2023

Multiple inequality support firebase/firebase-ios-sdk#11626

Merged

Simplify code and improve test coverage firebase/firebase-android-sdk#5220

Merged

use FieldPath.fromServerFormat for code simplicity

bdcfeb6

milaGGL changed the title ~~simplify code and add test coverage~~ Simplify code and improve test coverage Aug 3, 2023

ehsannas approved these changes Aug 4, 2023

View reviewed changes

ehsannas assigned milaGGL Aug 4, 2023

milaGGL merged commit 59796bc into mila/Multiple-Inequality-support Aug 8, 2023

milaGGL deleted the mila/polish-up-tests branch August 8, 2023 14:27

firebase locked and limited conversation to collaborators Sep 8, 2023

Simplify code and improve test coverage #7512

Simplify code and improve test coverage #7512

Conversation

milaGGL commented Aug 1, 2023 • edited by ehsannas Loading

changeset-bot bot commented Aug 1, 2023 • edited Loading

⚠️ No Changeset found

google-oss-bot commented Aug 1, 2023 • edited Loading

Size Report 1

Affected Products

@firebase/firestore

@firebase/firestore-lite

bundle

firebase

Test Logs

milaGGL Aug 1, 2023

Choose a reason for hiding this comment

ehsannas Aug 4, 2023

Choose a reason for hiding this comment

google-oss-bot commented Aug 1, 2023 • edited Loading

Size Analysis Report 1

Affected Products

@firebase/firestore

QueryCompositeFilterConstraint

QueryEndAtConstraint

QueryFieldFilterConstraint

QueryStartAtConstraint

addDoc

aggregateQuerySnapshotEqual

and

deleteDoc

disableNetwork

enableIndexedDbPersistence

enableMultiTabIndexedDbPersistence

enableNetwork

endAt

endBefore

executeWrite

getAggregateFromServer

getCountFromServer

getDoc

getDocFromCache

getDocFromServer

getDocs

getDocsFromCache

getDocsFromServer

loadBundle

memoryEagerGarbageCollector

memoryLocalCache

memoryLruGarbageCollector

namedQuery

onSnapshot

onSnapshotsInSync

or

persistentLocalCache

persistentMultipleTabManager

persistentSingleTabManager

query

queryEqual

runTransaction

setDoc

setIndexConfiguration

snapshotEqual

startAfter

startAt

updateDoc

waitForPendingWrites

where

writeBatch

Test Logs

milaGGL commented Aug 1, 2023 •

edited by ehsannas

Loading

changeset-bot bot commented Aug 1, 2023 •

edited

Loading

google-oss-bot commented Aug 1, 2023 •

edited

Loading

Size Report ¹

`@firebase/firestore`

`@firebase/firestore-lite`

`bundle`

`firebase`

google-oss-bot commented Aug 1, 2023 •

edited

Loading

Size Analysis Report ¹

`@firebase/firestore`

`QueryCompositeFilterConstraint`

`QueryEndAtConstraint`

`QueryFieldFilterConstraint`

`QueryStartAtConstraint`

`addDoc`

`aggregateQuerySnapshotEqual`

`and`

`deleteDoc`

`disableNetwork`

`enableIndexedDbPersistence`

`enableMultiTabIndexedDbPersistence`

`enableNetwork`

`endAt`

`endBefore`

`executeWrite`

`getAggregateFromServer`

`getCountFromServer`

`getDoc`

`getDocFromCache`

`getDocFromServer`

`getDocs`

`getDocsFromCache`

`getDocsFromServer`

`loadBundle`

`memoryEagerGarbageCollector`

`memoryLocalCache`

`memoryLruGarbageCollector`

`namedQuery`

`onSnapshot`

`onSnapshotsInSync`

`or`

`persistentLocalCache`

`persistentMultipleTabManager`

`persistentSingleTabManager`

`query`

`queryEqual`

`runTransaction`

`setDoc`

`setIndexConfiguration`

`snapshotEqual`

`startAfter`

`startAt`

`updateDoc`

`waitForPendingWrites`

`where`

`writeBatch`