Skip to content

Commit 9d20983

Browse files
committed
fix: remove null from != and not-in filters' result
1 parent b332825 commit 9d20983

File tree

3 files changed

+89
-3
lines changed

3 files changed

+89
-3
lines changed

packages/firestore/src/core/filter.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ export class FieldFilter extends Filter {
141141
if (this.op === Operator.NOT_EQUAL) {
142142
return (
143143
other !== null &&
144+
other.nullValue === undefined &&
144145
this.matchesComparison(valueCompare(other!, this.value))
145146
);
146147
}
@@ -495,7 +496,11 @@ export class NotInFilter extends FieldFilter {
495496
return false;
496497
}
497498
const other = doc.data.field(this.field);
498-
return other !== null && !arrayValueContains(this.value.arrayValue!, other);
499+
return (
500+
other !== null &&
501+
other.nullValue === undefined &&
502+
!arrayValueContains(this.value.arrayValue!, other)
503+
);
499504
}
500505
}
501506

packages/firestore/test/integration/api/query.test.ts

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1751,6 +1751,87 @@ apiDescribe('Queries', persistence => {
17511751
);
17521752
});
17531753
});
1754+
1755+
it('sdk uses != filter same as backend', async () => {
1756+
const testDocs = {
1757+
a: { zip: Number.NaN },
1758+
b: { zip: 91102 },
1759+
c: { zip: 98101 },
1760+
d: { zip: '98101' },
1761+
e: { zip: [98101] },
1762+
f: { zip: [98101, 98102] },
1763+
g: { zip: ['98101', { zip: 98101 }] },
1764+
h: { zip: { code: 500 } },
1765+
i: { zip: null },
1766+
j: { code: 500 }
1767+
};
1768+
1769+
await withTestCollection(persistence, testDocs, async coll => {
1770+
// populate cache with all documents first to ensure getDocsFromCache() scans all docs
1771+
await getDocs(coll);
1772+
1773+
let testQuery = query(coll, where('zip', '!=', 98101));
1774+
await checkOnlineAndOfflineResultsMatch(
1775+
testQuery,
1776+
'a',
1777+
'b',
1778+
'd',
1779+
'e',
1780+
'f',
1781+
'g',
1782+
'h'
1783+
);
1784+
1785+
testQuery = query(coll, where('zip', '!=', Number.NaN));
1786+
await checkOnlineAndOfflineResultsMatch(
1787+
testQuery,
1788+
'b',
1789+
'c',
1790+
'd',
1791+
'e',
1792+
'f',
1793+
'g',
1794+
'h'
1795+
);
1796+
});
1797+
});
1798+
1799+
it('sdk uses not-in filter same as backend', async () => {
1800+
const testDocs = {
1801+
a: { zip: Number.NaN },
1802+
b: { zip: 91102 },
1803+
c: { zip: 98101 },
1804+
d: { zip: '98101' },
1805+
e: { zip: [98101] },
1806+
f: { zip: [98101, 98102] },
1807+
g: { zip: ['98101', { zip: 98101 }] },
1808+
h: { zip: { code: 500 } },
1809+
i: { zip: null },
1810+
j: { code: 500 }
1811+
};
1812+
1813+
await withTestCollection(persistence, testDocs, async coll => {
1814+
// populate cache with all documents first to ensure getDocsFromCache() scans all docs
1815+
await getDocs(coll);
1816+
1817+
let testQuery = query(
1818+
coll,
1819+
where('zip', 'not-in', [98101, 98103, [98101, 98102]])
1820+
);
1821+
await checkOnlineAndOfflineResultsMatch(
1822+
testQuery,
1823+
'a',
1824+
'b',
1825+
'd',
1826+
'e',
1827+
'g',
1828+
'h'
1829+
);
1830+
1831+
testQuery = query(coll, where('zip', 'not-in', [null]));
1832+
await checkOnlineAndOfflineResultsMatch(testQuery);
1833+
});
1834+
});
17541835
});
17551836

17561837
// Reproduces https://github.com/firebase/firebase-js-sdk/issues/5873

packages/firestore/test/unit/core/query.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ describe('Query', () => {
256256
document = doc('collection/1', 0, {
257257
zip: null
258258
});
259-
expect(queryMatches(query1, document)).to.be.true;
259+
expect(queryMatches(query1, document)).to.be.false;
260260

261261
// NaN match.
262262
document = doc('collection/1', 0, {
@@ -354,7 +354,7 @@ describe('Query', () => {
354354
expect(queryMatches(query2, doc3)).to.equal(true);
355355
expect(queryMatches(query2, doc4)).to.equal(true);
356356
expect(queryMatches(query2, doc5)).to.equal(true);
357-
expect(queryMatches(query2, doc6)).to.equal(true);
357+
expect(queryMatches(query2, doc6)).to.equal(false);
358358
});
359359

360360
it('matches null for filters', () => {

0 commit comments

Comments
 (0)