Skip to content

Commit 5e5f31f

Browse files
schmidt-sebastianschmidt-sebastian
committed
Review
1 parent 7ccadd8 commit 5e5f31f

File tree

7 files changed

+144
-70
lines changed

7 files changed

+144
-70
lines changed

‎packages/firestore/src/core/target.ts

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ export function targetGetNotInValues(
259259
target: Target,
260260
fieldIndex: FieldIndex
261261
): ProtoValue[] | null {
262-
const values: ProtoValue[] = [];
262+
const values = new Map</* fieldPath = */ string, ProtoValue>();
263263

264264
for (const segment of fieldIndexGetDirectionalSegments(fieldIndex)) {
265265
for (const fieldFilter of targetGetFieldFiltersForPath(
@@ -272,14 +272,14 @@ export function targetGetNotInValues(
272272
// Encode equality prefix, which is encoded in the index value before
273273
// the inequality (e.g. `a == 'a' && b != 'b'` is encoded to
274274
// `value != 'ab'`).
275-
values.push(fieldFilter.value);
275+
values.set(segment.fieldPath.canonicalString(), fieldFilter.value);
276276
break;
277277
case Operator.NOT_IN:
278278
case Operator.NOT_EQUAL:
279279
// NotIn/NotEqual is always a suffix. There cannot be any remaining
280280
// segments and hence we can return early here.
281-
values.push(fieldFilter.value);
282-
return values;
281+
values.set(segment.fieldPath.canonicalString(), fieldFilter.value);
282+
return Array.from(values.values());
283283
default:
284284
// Remaining filters cannot be used as notIn bounds.
285285
}
@@ -330,12 +330,8 @@ export function targetGetLowerBound(
330330
filterInclusive = false;
331331
break;
332332
case Operator.NOT_EQUAL:
333-
filterValue = MIN_VALUE;
334-
break;
335333
case Operator.NOT_IN:
336-
filterValue = {
337-
arrayValue: { values: [MIN_VALUE] }
338-
};
334+
filterValue = MIN_VALUE;
339335
break;
340336
default:
341337
// Remaining filters cannot be used as lower bounds.
@@ -414,12 +410,8 @@ export function targetGetUpperBound(
414410
filterInclusive = false;
415411
break;
416412
case Operator.NOT_EQUAL:
417-
filterValue = MAX_VALUE;
418-
break;
419413
case Operator.NOT_IN:
420-
filterValue = {
421-
arrayValue: { values: [MAX_VALUE] }
422-
};
414+
filterValue = MAX_VALUE;
423415
break;
424416
default:
425417
// Remaining filters cannot be used as upper bounds.

‎packages/firestore/src/local/indexeddb_index_manager.ts

Lines changed: 66 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -554,12 +554,12 @@ export class IndexedDbIndexManager implements IndexManager {
554554
}
555555

556556
private isInFilter(target: Target, fieldPath: FieldPath): boolean {
557-
for (const filter of target.filters) {
558-
if (filter instanceof FieldFilter && filter.field.isEqual(fieldPath)) {
559-
return filter.op === Operator.IN || filter.op === Operator.NOT_IN;
560-
}
561-
}
562-
return false;
557+
return !!target.filters.find(
558+
f =>
559+
f instanceof FieldFilter &&
560+
f.field.isEqual(fieldPath) &&
561+
(f.op === Operator.IN || f.op === Operator.NOT_IN)
562+
);
563563
}
564564

565565
getFieldIndexes(
@@ -853,52 +853,75 @@ export class IndexedDbIndexManager implements IndexManager {
853853

854854
const notInRanges: IDBKeyRange[] = [];
855855
for (const indexRange of indexRanges) {
856-
// Use the existing bounds and interleave the notIn values. This means
857-
// that we would split an existing range into multiple ranges that exclude
858-
// the values from any notIn filter.
859-
860-
// The first index range starts with the lower bound and ends at the
861-
// first notIn value (exclusive).
862-
notInRanges.push(
863-
IDBKeyRange.bound(
864-
indexRange.lower,
865-
this.generateNotInBound(indexRange.lower, notInValues[0]),
866-
indexRange.lowerOpen,
867-
/* upperOpen= */ true
868-
)
856+
// Remove notIn values that are not applicable to this index range
857+
const filteredRanges = notInValues.filter(
858+
v =>
859+
compareByteArrays(v, new Uint8Array(indexRange.lower[3])) >= 0 &&
860+
compareByteArrays(v, new Uint8Array(indexRange.upper[3])) <= 0
869861
);
870862

871-
for (let i = 1; i < notInValues.length - 1; ++i) {
872-
// Each index range that we need to scan starts at the last notIn value
873-
// and ends at the next.
874-
notInRanges.push(
875-
IDBKeyRange.bound(
876-
this.generateNotInBound(indexRange.lower, notInValues[i - 1]),
877-
this.generateNotInBound(
878-
indexRange.lower,
879-
successor(notInValues[i])
880-
),
881-
/* lowerOpen= */ false,
882-
/* upperOpen= */ true
883-
)
884-
);
863+
if (filteredRanges.length === 0) {
864+
notInRanges.push(indexRange);
865+
} else {
866+
// Use the existing bounds and interleave the notIn values. This means
867+
// that we would split an existing range into multiple ranges that exclude
868+
// the values from any notIn filter.
869+
notInRanges.push(...this.interleaveRanges(indexRange, filteredRanges));
885870
}
871+
}
872+
return notInRanges;
873+
}
874+
875+
/**
876+
* Splits up the range defined by `indexRange` and removes any values
877+
* contained in `barrier`. As an example, if the original range is [1,4] and
878+
* the barrier is [2,3], then this method would return [1,2), (2,3), (3,4].
879+
*/
880+
private interleaveRanges(
881+
indexRange: IDBKeyRange,
882+
barriers: Uint8Array[]
883+
): IDBKeyRange[] {
884+
const ranges: IDBKeyRange[] = [];
885+
886+
// The first index range starts with the lower bound and ends before the
887+
// first barrier.
888+
ranges.push(
889+
IDBKeyRange.bound(
890+
indexRange.lower,
891+
this.generateNotInBound(indexRange.lower, barriers[0]),
892+
indexRange.lowerOpen,
893+
/* upperOpen= */ true
894+
)
895+
);
886896

887-
// The last index range starts at tha last notIn value (exclusive) and
888-
// ends at the upper bound;
889-
notInRanges.push(
897+
for (let i = 1; i < barriers.length - 1; ++i) {
898+
// Each index range that we need to scan starts after the last barrier
899+
// and ends before the next.
900+
ranges.push(
890901
IDBKeyRange.bound(
891-
this.generateNotInBound(
892-
indexRange.lower,
893-
successor(notInValues[notInValues.length - 1])
894-
),
895-
indexRange.upper,
902+
this.generateNotInBound(indexRange.lower, barriers[i - 1]),
903+
this.generateNotInBound(indexRange.lower, successor(barriers[i])),
896904
/* lowerOpen= */ false,
897-
indexRange.upperOpen
905+
/* upperOpen= */ true
898906
)
899907
);
900908
}
901-
return notInRanges;
909+
910+
// The last index range starts after the last barrier and ends at the upper
911+
// bound
912+
ranges.push(
913+
IDBKeyRange.bound(
914+
this.generateNotInBound(
915+
indexRange.lower,
916+
successor(barriers[barriers.length - 1])
917+
),
918+
indexRange.upper,
919+
/* lowerOpen= */ false,
920+
indexRange.upperOpen
921+
)
922+
);
923+
924+
return ranges;
902925
}
903926

904927
/**

‎packages/firestore/src/local/simple_db.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -652,8 +652,14 @@ export class SimpleDbStore<
652652
return wrapRequest<number>(request);
653653
}
654654

655+
/** Loads all elements from the object store. */
655656
loadAll(): PersistencePromise<ValueType[]>;
657+
/** Loads all elements for the index range from the object store. */
656658
loadAll(range: IDBKeyRange): PersistencePromise<ValueType[]>;
659+
/**
660+
* Loads all elements from the object store that fall into the provided in the
661+
* index range for the given index.
662+
*/
657663
loadAll(index: string, range: IDBKeyRange): PersistencePromise<ValueType[]>;
658664
loadAll(
659665
indexOrRange?: string | IDBKeyRange,
@@ -683,6 +689,10 @@ export class SimpleDbStore<
683689
}
684690
}
685691

692+
/**
693+
* Loads the first `count` elements from the provided index range. Loads all
694+
* elements if no limit is provided.
695+
*/
686696
loadFirst(
687697
range: IDBKeyRange,
688698
count: number | null

‎packages/firestore/src/model/type_order.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
*/
2525
export const enum TypeOrder {
2626
// This order is based on the backend's ordering, but modified to support
27-
// server timestamps.
27+
// server timestamps and `MAX_VALUE`.
2828
NullValue = 0,
2929
BooleanValue = 1,
3030
NumberValue = 2,
@@ -35,5 +35,6 @@ export const enum TypeOrder {
3535
RefValue = 7,
3636
GeoPointValue = 8,
3737
ArrayValue = 9,
38-
ObjectValue = 10
38+
ObjectValue = 10,
39+
MaxValue = 9007199254740991 // Number.MAX_SAFE_INTEGER
3940
}

‎packages/firestore/src/model/values.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,11 @@ import {
4141
} from './server_timestamps';
4242
import { TypeOrder } from './type_order';
4343

44+
const MAX_VALUE_TYPE = '__max__';
4445
export const MAX_VALUE: Value = {
4546
mapValue: {
4647
fields: {
47-
'__type__': { stringValue: '__max___' }
48+
'__type__': { stringValue: MAX_VALUE_TYPE }
4849
}
4950
}
5051
};
@@ -76,6 +77,8 @@ export function typeOrder(value: Value): TypeOrder {
7677
} else if ('mapValue' in value) {
7778
if (isServerTimestamp(value)) {
7879
return TypeOrder.ServerTimestampValue;
80+
} else if (isMaxValue(value)) {
81+
return TypeOrder.ArrayValue;
7982
}
8083
return TypeOrder.ObjectValue;
8184
} else {
@@ -122,6 +125,8 @@ export function valueEquals(left: Value, right: Value): boolean {
122125
);
123126
case TypeOrder.ObjectValue:
124127
return objectEquals(left, right);
128+
case TypeOrder.MaxValue:
129+
return true;
125130
default:
126131
return fail('Unexpected value type: ' + JSON.stringify(left));
127132
}
@@ -224,6 +229,7 @@ export function valueCompare(left: Value, right: Value): number {
224229

225230
switch (leftType) {
226231
case TypeOrder.NullValue:
232+
case TypeOrder.MaxValue:
227233
return 0;
228234
case TypeOrder.BooleanValue:
229235
return primitiveComparator(left.booleanValue!, right.booleanValue!);
@@ -604,7 +610,10 @@ export function deepClone(source: Value): Value {
604610

605611
/** Returns true if the Value represents the canonical {@link #MAX_VALUE} . */
606612
export function isMaxValue(value: Value): boolean {
607-
return valueEquals(value, MAX_VALUE);
613+
return (
614+
(((value.mapValue || {}).fields || {})['__type__'] || {}).stringValue ===
615+
MAX_VALUE_TYPE
616+
);
608617
}
609618

610619
/** Returns the lowest value for the given value type (inclusive). */

‎packages/firestore/test/unit/local/index_manager.test.ts

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,27 @@ describe('IndexedDbIndexManager', async () => {
346346
await verifyResults(q, 'coll/val2');
347347
});
348348

349+
it('applies equals with not equals filter on same field', async () => {
350+
await setUpSingleValueFilter();
351+
let q = queryWithAddedFilter(
352+
queryWithAddedFilter(query('coll'), filter('count', '>', 1)),
353+
filter('count', '!=', 2)
354+
);
355+
await verifyResults(q, 'coll/val3');
356+
357+
q = queryWithAddedFilter(
358+
queryWithAddedFilter(query('coll'), filter('count', '==', 1)),
359+
filter('count', '!=', 2)
360+
);
361+
await verifyResults(q, 'coll/val1');
362+
363+
q = queryWithAddedFilter(
364+
queryWithAddedFilter(query('coll'), filter('count', '==', 1)),
365+
filter('count', '!=', 1)
366+
);
367+
await verifyResults(q);
368+
});
369+
349370
it('applies less than filter', async () => {
350371
await setUpSingleValueFilter();
351372
const q = queryWithAddedFilter(query('coll'), filter('count', '<', 2));
@@ -442,7 +463,7 @@ describe('IndexedDbIndexManager', async () => {
442463
await verifyResults(q, 'coll/val1', 'coll/val3');
443464
});
444465

445-
it('applies notIn filter', async () => {
466+
it('applies not in filter', async () => {
446467
await setUpSingleValueFilter();
447468
const q = queryWithAddedFilter(
448469
query('coll'),
@@ -451,7 +472,25 @@ describe('IndexedDbIndexManager', async () => {
451472
await verifyResults(q, 'coll/val3');
452473
});
453474

454-
it('applies arrayContains filter', async () => {
475+
it('applies not in filter with greater than filter', async () => {
476+
await setUpSingleValueFilter();
477+
const q = queryWithAddedFilter(
478+
queryWithAddedFilter(query('coll'), filter('count', '>', 1)),
479+
filter('count', 'not-in', [2])
480+
);
481+
await verifyResults(q, 'coll/val3');
482+
});
483+
484+
it('applies not in filter with out of bounds greater than filter', async () => {
485+
await setUpSingleValueFilter();
486+
const q = queryWithAddedFilter(
487+
queryWithAddedFilter(query('coll'), filter('count', '>', 2)),
488+
filter('count', 'not-in', [1])
489+
);
490+
await verifyResults(q, 'coll/val3');
491+
});
492+
493+
it('applies array contains filter', async () => {
455494
await setUpArrayValueFilter();
456495
const q = queryWithAddedFilter(
457496
query('coll'),
@@ -460,7 +499,7 @@ describe('IndexedDbIndexManager', async () => {
460499
await verifyResults(q, 'coll/arr1');
461500
});
462501

463-
it('applies arrayContainsAny filter', async () => {
502+
it('applies array contains any filter', async () => {
464503
await setUpArrayValueFilter();
465504
const q = queryWithAddedFilter(
466505
query('coll'),

0 commit comments

Comments
 (0)