Skip to content

Commit 0b003ff

Browse files
schmidt-sebastianschmidt-sebastian
committed
Limit Timestamp normalization
The goal of this PR is to support fast comparisons for Timestamp types when the Timestamps are represented as ISO dates
1 parent 4eab635 commit 0b003ff

File tree

4 files changed

+121
-16
lines changed

4 files changed

+121
-16
lines changed

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

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,9 @@ const ISO_TIMESTAMP_REG_EXP = new RegExp(
3333
);
3434

3535
// Denotes the possible representations for timestamps in the Value type.
36-
type ProtoTimestampValue = string | { seconds?: string; nanos?: number };
36+
type ProtoTimestampValue =
37+
| string
38+
| { seconds?: string | number; nanos?: number };
3739

3840
/** Extracts the backend's type order for the provided value. */
3941
export function typeOrder(value: api.Value): TypeOrder {
@@ -105,6 +107,15 @@ export function equals(left: api.Value, right: api.Value): boolean {
105107
}
106108

107109
function timestampEquals(left: api.Value, right: api.Value): boolean {
110+
if (
111+
typeof left.timestampValue === 'string' &&
112+
typeof right.timestampValue === 'string' &&
113+
left.timestampValue.length === right.timestampValue.length
114+
) {
115+
// Use string equality for ISO 8601 timestamps
116+
return left.timestampValue === right.timestampValue;
117+
}
118+
108119
const leftTimestamp = normalizeTimestamp(left.timestampValue!);
109120
const rightTimestamp = normalizeTimestamp(right.timestampValue!);
110121
return (
@@ -228,6 +239,14 @@ function compareTimestamps(
228239
left: ProtoTimestampValue,
229240
right: ProtoTimestampValue
230241
): number {
242+
if (typeof left === 'string' && typeof right === 'string') {
243+
// Use string ordering for ISO 8601 timestamps, but strip the timezone
244+
// suffix to ensure proper ordering for timestamps of different precision.
245+
// The only supported timezone is UTC (i.e. 'Z') based on
246+
// ISO_TIMESTAMP_REG_EXP.
247+
return primitiveComparator(left.slice(0, -1), right.slice(0, -1));
248+
}
249+
231250
const leftTimestamp = normalizeTimestamp(left);
232251
const rightTimestamp = normalizeTimestamp(right);
233252

‎packages/firestore/src/protos/firestore_proto_api.d.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -362,8 +362,8 @@ export declare namespace firestoreV1ApiClientInterfaces {
362362
nullValue?: ValueNullValue;
363363
booleanValue?: boolean;
364364
integerValue?: string | number;
365-
doubleValue?: number;
366-
timestampValue?: string | { seconds: string; nanos: number };
365+
doubleValue?: string | number;
366+
timestampValue?: string | { seconds?: string | number; nanos?: number };
367367
stringValue?: string;
368368
bytesValue?: string | Uint8Array;
369369
referenceValue?: string;

‎packages/firestore/src/remote/serializer.ts

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ function assertPresent(value: unknown, description: string): asserts value {
105105
// This is a supplement to the generated proto interfaces, which fail to account
106106
// for the fact that a timestamp may be encoded as either a string OR this.
107107
interface TimestampProto {
108-
seconds?: string;
108+
seconds?: string | number;
109109
nanos?: number;
110110
}
111111

@@ -352,6 +352,9 @@ export class JsonProtoSerializer {
352352
);
353353
}
354354

355+
// TODO(mrschmidt): Even when we remove our custom FieldValues, we still
356+
// need to re-encode field values to their expected type based on the
357+
// `useProto3Json` setting.
355358
toValue(val: fieldValue.FieldValue): api.Value {
356359
if (val instanceof fieldValue.NullValue) {
357360
return { nullValue: 'NULL_VALUE' };
@@ -412,18 +415,9 @@ export class JsonProtoSerializer {
412415
} else if ('integerValue' in obj) {
413416
return new fieldValue.IntegerValue(normalizeNumber(obj.integerValue!));
414417
} else if ('doubleValue' in obj) {
415-
if (this.options.useProto3Json) {
416-
// Proto 3 uses the string values 'NaN' and 'Infinity'.
417-
if ((obj.doubleValue as {}) === 'NaN') {
418-
return fieldValue.DoubleValue.NAN;
419-
} else if ((obj.doubleValue as {}) === 'Infinity') {
420-
return fieldValue.DoubleValue.POSITIVE_INFINITY;
421-
} else if ((obj.doubleValue as {}) === '-Infinity') {
422-
return fieldValue.DoubleValue.NEGATIVE_INFINITY;
423-
}
424-
}
425-
426-
return new fieldValue.DoubleValue(obj.doubleValue!);
418+
// Note: Proto 3 uses the string values 'NaN' and 'Infinity'.
419+
const parsedNumber = Number(obj.doubleValue!);
420+
return new fieldValue.DoubleValue(parsedNumber);
427421
} else if ('stringValue' in obj) {
428422
return new fieldValue.StringValue(obj.stringValue!);
429423
} else if ('mapValue' in obj) {

‎packages/firestore/test/unit/model/field_value.test.ts

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,44 @@ describe('FieldValue', () => {
426426
expectEqualitySets(values, (v1, v2) => v1.isEqual(v2));
427427
});
428428

429+
it('normalizes values for equality', () => {
430+
// Each subarray compares equal to each other and false to every other value
431+
const values: FieldValue[][] = [
432+
[
433+
new PrimitiveValue({ integerValue: '1' }),
434+
new PrimitiveValue({ integerValue: 1 })
435+
],
436+
[
437+
new PrimitiveValue({ doubleValue: '1.0' }),
438+
new PrimitiveValue({ doubleValue: 1.0 })
439+
],
440+
[
441+
new PrimitiveValue({ timestampValue: '2007-04-05T14:30:01Z' }),
442+
new PrimitiveValue({ timestampValue: '2007-04-05T14:30:01.000Z' }),
443+
new PrimitiveValue({ timestampValue: '2007-04-05T14:30:01.000000Z' }),
444+
new PrimitiveValue({
445+
timestampValue: '2007-04-05T14:30:01.000000000Z'
446+
}),
447+
new PrimitiveValue({ timestampValue: { seconds: 1175783401 } }),
448+
new PrimitiveValue({ timestampValue: { seconds: '1175783401' } }),
449+
new PrimitiveValue({
450+
timestampValue: { seconds: 1175783401, nanos: 0 }
451+
})
452+
],
453+
[
454+
new PrimitiveValue({ timestampValue: '2007-04-05T14:30:01.100Z' }),
455+
new PrimitiveValue({
456+
timestampValue: { seconds: 1175783401, nanos: 100000000 }
457+
})
458+
],
459+
[
460+
new PrimitiveValue({ bytesValue: new Uint8Array([0, 1, 2]) }),
461+
new PrimitiveValue({ bytesValue: 'AAEC' })
462+
]
463+
];
464+
expectEqualitySets(values, (v1, v2) => v1.isEqual(v2));
465+
});
466+
429467
it('orders types correctly', () => {
430468
const groups = [
431469
// null first
@@ -537,6 +575,60 @@ describe('FieldValue', () => {
537575
);
538576
});
539577

578+
it('normalizes values for comparison', () => {
579+
const groups = [
580+
[
581+
new PrimitiveValue({ integerValue: '1' }),
582+
new PrimitiveValue({ integerValue: 1 })
583+
],
584+
[
585+
new PrimitiveValue({ doubleValue: '2' }),
586+
new PrimitiveValue({ doubleValue: 2 })
587+
],
588+
[
589+
new PrimitiveValue({ timestampValue: '2007-04-05T14:30:01Z' }),
590+
new PrimitiveValue({ timestampValue: { seconds: 1175783401 } })
591+
],
592+
[
593+
new PrimitiveValue({ timestampValue: '2007-04-05T14:30:01.999Z' }),
594+
new PrimitiveValue({
595+
timestampValue: { seconds: 1175783401, nanos: 999000000 }
596+
})
597+
],
598+
[
599+
new PrimitiveValue({ timestampValue: '2007-04-05T14:30:02Z' }),
600+
new PrimitiveValue({ timestampValue: { seconds: 1175783402 } })
601+
],
602+
[
603+
new PrimitiveValue({ timestampValue: '2007-04-05T14:30:02.100Z' }),
604+
new PrimitiveValue({
605+
timestampValue: { seconds: 1175783402, nanos: 100000000 }
606+
})
607+
],
608+
[
609+
new PrimitiveValue({ timestampValue: '2007-04-05T14:30:02.100001Z' }),
610+
new PrimitiveValue({
611+
timestampValue: { seconds: 1175783402, nanos: 100001000 }
612+
})
613+
],
614+
[
615+
new PrimitiveValue({ bytesValue: new Uint8Array([0, 1, 2]) }),
616+
new PrimitiveValue({ bytesValue: 'AAEC' })
617+
],
618+
[
619+
new PrimitiveValue({ bytesValue: new Uint8Array([0, 1, 3]) }),
620+
new PrimitiveValue({ bytesValue: 'AAED' })
621+
]
622+
];
623+
624+
expectCorrectComparisonGroups(
625+
groups,
626+
(left: FieldValue, right: FieldValue) => {
627+
return left.compareTo(right);
628+
}
629+
);
630+
});
631+
540632
// TODO(mrschmidt): Fix size accounting
541633
// eslint-disable-next-line no-restricted-properties
542634
it.skip('estimates size correctly for fixed sized values', () => {

0 commit comments

Comments
 (0)