Skip to content

Commit

Permalink
use cursor to force the sdk add implicit orderbys
Browse files Browse the repository at this point in the history
  • Loading branch information
@milaGGL
milaGGL committed Aug 15, 2023
1 parent 2b36059 commit 66bd5c5
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 35 deletions.
9 changes: 5 additions & 4 deletions dev/src/reference.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1805,22 +1805,22 @@ export class Query<T = firestore.DocumentData> implements firestore.Query<T> {
}

/**
* Returns the sorted array of unique inequality filter fields used in this query.
* Returns the sorted array of inequality filter fields used in this query.
*
* @return An array of inequality filter fields sorted lexicographically by FieldPath.
*/
private getInequalityFilterFields(): FieldPath[] {
let FieldPathSet = new Set<FieldPath>();
const inequalityFields: FieldPath[] = [];

for (const filter of this._queryOptions.filters) {
for (const subFilter of filter.getFlattenedFilters()) {
if (subFilter.isInequalityFilter()) {
FieldPathSet = FieldPathSet.add(subFilter.field);
inequalityFields.push(subFilter.field);
}
}
}

return [...FieldPathSet].sort((a, b) => a.compareTo(b));
return inequalityFields.sort((a, b) => a.compareTo(b));
}

/**
Expand Down Expand Up @@ -1872,6 +1872,7 @@ export class Query<T = firestore.DocumentData> implements firestore.Query<T> {
!field.isEqual(FieldPath.documentId())
) {
fieldOrders.push(new FieldOrder(field, lastDirection));
fieldsNormalized.add(field.toString());
}
}

Expand Down
59 changes: 28 additions & 31 deletions dev/system-test/firestore.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2666,6 +2666,8 @@ describe('Query class', () => {
expectDocs(results, 'doc2', 'doc4');
});

// Use cursor in following test cases to add implicit order by fields in the sdk.

it('can use with nested field', async () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const testData = (n?: number): any => {
Expand All @@ -2688,32 +2690,27 @@ describe('Query class', () => {
doc4: testData(300),
});

let docSnap = await collection.doc('doc4').get();
let results = await collection
.where('metadata.createdAt', '<=', 500)
.where('metadata.createdAt', '>', 100)
.where('name', '!=', 'room 200')
.orderBy('name')
.startAt(docSnap)
.get();
// ordered by: name asc, metadata.createdAt asc, __name__ asc
expectDocs(results, 'doc4', 'doc1');

docSnap = await collection.doc('doc2').get();
results = await collection
.where('field', '>=', 'field 100')
.where(new FieldPath('field.dot'), '!=', 300)
.where('field\\slash', '<', 400)
.orderBy('name', 'desc')
.startAt(docSnap)
.get();
// ordered by: name desc, field desc, field.dot desc, field\\slash desc, __name__ desc
expectDocs(results, 'doc2', 'doc3');

// Cursor will add implicit order by fields in the same order as server does.
const docSnap = await collection.doc('doc2').get();
results = await collection
.where('field', '>=', 'field 100')
.where(new FieldPath('field.dot'), '!=', 300)
.where('field\\slash', '<', 400)
.orderBy('name', 'desc')
.startAfter(docSnap)
.get();
expectDocs(results, 'doc3');
});

it('can use with nested composite filters', async () => {
Expand All @@ -2725,7 +2722,7 @@ describe('Query class', () => {
doc5: {key: 'b', sort: 2, v: 1},
doc6: {key: 'b', sort: 0, v: 0},
});

let docSnap = await collection.doc('doc1').get();
let results = await collection
.where(
Filter.or(
Expand All @@ -2739,10 +2736,12 @@ describe('Query class', () => {
)
)
)
.startAt(docSnap)
.get();
// Implicitly ordered by: 'key' asc, 'sort' asc, 'v' asc, __name__ asc
expectDocs(results, 'doc1', 'doc6', 'doc5', 'doc4');

docSnap = await collection.doc('doc5').get();
results = await collection
.where(
Filter.or(
Expand All @@ -2758,10 +2757,12 @@ describe('Query class', () => {
)
.orderBy('sort', 'desc')
.orderBy('key')
.startAt(docSnap)
.get();
// Ordered by: 'sort' desc, 'key' asc, 'v' asc, __name__ asc
expectDocs(results, 'doc5', 'doc4', 'doc1', 'doc6');

docSnap = await collection.doc('doc1').get();
results = await collection
.where(
Filter.and(
Expand All @@ -2787,6 +2788,7 @@ describe('Query class', () => {
)
)
)
.startAt(docSnap)
.get();
// Implicitly ordered by: 'key' asc, 'sort' asc, 'v' asc, __name__ asc
expectDocs(results, 'doc1', 'doc2');
Expand All @@ -2802,10 +2804,13 @@ describe('Query class', () => {
doc6: {key: 'b', sort: 0, v: 0},
});

const docSnap = await collection.doc('doc2').get();

let results = await collection
.where('key', '!=', 'a')
.where('sort', '>', 1)
.where('v', 'in', [1, 2, 3, 4])
.startAt(docSnap)
.get();
// Implicitly ordered by: 'key' asc, 'sort' asc, __name__ asc
expectDocs(results, 'doc2', 'doc4', 'doc5', 'doc3');
Expand All @@ -2814,6 +2819,7 @@ describe('Query class', () => {
.where('sort', '>', 1)
.where('key', '!=', 'a')
.where('v', 'in', [1, 2, 3, 4])
.startAt(docSnap)
.get();
// Changing filters order will not effect implicit order.
// Implicitly ordered by: 'key' asc, 'sort' asc, __name__ asc
Expand All @@ -2830,10 +2836,12 @@ describe('Query class', () => {
doc6: {key: 'c', sort: 0, v: 2},
});

let docSnap = await collection.doc('doc2').get();
let results = await collection
.where('key', '>', 'a')
.where('sort', '>=', 1)
.orderBy('v')
.startAt(docSnap)
.get();
// Ordered by: 'v' asc, 'key' asc, 'sort' asc, __name__ asc
expectDocs(results, 'doc2', 'doc4', 'doc3', 'doc5');
Expand All @@ -2843,14 +2851,17 @@ describe('Query class', () => {
.where('sort', '>=', 1)
.orderBy('v')
.orderBy('sort')
.startAt(docSnap)
.get();
// Ordered by: 'v asc, 'sort' asc, 'key' asc, __name__ asc
expectDocs(results, 'doc2', 'doc5', 'doc4', 'doc3');

docSnap = await collection.doc('doc5').get();
results = await collection
.where('key', '>', 'a')
.where('sort', '>=', 1)
.orderBy('v', 'desc')
.startAt(docSnap)
.get();
// Implicit order by matches the direction of last explicit order by.
// Ordered by: 'v' desc, 'key' desc, 'sort' desc, __name__ desc
Expand All @@ -2861,6 +2872,7 @@ describe('Query class', () => {
.where('sort', '>=', 1)
.orderBy('v', 'desc')
.orderBy('sort')
.startAt(docSnap)
.get();
// Ordered by: 'v desc, 'sort' asc, 'key' asc, __name__ asc
expectDocs(results, 'doc5', 'doc4', 'doc3', 'doc2');
Expand Down Expand Up @@ -2893,11 +2905,13 @@ describe('Query class', () => {
doc4: {key: 'b', sort: 2},
doc5: {key: 'bb', sort: 1},
});
let docSnap = await collection.doc('doc2').get();

Check failure on line 2908 in dev/system-test/firestore.ts

View workflow job for this annotation

GitHub Actions / lint 

'docSnap' is never reassigned. Use 'const' instead

let results = await collection
.where('sort', '>=', 1)
.where('key', '!=', 'a')
.where(FieldPath.documentId(), '<', 'doc5')
.startAt(docSnap)
.get();
// Document Key in inequality field will implicitly ordered to the last.
// Implicitly ordered by: 'key' asc, 'sort' asc, __name__ asc
Expand All @@ -2907,6 +2921,7 @@ describe('Query class', () => {
.where(FieldPath.documentId(), '<', 'doc5')
.where('sort', '>=', 1)
.where('key', '!=', 'a')
.startAt(docSnap)
.get();
// Changing filters order will not effect implicit order.
// Implicitly ordered by: 'key' asc, 'sort' asc, __name__ asc
Expand All @@ -2917,29 +2932,11 @@ describe('Query class', () => {
.where('sort', '>=', 1)
.where('key', '!=', 'a')
.orderBy('sort', 'desc')
.startAt(docSnap)
.get();
// Ordered by: 'sort' desc,'key' desc, __name__ desc
expectDocs(results, 'doc2', 'doc3', 'doc4');
});

it('Cursors with DocumentSnapshot implicitly adds inequality fields', async () => {
const collection = await testCollectionWithDocs({
doc1: {key: 'a', sort: 0, v: 5},
doc2: {key: 'aa', sort: 4, v: 4},
doc3: {key: 'b', sort: 3, v: 3},
doc4: {key: 'b', sort: 2, v: 2},
doc5: {key: 'b', sort: 0, v: 1},
});

const docSnap = await collection.doc('doc4').get();
const results = await collection
.where('key', '!=', 'a')
.where('sort', '>', 1)
.startAt(docSnap)
.get();
// Implicitly ordered by: 'key' asc, 'sort' asc, __name__ asc
expectDocs(results, 'doc4', 'doc3');
});
});
});

Expand Down

0 comments on commit 66bd5c5

Please sign in to comment.