Skip to content

Commit b388022

Browse files
JiriHoffmannmikehardy
JiriHoffmann
authored and
mikehardy
committed
fix(firestore): remove inequality filter with orderBy limitation
1 parent e51ebfb commit b388022

File tree

2 files changed

+176
-38
lines changed

2 files changed

+176
-38
lines changed

‎packages/firestore/e2e/Query/query.e2e.js‎

Lines changed: 176 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -66,20 +66,88 @@ describe('FirestoreQuery/FirestoreQueryModifiers', function () {
6666
}
6767
});
6868

69-
it('throws if where inequality operator is invoked, and the where fieldPath does not match initial orderBy parameter', async function () {
70-
try {
71-
firebase
72-
.firestore()
73-
.collection(COLLECTION)
74-
.where('foo', '>', 'bar')
75-
.orderBy('bar')
76-
.orderBy('foo')
77-
.limit(1)
78-
.endAt(2);
79-
return Promise.reject(new Error('Did not throw an Error.'));
80-
} catch (error) {
81-
error.message.should.containEql('Invalid query');
82-
}
69+
it('should allow inequality where fieldPath that does not match initial orderBy parameter', async function () {
70+
const colRef = firebase
71+
.firestore()
72+
// Firestore caches aggressively, even if you wipe the emulator, local documents are cached
73+
// between runs, so use random collections to make sure `tests:*:test-reuse` works while iterating
74+
.collection(`${COLLECTION}/${Utils.randString(12, '#aA')}/order-asc`);
75+
76+
await colRef.add({ value: 1, foo: 'baz' });
77+
await colRef.add({ value: 2, foo: 'bar' });
78+
await colRef.add({ value: 3, foo: 'baz' });
79+
await colRef.add({ value: 4, foo: 'bar' });
80+
await colRef.add({ value: 5, foo: 'baz' });
81+
82+
const snapshot = await colRef.where('foo', '!=', 'bar').orderBy('value', 'desc').get();
83+
const expected = [5, 3, 1];
84+
85+
snapshot.forEach((docSnap, i) => {
86+
docSnap.data().value.should.eql(expected[i]);
87+
});
88+
});
89+
90+
it('should allow inequality where fieldPath that does not match initial orderBy parameter - multiple where filters', async function () {
91+
const colRef = firebase
92+
.firestore()
93+
// Firestore caches aggressively, even if you wipe the emulator, local documents are cached
94+
// between runs, so use random collections to make sure `tests:*:test-reuse` works while iterating
95+
.collection(`${COLLECTION}/${Utils.randString(12, '#aA')}/order-asc`);
96+
97+
await colRef.add({ value: 1, foo: 'baz' });
98+
await colRef.add({ value: 2, foo: 'bar' });
99+
await colRef.add({ value: 3, foo: 'baz' });
100+
await colRef.add({ value: 4, foo: 'bar' });
101+
await colRef.add({ value: 5, foo: 'baz' });
102+
103+
let snapshot = await colRef
104+
.where('foo', '!=', 'bar')
105+
.where('value', '>', 2)
106+
.orderBy('value', 'desc')
107+
.get();
108+
let expected = [5, 3];
109+
110+
snapshot.forEach((docSnap, i) => {
111+
docSnap.data().value.should.eql(expected[i]);
112+
});
113+
114+
// flipped where order
115+
snapshot = await colRef
116+
.where('value', '>', 2)
117+
.where('foo', '!=', 'bar')
118+
.orderBy('value', 'desc')
119+
.get();
120+
const expected2 = [5, 3];
121+
122+
snapshot.forEach((docSnap, i) => {
123+
docSnap.data().value.should.eql(expected2[i]);
124+
});
125+
});
126+
127+
it('should allow multiple inequality where fieldPath with orderBy filters', async function () {
128+
const colRef = firebase
129+
.firestore()
130+
// Firestore caches aggressively, even if you wipe the emulator, local documents are cached
131+
// between runs, so use random collections to make sure `tests:*:test-reuse` works while iterating
132+
.collection(`${COLLECTION}/${Utils.randString(12, '#aA')}/order-asc`);
133+
134+
await colRef.add({ value: 1, value2: 4 });
135+
await colRef.add({ value: 2, value2: 3 });
136+
await colRef.add({ value: 3, value2: 2 });
137+
await colRef.add({ value: 3, value2: 1 });
138+
139+
const snapshot = await colRef
140+
.where('value', '>', 1)
141+
.where('value2', '<', 4)
142+
.orderBy('value', 'desc')
143+
.orderBy('value2', 'desc')
144+
.get();
145+
146+
const expected = [2, 1, 3];
147+
148+
snapshot.forEach((docSnap, i) => {
149+
docSnap.data().value2.should.eql(expected[i]);
150+
});
83151
});
84152
});
85153

@@ -126,21 +194,100 @@ describe('FirestoreQuery/FirestoreQueryModifiers', function () {
126194
}
127195
});
128196

