Skip to content

Commit d40afa3

Browse files
russellwheatleymikehardy
russellwheatley
authored and
mikehardy
committed
fix(firestore): remove exception throw on inequality queries on different fields
1 parent a41e556 commit d40afa3

File tree

6 files changed

+276
-360
lines changed

6 files changed

+276
-360
lines changed

‎.github/workflows/scripts/firestore.indexes.json‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,14 @@
1111
{
1212
"fieldPath": "b",
1313
"order": "ASCENDING"
14+
},
15+
{
16+
"fieldPath": "foo.bar",
17+
"order": "ASCENDING"
18+
},
19+
{
20+
"fieldPath": "baz",
21+
"order": "ASCENDING"
1422
}
1523
]
1624
}

‎packages/firestore/e2e/Query/where.and.filter.e2e.js‎

Lines changed: 74 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -117,22 +117,29 @@ describe(' firestore().collection().where(AND Filters)', function () {
117117
.where(Filter.and(Filter('foo.bar', '==', null), Filter('foo.bar', '!=', null)));
118118
});
119119

120-
it('throws if multiple inequalities on different paths is provided', function () {
121-
try {
122-
firebase
123-
.firestore()
124-
.collection(COLLECTION)
125-
.where(Filter.and(Filter('foo.bar', '>', 123), Filter('bar', '>', 123)));
120+
it('allows multiple inequalities (excluding `!=`) on different paths provided', async function () {
121+
const colRef = firebase
122+
.firestore()
123+
.collection(`${COLLECTION}/filter/different-path-inequality-filter`);
124+
const expected = { foo: { bar: 300 }, bar: 200 };
125+
await Promise.all([
126+
colRef.add({ foo: { bar: 1 }, bar: 1 }),
127+
colRef.add(expected),
128+
colRef.add(expected),
129+
]);
126130

127-
return Promise.reject(new Error('Did not throw an Error.'));
128-
} catch (error) {
129-
error.message.should.containEql('All where filters with an inequality');
130-
return Promise.resolve();
131-
}
131+
const snapshot = await colRef
132+
.where(Filter.and(Filter('foo.bar', '>', 123), Filter('bar', '>', 123)))
133+
.get();
134+
135+
snapshot.size.should.eql(2);
136+
snapshot.forEach(s => {
137+
s.data().should.eql(jet.contextify(expected));
138+
});
132139
});
133140

134-
it('allows inequality on the same path', function () {
135-
firebase
141+
it('allows inequality on the same path', async function () {
142+
await firebase
136143
.firestore()
137144
.collection(COLLECTION)
138145
.where(
@@ -323,38 +330,24 @@ describe(' firestore().collection().where(AND Filters)', function () {
323330
}
324331
});
325332

326-
it("should throw error when combining '!=' operator with any other inequality operator on a different field", async function () {
327-
const ref = firebase.firestore().collection(COLLECTION);
328-
329-
try {
330-
ref.where(Filter.and(Filter('foo.bar', '!=', 1), Filter('differentField', '>', 2)));
331-
return Promise.reject(new Error('Did not throw an Error on >.'));
332-
} catch (error) {
333-
error.message.should.containEql('must be on the same field.');
334-
}
335-
336-
try {
337-
ref.where(Filter.and(Filter('foo.bar', '!=', 1), Filter('differentField', '<', 2)));
338-
return Promise.reject(new Error('Did not throw an Error on <.'));
339-
} catch (error) {
340-
error.message.should.containEql('must be on the same field.');
341-
}
342-
343-
try {
344-
ref.where(Filter.and(Filter('foo.bar', '!=', 1), Filter('differentField', '<=', 2)));
345-
return Promise.reject(new Error('Did not throw an Error <=.'));
346-
} catch (error) {
347-
error.message.should.containEql('must be on the same field.');
348-
}
349-
350-
try {
351-
ref.where(Filter.and(Filter('foo.bar', '!=', 1), Filter('differentField', '>=', 2)));
352-
return Promise.reject(new Error('Did not throw an Error >=.'));
353-
} catch (error) {
354-
error.message.should.containEql('must be on the same field.');
355-
}
333+
it("should allow query when combining '!=' operator with any other inequality operator on a different field", async function () {
334+
const colRef = firebase
335+
.firestore()
336+
.collection(`${COLLECTION}/filter/inequality-combine-not-equal`);
337+
const expected = { foo: { bar: 300 }, bar: 200 };
338+
await Promise.all([
339+
colRef.add({ foo: { bar: 1 }, bar: 1 }),
340+
colRef.add(expected),
341+
colRef.add(expected),
342+
]);
356343

357-
return Promise.resolve();
344+
const snapshot = await colRef
345+
.where(Filter.and(Filter('foo.bar', '>', 123), Filter('bar', '!=', 123)))
346+
.get();
347+
snapshot.size.should.eql(2);
348+
snapshot.forEach(s => {
349+
s.data().should.eql(jet.contextify(expected));
350+
});
358351
});
359352

360353
/* Queries */
@@ -774,19 +767,26 @@ describe(' firestore().collection().where(AND Filters)', function () {
774767
);
775768
});
776769

777-
it('throws if multiple inequalities on different paths is provided', function () {
770+
it('allows multiple inequalities (excluding `!=`) on different paths provided', async function () {
778771
const { getFirestore, collection, query, and, where } = firestoreModular;
779-
try {
780-
query(
781-
collection(getFirestore(), COLLECTION),
782-
and(where('foo.bar', '>', 123), where('bar', '>', 123)),
783-
);
772+
const colRef = collection(getFirestore(), `${COLLECTION}/filter/different-path-inequality`);
784773

785-
return Promise.reject(new Error('Did not throw an Error.'));
786-
} catch (error) {
787-
error.message.should.containEql('All where filters with an inequality');
788-
return Promise.resolve();
789-
}
774+
const expected = { foo: { bar: 300 }, bar: 200 };
775+
await Promise.all([
776+
colRef.add({ foo: { bar: 1 }, bar: 1 }),
777+
colRef.add(expected),
778+
colRef.add(expected),
779+
]);
780+
781+
const snapshot = await query(
782+
colRef,
783+
and(where('foo.bar', '>', 123), where('bar', '>', 123)),
784+
).get();
785+
786+
snapshot.size.should.eql(2);
787+
snapshot.forEach(s => {
788+
s.data().should.eql(jet.contextify(expected));
789+
});
790790
});
791791

792792
it('allows inequality on the same path', function () {
@@ -979,39 +979,26 @@ describe(' firestore().collection().where(AND Filters)', function () {
979979
}
980980
});
981981

982-
it("should throw error when combining '!=' operator with any other inequality operator on a different field", async function () {
983-
const { getFirestore, collection, query, where, and } = firestoreModular;
984-
const ref = collection(getFirestore(), COLLECTION);
985-
986-
try {
987-
query(ref, and(where('foo.bar', '!=', 1), where('differentField', '>', 2)));
988-
return Promise.reject(new Error('Did not throw an Error on >.'));
989-
} catch (error) {
990-
error.message.should.containEql('must be on the same field.');
991-
}
992-
993-
try {
994-
query(ref, and(where('foo.bar', '!=', 1), where('differentField', '<', 2)));
995-
return Promise.reject(new Error('Did not throw an Error on <.'));
996-
} catch (error) {
997-
error.message.should.containEql('must be on the same field.');
998-
}
999-
1000-
try {
1001-
query(ref, and(where('foo.bar', '!=', 1), where('differentField', '<=', 2)));
1002-
return Promise.reject(new Error('Did not throw an Error <=.'));
1003-
} catch (error) {
1004-
error.message.should.containEql('must be on the same field.');
1005-
}
1006-
1007-
try {
1008-
query(ref, and(where('foo.bar', '!=', 1), where('differentField', '>=', 2)));
1009-
return Promise.reject(new Error('Did not throw an Error >=.'));
1010-
} catch (error) {
1011-
error.message.should.containEql('must be on the same field.');
1012-
}
982+
it("should allow query when combining '!=' operator with any other inequality operator on a different field", async function () {
983+
const { query, where, and } = firestoreModular;
984+
const colRef = firebase
985+
.firestore()
986+
.collection(`${COLLECTION}/filter/inequality-combine-not-equal`);
987+
const expected = { foo: { bar: 300 }, bar: 200 };
988+
await Promise.all([
989+
colRef.add({ foo: { bar: 1 }, bar: 1 }),
990+
colRef.add(expected),
991+
colRef.add(expected),
992+
]);
1013993

1014-
return Promise.resolve();
994+
const snapshot = await query(
995+
colRef,
996+
and(where('foo.bar', '>', 123), where('bar', '!=', 123)),
997+
).get();
998+
snapshot.size.should.eql(2);
999+
snapshot.forEach(s => {
1000+
s.data().should.eql(jet.contextify(expected));
1001+
});
10151002
});
10161003

10171004
/* Queries */

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

Lines changed: 70 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -96,18 +96,23 @@ describe('firestore().collection().where()', function () {
9696
firebase.firestore().collection(COLLECTION).where('foo.bar', '!=', null);
9797
});
9898

99-
it('throws if multiple inequalities on different paths is provided', function () {
100-
try {
101-
firebase
102-
.firestore()
103-
.collection(COLLECTION)
104-
.where('foo.bar', '>', 123)
105-
.where('bar', '>', 123);
106-
return Promise.reject(new Error('Did not throw an Error.'));
107-
} catch (error) {
108-
error.message.should.containEql('All where filters with an inequality');
109-
return Promise.resolve();
110-
}
99+
it('allows multiple inequalities (excluding `!=`) on different paths provided', async function () {
100+
const colRef = firebase
101+
.firestore()
102+
.collection(`${COLLECTION}/filter/different-path-inequality`);
103+
const expected = { foo: { bar: 300 }, bar: 200 };
104+
await Promise.all([
105+
colRef.add({ foo: { bar: 1 }, bar: 1 }),
106+
colRef.add(expected),
107+
colRef.add(expected),
108+
]);
109+
110+
const snapshot = await colRef.where('foo.bar', '>', 123).where('bar', '>', 123).get();
111+
112+
snapshot.size.should.eql(2);
113+
snapshot.forEach(s => {
114+
s.data().should.eql(jet.contextify(expected));
115+
});
111116
});
112117

113118
it('allows inequality on the same path', function () {
@@ -520,38 +525,22 @@ describe('firestore().collection().where()', function () {
520525
}
521526
});
522527

523-
it("should throw error when combining '!=' operator with any other inequality operator on a different field", async function () {
524-
const ref = firebase.firestore().collection(COLLECTION);
525-
526-
try {
527-
ref.where('test', '!=', 1).where('differentField', '>', 1);
528-
return Promise.reject(new Error('Did not throw an Error on >.'));
529-
} catch (error) {
530-
error.message.should.containEql('must be on the same field.');
531-
}
532-
533-
try {
534-
ref.where('test', '!=', 1).where('differentField', '<', 1);
535-
return Promise.reject(new Error('Did not throw an Error on <.'));
536-
} catch (error) {
537-
error.message.should.containEql('must be on the same field.');
538-
}
539-
540-
try {
541-
ref.where('test', '!=', 1).where('differentField', '<=', 1);
542-
return Promise.reject(new Error('Did not throw an Error <=.'));
543-
} catch (error) {
544-
error.message.should.containEql('must be on the same field.');
545-
}
546-
547-
try {
548-
ref.where('test', '!=', 1).where('differentField', '>=', 1);
549-
return Promise.reject(new Error('Did not throw an Error >=.'));
550-
} catch (error) {
551-
error.message.should.containEql('must be on the same field.');
552-
}
528+
it("should allow query when combining '!=' operator with any other inequality operator on a different field", async function () {
529+
const colRef = firebase
530+
.firestore()
531+
.collection(`${COLLECTION}/filter/inequality-combine-not-equal`);
532+
const expected = { foo: { bar: 300 }, bar: 200 };
533+
await Promise.all([
534+
colRef.add({ foo: { bar: 1 }, bar: 1 }),
535+
colRef.add(expected),
536+
colRef.add(expected),
537+
]);
553538

554-
return Promise.resolve();
539+
const snapshot = await colRef.where('foo.bar', '>', 123).where('bar', '!=', 123).get();
540+
snapshot.size.should.eql(2);
541+
snapshot.forEach(s => {
542+
s.data().should.eql(jet.contextify(expected));
543+
});
555544
});
556545

557546
it('should handle where clause after sort by', async function () {
@@ -659,19 +648,28 @@ describe('firestore().collection().where()', function () {
659648
query(collection(getFirestore(), COLLECTION), where('foo.bar', '!=', null));
660649
});
661650

662-
it('throws if multiple inequalities on different paths is provided', function () {
663-
const { getFirestore, collection, query, where } = firestoreModular;
664-
try {
665-
query(
666-
collection(getFirestore(), COLLECTION),
667-
where('foo.bar', '>', 123),
668-
where('bar', '>', 123),
669-
);
670-
return Promise.reject(new Error('Did not throw an Error.'));
671-
} catch (error) {
672-
error.message.should.containEql('All where filters with an inequality');
673-
return Promise.resolve();
674-
}
651+
it('allows multiple inequalities (excluding `!=`) on different paths provided', async function () {
652+
const { query, where } = firestoreModular;
653+
654+
const colRef = firebase
655+
.firestore()
656+
.collection(`${COLLECTION}/filter/different-path-inequality`);
657+
const expected = { foo: { bar: 300 }, bar: 200 };
658+
await Promise.all([
659+
colRef.add({ foo: { bar: 1 }, bar: 1 }),
660+
colRef.add(expected),
661+
colRef.add(expected),
662+
]);
663+
664+
const snapshot = await query(
665+
colRef,
666+
where('foo.bar', '>', 123),
667+
where('bar', '>', 123),
668+
).get();
669+
snapshot.size.should.eql(2);
670+
snapshot.forEach(s => {
671+
s.data().should.eql(jet.contextify(expected));
672+
});
675673
});
676674

677675
it('allows inequality on the same path', function () {
@@ -1123,39 +1121,25 @@ describe('firestore().collection().where()', function () {
11231121
}
11241122
});
11251123

1126-
it("should throw error when combining '!=' operator with any other inequality operator on a different field", async function () {
1124+
it("should allow query when combining '!=' operator with any other inequality operator on a different field", async function () {
11271125
const { getFirestore, collection, query, where } = firestoreModular;
1128-
const ref = collection(getFirestore(), COLLECTION);
1129-
1130-
try {
1131-
query(ref, where('test', '!=', 1), where('differentField', '>', 1));
1132-
return Promise.reject(new Error('Did not throw an Error on >.'));
1133-
} catch (error) {
1134-
error.message.should.containEql('must be on the same field.');
1135-
}
1136-
1137-
try {
1138-
query(ref, where('test', '!=', 1), where('differentField', '<', 1));
1139-
return Promise.reject(new Error('Did not throw an Error on <.'));
1140-
} catch (error) {
1141-
error.message.should.containEql('must be on the same field.');
1142-
}
1143-
1144-
try {
1145-
query(ref, where('test', '!=', 1), where('differentField', '<=', 1));
1146-
return Promise.reject(new Error('Did not throw an Error <=.'));
1147-
} catch (error) {
1148-
error.message.should.containEql('must be on the same field.');
1149-
}
1126+
const colRef = collection(
1127+
getFirestore(),
1128+
`${COLLECTION}/filter/inequality-combine-not-equal`,
1129+
);
1130+
const expected = { foo: { bar: 300 }, bar: 200 };
1131+
await Promise.all([
1132+
colRef.add({ foo: { bar: 1 }, bar: 1 }),
1133+
colRef.add(expected),
1134+
colRef.add(expected),
1135+
]);
11501136

1151-
try {
1152-
query(ref, where('test', '!=', 1), where('differentField', '>=', 1));
1153-
return Promise.reject(new Error('Did not throw an Error >=.'));
1154-
} catch (error) {
1155-
error.message.should.containEql('must be on the same field.');
1156-
}
1137+
const snapshot = await query(colRef, where('foo.bar', '>', 123), where('bar', '!=', 1)).get();
11571138

1158-
return Promise.resolve();
1139+
snapshot.size.should.eql(2);
1140+
snapshot.forEach(s => {
1141+
s.data().should.eql(jet.contextify(expected));
1142+
});
11591143
});
11601144

11611145
it('should handle where clause after sort by', async function () {

0 commit comments

Comments
 (0)