fix: remove null value inclusion from isNotEqualTo and notIn filter results (#14704)

Author: milaGGL

Firestore/CHANGELOG.md

@@ -1,3 +1,6 @@
+# Unreleased
+- [fixed] Fixed the `null` value handling in `isNotEqualTo` and `notIn` filters.
+
 # 11.11.0
 - [fixed] Fixed the customized priority queue compare function used cache index manager. (#14496)
 

Firestore/Example/Tests/Integration/API/FIRQueryTests.mm

@@ -537,6 +537,35 @@ - (void)testQueriesCanUseNotEqualFiltersWithDocIds {
                         (@[ testDocs[@"ab"], testDocs[@"ba"], testDocs[@"bb"] ]));
 }
 
+- (void)testSDKUsesNotEqualFiltersSameAsServer {
+  NSDictionary *testDocs = @{
+    @"a" : @{@"zip" : @(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" : [NSNull null]},
+    @"j" : @{@"code" : @500},
+  };
+  FIRCollectionReference *collection = [self collectionRefWithDocuments:testDocs];
+
+  // populate cache with all documents first to ensure getDocsFromCache() scans all docs
+  [self readDocumentSetForRef:collection];
+
+  [self checkOnlineAndOfflineQuery:[collection queryWhereField:@"zip" isNotEqualTo:@98101]
+                     matchesResult:@[ @"a", @"b", @"d", @"e", @"f", @"g", @"h" ]];
+
+  [self checkOnlineAndOfflineQuery:[collection queryWhereField:@"zip" isNotEqualTo:@(NAN)]
+                     matchesResult:@[ @"b", @"c", @"d", @"e", @"f", @"g", @"h" ]];
+
+  [self checkOnlineAndOfflineQuery:[collection queryWhereField:@"zip"
+                                                  isNotEqualTo:@[ [NSNull null] ]]
+                     matchesResult:@[ @"a", @"b", @"c", @"d", @"e", @"f", @"g", @"h" ]];
+}
+
 - (void)testQueriesCanUseArrayContainsFilters {
   NSDictionary *testDocs = @{
     @"a" : @{@"array" : @[ @42 ]},
@@ -694,6 +723,33 @@ - (void)testQueriesCanUseNotInFiltersWithDocIds {
   XCTAssertEqualObjects(FIRQuerySnapshotGetData(snapshot), (@[ testDocs[@"ba"], testDocs[@"bb"] ]));
 }
 
+- (void)testSDKUsesNotInFiltersSameAsServer {
+  NSDictionary *testDocs = @{
+    @"a" : @{@"zip" : @(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" : [NSNull null]},
+    @"j" : @{@"code" : @500},
+  };
+  FIRCollectionReference *collection = [self collectionRefWithDocuments:testDocs];
+
+  // populate cache with all documents first to ensure getDocsFromCache() scans all docs
+  [self readDocumentSetForRef:collection];
+
+  [self checkOnlineAndOfflineQuery:[collection
+                                       queryWhereField:@"zip"
+                                                 notIn:@[ @98101, @98103, @[ @98101, @98102 ] ]]
+                     matchesResult:@[ @"a", @"b", @"d", @"e", @"g", @"h" ]];
+
+  [self checkOnlineAndOfflineQuery:[collection queryWhereField:@"zip" notIn:@[ [NSNull null] ]]
+                     matchesResult:@[]];
+}
+
 - (void)testQueriesCanUseArrayContainsAnyFilters {
   NSDictionary *testDocs = @{
     @"a" : @{@"array" : @[ @42 ]},

Firestore/core/src/core/field_filter.cc

@@ -157,7 +157,8 @@ bool FieldFilter::Rep::Matches(const model::Document& doc) const {
 
   // Types do not have to match in NotEqual filters.
   if (op_ == Operator::NotEqual) {
-    return MatchesComparison(Compare(lhs, *value_rhs_));
+    return lhs.which_value_type != google_firestore_v1_Value_null_value_tag &&
+           MatchesComparison(Compare(lhs, *value_rhs_));
   }
 
   // Only compare types with matching backend order (such as double and int).

Firestore/core/src/core/not_in_filter.cc

@@ -62,7 +62,10 @@ bool NotInFilter::Rep::Matches(const Document& doc) const {
     return false;
   }
   absl::optional<google_firestore_v1_Value> maybe_lhs = doc->field(field());
-  return maybe_lhs && !Contains(array_value, *maybe_lhs);
+  return maybe_lhs &&
+         maybe_lhs->which_value_type !=
+             google_firestore_v1_Value_null_value_tag &&
+         !Contains(array_value, *maybe_lhs);
 }
 
 }  // namespace core

Firestore/core/test/unit/core/query_test.cc

@@ -279,7 +279,7 @@ TEST(QueryTest, NanFilter) {
   EXPECT_THAT(query, Matches(doc3));
   EXPECT_THAT(query, Matches(doc4));
   EXPECT_THAT(query, Matches(doc5));
-  EXPECT_THAT(query, Matches(doc6));
+  EXPECT_THAT(query, Not(Matches(doc6)));
 }
 
 TEST(QueryTest, ArrayContainsFilter) {
@@ -383,7 +383,7 @@ TEST(QueryTest, NotInFilters) {
 
   // Null match.
   doc = Doc("collection/1", 0, Map("zip", nullptr));
-  EXPECT_THAT(query, Matches(doc));
+  EXPECT_THAT(query, Not(Matches(doc)));
 
   // NAN match.
   doc = Doc("collection/1", 0, Map("zip", NAN));