fix: remove null value inclusion from != and not-in filter results (#8915)

Author: milaGGL

.changeset/cyan-frogs-relate.md

@@ -0,0 +1,6 @@
+---
+'@firebase/firestore': patch
+'firebase': patch
+---
+
+Fixed the `null` value handling in `!=` and `not-in` filters.

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)
+    );
   }
 }
 

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

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', () => {