129-
it('throws if where inequality operator is invoked, and the where fieldPath does not match initial orderBy parameter', async function () {
130-
const { getFirestore, collection, query, where, orderBy, limit, endAt } = firestoreModular;
131-
try {
132-
query(
133-
collection(getFirestore(), COLLECTION),
134-
where('foo', '>', 'bar'),
135-
orderBy('bar'),
136-
orderBy('foo'),
137-
limit(1),
138-
endAt(2),
139-
);
140-
return Promise.reject(new Error('Did not throw an Error.'));
141-
} catch (error) {
142-
error.message.should.containEql('Invalid query');
143-
}
197+
it('should allow inequality where fieldPath that does not match initial orderBy parameter', async function () {
198+
const { getFirestore, collection, addDoc, query, where, orderBy, getDocs } = firestoreModular;
199+
// Firestore caches aggressively, even if you wipe the emulator, local documents are cached
200+
// between runs, so use random collections to make sure `tests:*:test-reuse` works while iterating
201+
const colRef = collection(
202+
getFirestore(),
203+
`${COLLECTION}/${Utils.randString(12, '#aA')}/order-asc`,
204+
);
205+
206+
await addDoc(colRef, { value: 1, foo: 'baz' });
207+
await addDoc(colRef, { value: 2, foo: 'bar' });
208+
await addDoc(colRef, { value: 3, foo: 'baz' });
209+
await addDoc(colRef, { value: 4, foo: 'bar' });
210+
await addDoc(colRef, { value: 5, foo: 'baz' });
211+
212+
const q = query(colRef, where('foo', '!=', 'bar'), orderBy('value', 'desc'));
213+
const snapshot = await getDocs(q);
214+
const expected = [5, 3, 1];
215+
216+
snapshot.forEach((docSnap, i) => {
217+
docSnap.data().value.should.eql(expected[i]);
218+
});
219+
});
220+
221+
it('should allow inequality where fieldPath that does not match initial orderBy parameter - multiple where filters', async function () {
222+
const { getFirestore, collection, addDoc, query, where, orderBy, getDocs } = firestoreModular;
223+
// Firestore caches aggressively, even if you wipe the emulator, local documents are cached
224+
// between runs, so use random collections to make sure `tests:*:test-reuse` works while iterating
225+
const colRef = collection(
226+
getFirestore(),
227+
`${COLLECTION}/${Utils.randString(12, '#aA')}/order-asc`,
228+
);
229+
230+
await addDoc(colRef, { value: 1, foo: 'baz' });
231+
await addDoc(colRef, { value: 2, foo: 'bar' });
232+
await addDoc(colRef, { value: 3, foo: 'baz' });
233+
await addDoc(colRef, { value: 4, foo: 'bar' });
234+
await addDoc(colRef, { value: 5, foo: 'baz' });
235+
236+
const q = query(
237+
colRef,
238+
where('foo', '!=', 'bar'),
239+
where('value', '>', 2),
240+
orderBy('value', 'desc'),
241+
);
242+
const snapshot = await getDocs(q);
243+
const expected = [5, 3];
244+
245+
snapshot.forEach((docSnap, i) => {
246+
docSnap.data().value.should.eql(expected[i]);
247+
});
248+
249+
// flipped where order
250+
const q2 = query(
251+
colRef,
252+
where('value', '>', 2),
253+
where('foo', '!=', 'bar'),
254+
orderBy('value', 'desc'),
255+
);
256+
const snapshot2 = await getDocs(q2);
257+
const expected2 = [5, 3];
258+
259+
snapshot2.forEach((docSnap, i) => {
260+
docSnap.data().value.should.eql(expected2[i]);
261+
});
262+
});
263+
264+
it('should allow multiple inequality where fieldPath with orderBy filters', async function () {
265+
const { getFirestore, collection, addDoc, query, where, orderBy, getDocs } = firestoreModular;
266+
// Firestore caches aggressively, even if you wipe the emulator, local documents are cached
267+
// between runs, so use random collections to make sure `tests:*:test-reuse` works while iterating
268+
const colRef = collection(
269+
getFirestore(),
270+
`${COLLECTION}/${Utils.randString(12, '#aA')}/order-asc`,
271+
);
272+
273+
await addDoc(colRef, { value: 1, value2: 4 });
274+
await addDoc(colRef, { value: 2, value2: 3 });
275+
await addDoc(colRef, { value: 3, value2: 2 });
276+
await addDoc(colRef, { value: 3, value2: 1 });
277+
278+
const q = query(
279+
colRef,
280+
where('value', '>', 1),
281+
where('value2', '<', 4),
282+
orderBy('value', 'desc'),
283+
orderBy('value2', 'desc'),
284+
);
285+
const snapshot = await getDocs(q);
286+
const expected = [2, 1, 3];
287+
288+
snapshot.forEach((docSnap, i) => {
289+
docSnap.data().value2.should.eql(expected[i]);
290+
});
144291
});
145292
});
146293
});

‎packages/firestore/lib/FirestoreQueryModifiers.js‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -398,15 +398,6 @@ export default class FirestoreQueryModifiers {
398398
"Invalid query. Query.where() fieldPath parameter: 'FirestoreFieldPath' cannot be used in conjunction with a different Query.orderBy() parameter",
399399
);
400400
}
401-
402-
if (INEQUALITY[filter.operator]) {
403-
// Initial orderBy() parameter has to match every where() fieldPath parameter when inequality operator is invoked
404-
if (filterFieldPath !== this._orders[0].fieldPath._toPath()) {
405-
throw new Error(
406-
`Invalid query. Initial Query.orderBy() parameter: ${orderFieldPath} has to be the same as the Query.where() fieldPath parameter(s): ${filterFieldPath} when an inequality operator is invoked `,
407-
);
408-
}
409-
}
410401
}
411402
}
412403
}

0 commit comments

Comments
 (0)