diff --git a/.changeset/cyan-frogs-relate.md b/.changeset/cyan-frogs-relate.md new file mode 100644 index 00000000000..08af27593a9 --- /dev/null +++ b/.changeset/cyan-frogs-relate.md @@ -0,0 +1,6 @@ +--- +'@firebase/firestore': patch +'firebase': patch +--- + +Fixed the `null` value handling in `!=` and `not-in` filters. diff --git a/packages/firestore/src/core/filter.ts b/packages/firestore/src/core/filter.ts index 06e2740c315..12b57729f81 100644 --- a/packages/firestore/src/core/filter.ts +++ b/packages/firestore/src/core/filter.ts @@ -141,6 +141,7 @@ export class FieldFilter extends Filter { if (this.op === Operator.NOT_EQUAL) { return ( other !== null && + other.nullValue === undefined && this.matchesComparison(valueCompare(other!, this.value)) ); } @@ -495,7 +496,11 @@ export class NotInFilter extends FieldFilter { return false; } const other = doc.data.field(this.field); - return other !== null && !arrayValueContains(this.value.arrayValue!, other); + return ( + other !== null && + other.nullValue === undefined && + !arrayValueContains(this.value.arrayValue!, other) + ); } } diff --git a/packages/firestore/test/integration/api/query.test.ts b/packages/firestore/test/integration/api/query.test.ts index 01fd0e47e35..5871607eb03 100644 --- a/packages/firestore/test/integration/api/query.test.ts +++ b/packages/firestore/test/integration/api/query.test.ts @@ -1751,6 +1751,100 @@ apiDescribe('Queries', persistence => { ); }); }); + + it('sdk uses != filter same as backend', async () => { + const testDocs = { + a: { zip: Number.NaN }, + b: { zip: 91102 }, + c: { zip: 98101 }, + d: { zip: '98101' }, + e: { zip: [98101] }, + f: { zip: [98101, 98102] }, + g: { zip: ['98101', { zip: 98101 }] }, + h: { zip: { code: 500 } }, + i: { zip: null }, + j: { code: 500 } + }; + + await withTestCollection(persistence, testDocs, async coll => { + // populate cache with all documents first to ensure getDocsFromCache() scans all docs + await getDocs(coll); + + let testQuery = query(coll, where('zip', '!=', 98101)); + await checkOnlineAndOfflineResultsMatch( + testQuery, + 'a', + 'b', + 'd', + 'e', + 'f', + 'g', + 'h' + ); + + testQuery = query(coll, where('zip', '!=', Number.NaN)); + await checkOnlineAndOfflineResultsMatch( + testQuery, + 'b', + 'c', + 'd', + 'e', + 'f', + 'g', + 'h' + ); + + testQuery = query(coll, where('zip', '!=', null)); + await checkOnlineAndOfflineResultsMatch( + testQuery, + 'a', + 'b', + 'c', + 'd', + 'e', + 'f', + 'g', + 'h' + ); + }); + }); + + it('sdk uses not-in filter same as backend', async () => { + const testDocs = { + a: { zip: Number.NaN }, + b: { zip: 91102 }, + c: { zip: 98101 }, + d: { zip: '98101' }, + e: { zip: [98101] }, + f: { zip: [98101, 98102] }, + g: { zip: ['98101', { zip: 98101 }] }, + h: { zip: { code: 500 } }, + i: { zip: null }, + j: { code: 500 } + }; + + await withTestCollection(persistence, testDocs, async coll => { + // populate cache with all documents first to ensure getDocsFromCache() scans all docs + await getDocs(coll); + + let testQuery = query( + coll, + where('zip', 'not-in', [98101, 98103, [98101, 98102]]) + ); + await checkOnlineAndOfflineResultsMatch( + testQuery, + 'a', + 'b', + 'd', + 'e', + 'g', + 'h' + ); + + testQuery = query(coll, where('zip', 'not-in', [null])); + await checkOnlineAndOfflineResultsMatch(testQuery); + }); + }); }); // Reproduces https://github.com/firebase/firebase-js-sdk/issues/5873 diff --git a/packages/firestore/test/unit/core/query.test.ts b/packages/firestore/test/unit/core/query.test.ts index fd0e6884c66..e589e7bf027 100644 --- a/packages/firestore/test/unit/core/query.test.ts +++ b/packages/firestore/test/unit/core/query.test.ts @@ -256,7 +256,7 @@ describe('Query', () => { document = doc('collection/1', 0, { zip: null }); - expect(queryMatches(query1, document)).to.be.true; + expect(queryMatches(query1, document)).to.be.false; // NaN match. document = doc('collection/1', 0, { @@ -354,7 +354,7 @@ describe('Query', () => { expect(queryMatches(query2, doc3)).to.equal(true); expect(queryMatches(query2, doc4)).to.equal(true); expect(queryMatches(query2, doc5)).to.equal(true); - expect(queryMatches(query2, doc6)).to.equal(true); + expect(queryMatches(query2, doc6)).to.equal(false); }); it('matches null for filters', () => {