From 5f538ce4514295bbcfc25d45c4b8cccebb67a0b9 Mon Sep 17 00:00:00 2001 From: cherylEnkidu Date: Mon, 5 Jan 2026 15:43:08 -0500 Subject: [PATCH 1/6] Change the behaviour of not-in and != --- .../Integration/QueryIntegrationTests.swift | 3 ++ .../Integration/QueryToPipelineTests.swift | 33 ++++++++++++++++++- Firestore/core/src/core/pipeline_util.cc | 24 +++++++++++--- 3 files changed, 54 insertions(+), 6 deletions(-) diff --git a/Firestore/Swift/Tests/Integration/QueryIntegrationTests.swift b/Firestore/Swift/Tests/Integration/QueryIntegrationTests.swift index e3f5b5f6888..293eb62fc60 100644 --- a/Firestore/Swift/Tests/Integration/QueryIntegrationTests.swift +++ b/Firestore/Swift/Tests/Integration/QueryIntegrationTests.swift @@ -206,6 +206,9 @@ class QueryIntegrationTests: FSTIntegrationTestCase { } func testMultipleInOps() async throws { + try XCTSkipIf(!FSTIntegrationTestCase.isRunningAgainstEmulator(), + "Skip this test if running against production.") + let collRef = collectionRef( withDocuments: ["doc1": ["a": 1, "b": 0], "doc2": ["b": 1], diff --git a/Firestore/Swift/Tests/Integration/QueryToPipelineTests.swift b/Firestore/Swift/Tests/Integration/QueryToPipelineTests.swift index 8588bd1b0b9..59efc9840df 100644 --- a/Firestore/Swift/Tests/Integration/QueryToPipelineTests.swift +++ b/Firestore/Swift/Tests/Integration/QueryToPipelineTests.swift @@ -565,6 +565,11 @@ class QueryToPipelineTests: FSTIntegrationTestCase { } func testSupportsNeqNan() async throws { + try XCTSkipIf( + FSTIntegrationTestCase.isRunningAgainstEmulator(), + "Skipping test because the emulator's behavior deviates from the expected outcome." + ) + let collRef = collectionRef(withDocuments: [ "1": ["foo": 1, "bar": Double.nan], "2": ["foo": 2, "bar": 1], @@ -579,6 +584,11 @@ class QueryToPipelineTests: FSTIntegrationTestCase { } func testSupportsEqNull() async throws { + try XCTSkipIf( + FSTIntegrationTestCase.isRunningAgainstEmulator(), + "Skipping test because the emulator's behavior deviates from the expected outcome." + ) + let collRef = collectionRef(withDocuments: [ "1": ["foo": 1, "bar": NSNull()], "2": ["foo": 2, "bar": 1], @@ -593,6 +603,11 @@ class QueryToPipelineTests: FSTIntegrationTestCase { } func testSupportsNeqNull() async throws { + try XCTSkipIf( + FSTIntegrationTestCase.isRunningAgainstEmulator(), + "Skipping test because the emulator's behavior deviates from the expected outcome." + ) + let collRef = collectionRef(withDocuments: [ "1": ["foo": 1, "bar": NSNull()], "2": ["foo": 2, "bar": 1], @@ -701,6 +716,11 @@ class QueryToPipelineTests: FSTIntegrationTestCase { } func testSupportsNotInWith1() async throws { + try XCTSkipIf( + FSTIntegrationTestCase.isRunningAgainstEmulator(), + "Skipping test because the emulator's behavior deviates from the expected outcome." + ) + let collRef = collectionRef(withDocuments: [ "1": ["foo": 1, "bar": 2], "2": ["foo": 2], @@ -712,7 +732,18 @@ class QueryToPipelineTests: FSTIntegrationTestCase { let pipeline = db.pipeline().create(from: query) let snapshot = try await pipeline.execute() - verifyResults(snapshot, [["foo": 3, "bar": 10]]) + switch FSTIntegrationTestCase.backendEdition() { + case .standard: + // In Standard, `NOT_IN` requires the field to exist. + // So document "2" (with no "bar" field) is filtered out. + verifyResults(snapshot, [["foo": 3, "bar": 10]]) + case .enterprise: + // In Enterprise, `NOT_IN` does not require the field to exist. + // So document "2" (with no "bar" field) is included. + verifyResults(snapshot, [["foo": 2], ["foo": 3, "bar": 10]]) + @unknown default: + XCTFail("Unknown backend edition") + } } func testSupportsOrOperator() async throws { diff --git a/Firestore/core/src/core/pipeline_util.cc b/Firestore/core/src/core/pipeline_util.cc index 0ebd3c39b52..0acfac5c5b0 100644 --- a/Firestore/core/src/core/pipeline_util.cc +++ b/Firestore/core/src/core/pipeline_util.cc @@ -602,6 +602,10 @@ std::shared_ptr ToPipelineBooleanExpr(const Filter& filter) { comparison_expr = std::make_shared( func_name, std::vector>{api_field, api_constant}); + if (op == FieldFilter::Operator::NotIn || + op == FieldFilter::Operator::NotEqual) { + return comparison_expr; + } return std::make_shared( "and", std::vector>{exists_expr, comparison_expr}); @@ -703,16 +707,26 @@ std::vector> ToPipelineStages( if (!query_order_bys.empty()) { std::vector> exists_exprs; exists_exprs.reserve(query_order_bys.size()); + const auto inequality_fields = query.InequalityFilterFields(); for (const auto& core_order_by : query_order_bys) { + if (inequality_fields.find(core_order_by.field()) != + inequality_fields.end()) { + continue; + } + if (core_order_by.field().IsKeyFieldPath()) { + continue; + } exists_exprs.push_back(std::make_shared( "exists", std::vector>{ std::make_shared(core_order_by.field())})); } - if (exists_exprs.size() == 1) { - stages.push_back(std::make_shared(exists_exprs[0])); - } else { - stages.push_back(std::make_shared( - std::make_shared("and", exists_exprs))); + if (!exists_exprs.empty()) { + if (exists_exprs.size() == 1) { + stages.push_back(std::make_shared(exists_exprs[0])); + } else { + stages.push_back(std::make_shared( + std::make_shared("and", exists_exprs))); + } } } From 59f6ced4d83fc7e4425a933716c02e05c6fa277a Mon Sep 17 00:00:00 2001 From: cherylEnkidu Date: Wed, 14 Jan 2026 16:24:08 -0500 Subject: [PATCH 2/6] correct code logic --- .../Integration/QueryToPipelineTests.swift | 20 ------------------- Firestore/core/src/core/pipeline_util.cc | 18 +---------------- 2 files changed, 1 insertion(+), 37 deletions(-) diff --git a/Firestore/Swift/Tests/Integration/QueryToPipelineTests.swift b/Firestore/Swift/Tests/Integration/QueryToPipelineTests.swift index 59efc9840df..a408f708019 100644 --- a/Firestore/Swift/Tests/Integration/QueryToPipelineTests.swift +++ b/Firestore/Swift/Tests/Integration/QueryToPipelineTests.swift @@ -565,11 +565,6 @@ class QueryToPipelineTests: FSTIntegrationTestCase { } func testSupportsNeqNan() async throws { - try XCTSkipIf( - FSTIntegrationTestCase.isRunningAgainstEmulator(), - "Skipping test because the emulator's behavior deviates from the expected outcome." - ) - let collRef = collectionRef(withDocuments: [ "1": ["foo": 1, "bar": Double.nan], "2": ["foo": 2, "bar": 1], @@ -584,11 +579,6 @@ class QueryToPipelineTests: FSTIntegrationTestCase { } func testSupportsEqNull() async throws { - try XCTSkipIf( - FSTIntegrationTestCase.isRunningAgainstEmulator(), - "Skipping test because the emulator's behavior deviates from the expected outcome." - ) - let collRef = collectionRef(withDocuments: [ "1": ["foo": 1, "bar": NSNull()], "2": ["foo": 2, "bar": 1], @@ -603,11 +593,6 @@ class QueryToPipelineTests: FSTIntegrationTestCase { } func testSupportsNeqNull() async throws { - try XCTSkipIf( - FSTIntegrationTestCase.isRunningAgainstEmulator(), - "Skipping test because the emulator's behavior deviates from the expected outcome." - ) - let collRef = collectionRef(withDocuments: [ "1": ["foo": 1, "bar": NSNull()], "2": ["foo": 2, "bar": 1], @@ -716,11 +701,6 @@ class QueryToPipelineTests: FSTIntegrationTestCase { } func testSupportsNotInWith1() async throws { - try XCTSkipIf( - FSTIntegrationTestCase.isRunningAgainstEmulator(), - "Skipping test because the emulator's behavior deviates from the expected outcome." - ) - let collRef = collectionRef(withDocuments: [ "1": ["foo": 1, "bar": 2], "2": ["foo": 2], diff --git a/Firestore/core/src/core/pipeline_util.cc b/Firestore/core/src/core/pipeline_util.cc index 0acfac5c5b0..5c58886c929 100644 --- a/Firestore/core/src/core/pipeline_util.cc +++ b/Firestore/core/src/core/pipeline_util.cc @@ -703,31 +703,15 @@ std::vector> ToPipelineStages( } // 3. OrderBy Existence Checks - const auto& query_order_bys = query.normalized_order_bys(); + const auto& query_order_bys = query.explicit_order_bys(); if (!query_order_bys.empty()) { std::vector> exists_exprs; exists_exprs.reserve(query_order_bys.size()); - const auto inequality_fields = query.InequalityFilterFields(); for (const auto& core_order_by : query_order_bys) { - if (inequality_fields.find(core_order_by.field()) != - inequality_fields.end()) { - continue; - } - if (core_order_by.field().IsKeyFieldPath()) { - continue; - } exists_exprs.push_back(std::make_shared( "exists", std::vector>{ std::make_shared(core_order_by.field())})); } - if (!exists_exprs.empty()) { - if (exists_exprs.size() == 1) { - stages.push_back(std::make_shared(exists_exprs[0])); - } else { - stages.push_back(std::make_shared( - std::make_shared("and", exists_exprs))); - } - } } // 4. Orderings, Cursors, Limit From c66338a9feb65910ce23e647ad4a7d9c85f5fe93 Mon Sep 17 00:00:00 2001 From: cherylEnkidu Date: Fri, 16 Jan 2026 11:49:43 -0500 Subject: [PATCH 3/6] fix bugs --- .../Integration/QueryToPipelineTests.swift | 73 +++++++++++++++++++ Firestore/core/src/core/pipeline_util.cc | 4 +- 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/Firestore/Swift/Tests/Integration/QueryToPipelineTests.swift b/Firestore/Swift/Tests/Integration/QueryToPipelineTests.swift index a408f708019..d61784060d7 100644 --- a/Firestore/Swift/Tests/Integration/QueryToPipelineTests.swift +++ b/Firestore/Swift/Tests/Integration/QueryToPipelineTests.swift @@ -750,4 +750,77 @@ class QueryToPipelineTests: FSTIntegrationTestCase { enforceOrder: true ) } + +// private func verifyIDs(_ snapshot: Pipeline.Snapshot, +// _ expected: [String], +// enforceOrder: Bool = false, +// file: StaticString = #file, +// line: UInt = #line) { +// let results = snapshot.results.map { $0.ref!.documentID } +// if enforceOrder { +// XCTAssertEqual(results, expected, "Result IDs do not match or are not in order.", +// file: file, line: line) +// } else { +// XCTAssertEqual(Set(results), Set(expected), "Result ID sets do not match.", +// file: file, line: line) +// } +// } +// +// func testNotInRemovesExistenceFilter() async throws { +// let collRef = collectionRef(withDocuments: [ +// "doc1": ["field": 2], +// "doc2": ["field": 1], +// "doc3": [:], +// ]) +// let db = collRef.firestore +// +// let query = collRef.whereField("field", notIn: [1]) +// let pipeline = db.pipeline().create(from: query) +// let snapshot = try await pipeline.execute() +// +// verifyIDs(snapshot, ["doc1", "doc3"]) +// } +// +// func testNotEqualRemovesExistenceFilter() async throws { +// let collRef = collectionRef(withDocuments: [ +// "doc1": ["field": 2], +// "doc2": ["field": 1], +// "doc3": [:], +// ]) +// let db = collRef.firestore +// +// let query = collRef.whereField("field", isNotEqualTo: 1) +// let pipeline = db.pipeline().create(from: query) +// let snapshot = try await pipeline.execute() +// +// verifyIDs(snapshot, ["doc1", "doc3"]) +// } +// +// func testInequalityMaintainsExistenceFilter() async throws { +// let collRef = collectionRef(withDocuments: [ +// "doc1": ["field": 0], +// "doc2": [:], +// ]) +// let db = collRef.firestore +// +// let query = collRef.whereField("field", isLessThan: 1) +// let pipeline = db.pipeline().create(from: query) +// let snapshot = try await pipeline.execute() +// +// verifyIDs(snapshot, ["doc1"]) +// } +// +// func testExplicitOrderMaintainsExistenceFilter() async throws { +// let collRef = collectionRef(withDocuments: [ +// "doc1": ["field": 1], +// "doc2": [:], +// ]) +// let db = collRef.firestore +// +// let query = collRef.order(by: "field") +// let pipeline = db.pipeline().create(from: query) +// let snapshot = try await pipeline.execute() +// +// verifyIDs(snapshot, ["doc1"]) +// } } diff --git a/Firestore/core/src/core/pipeline_util.cc b/Firestore/core/src/core/pipeline_util.cc index 5c58886c929..be8064a9ab2 100644 --- a/Firestore/core/src/core/pipeline_util.cc +++ b/Firestore/core/src/core/pipeline_util.cc @@ -755,7 +755,9 @@ std::vector> ToPipelineStages( stages.push_back(std::make_shared(api_orderings)); } } else { - stages.push_back(std::make_shared(api_orderings)); + if (!api_orderings.empty()) { + stages.push_back(std::make_shared(api_orderings)); + } } return stages; From ed027677723793718a076e0b4f39e241903b2f93 Mon Sep 17 00:00:00 2001 From: cherylEnkidu Date: Mon, 19 Jan 2026 16:03:33 -0500 Subject: [PATCH 4/6] fix impl error --- .../Tests/Integration/QueryToPipelineTests.swift | 12 +++++++++++- Firestore/core/src/core/pipeline_util.cc | 13 +++++++------ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/Firestore/Swift/Tests/Integration/QueryToPipelineTests.swift b/Firestore/Swift/Tests/Integration/QueryToPipelineTests.swift index d61784060d7..5d125cfad7c 100644 --- a/Firestore/Swift/Tests/Integration/QueryToPipelineTests.swift +++ b/Firestore/Swift/Tests/Integration/QueryToPipelineTests.swift @@ -37,7 +37,17 @@ class QueryToPipelineTests: FSTIntegrationTestCase { file: StaticString = #file, line: UInt = #line) { let results = snapshot.results.map { $0.data as! [String: AnyHashable?] } - XCTAssertEqual(results.count, expected.count, "Result count mismatch.", file: file, line: line) + print("results: \(results)") + print("expected: \(expected.map(\.debugDescription).joined(separator: "\n"))") + print("results.count: \(results.count), expected.count: \(expected.count)") + guard results.count == expected.count else { + XCTFail( + "Result count mismatch. Got \(results.count), expected \(expected.count)", + file: file, + line: line + ) + return + } if enforceOrder { for i in 0 ..< expected.count { diff --git a/Firestore/core/src/core/pipeline_util.cc b/Firestore/core/src/core/pipeline_util.cc index be8064a9ab2..2b1d70b4bfe 100644 --- a/Firestore/core/src/core/pipeline_util.cc +++ b/Firestore/core/src/core/pipeline_util.cc @@ -703,11 +703,11 @@ std::vector> ToPipelineStages( } // 3. OrderBy Existence Checks - const auto& query_order_bys = query.explicit_order_bys(); - if (!query_order_bys.empty()) { + const auto& query_explicit_order_bys = query.explicit_order_bys(); + if (!query_explicit_order_bys.empty()) { std::vector> exists_exprs; - exists_exprs.reserve(query_order_bys.size()); - for (const auto& core_order_by : query_order_bys) { + exists_exprs.reserve(query_explicit_order_bys.size()); + for (const auto& core_order_by : query.explicit_order_bys()) { exists_exprs.push_back(std::make_shared( "exists", std::vector>{ std::make_shared(core_order_by.field())})); @@ -716,8 +716,9 @@ std::vector> ToPipelineStages( // 4. Orderings, Cursors, Limit std::vector api_orderings; - api_orderings.reserve(query_order_bys.size()); - for (const auto& core_order_by : query_order_bys) { + const auto& query_normalized_order_bys = query.normalized_order_bys(); + api_orderings.reserve(query_normalized_order_bys.size()); + for (const auto& core_order_by : query_normalized_order_bys) { api_orderings.emplace_back( std::make_shared(core_order_by.field()), core_order_by.direction() == Direction::Ascending From 3900a4504f6448c7a3a57646d4eb8738266a3b4f Mon Sep 17 00:00:00 2001 From: cherylEnkidu Date: Mon, 19 Jan 2026 16:59:29 -0500 Subject: [PATCH 5/6] fix impl --- .../Integration/QueryToPipelineTests.swift | 147 +++++++++--------- Firestore/core/src/core/pipeline_util.cc | 11 ++ 2 files changed, 83 insertions(+), 75 deletions(-) diff --git a/Firestore/Swift/Tests/Integration/QueryToPipelineTests.swift b/Firestore/Swift/Tests/Integration/QueryToPipelineTests.swift index 5d125cfad7c..fd946669333 100644 --- a/Firestore/Swift/Tests/Integration/QueryToPipelineTests.swift +++ b/Firestore/Swift/Tests/Integration/QueryToPipelineTests.swift @@ -37,9 +37,6 @@ class QueryToPipelineTests: FSTIntegrationTestCase { file: StaticString = #file, line: UInt = #line) { let results = snapshot.results.map { $0.data as! [String: AnyHashable?] } - print("results: \(results)") - print("expected: \(expected.map(\.debugDescription).joined(separator: "\n"))") - print("results.count: \(results.count), expected.count: \(expected.count)") guard results.count == expected.count else { XCTFail( "Result count mismatch. Got \(results.count), expected \(expected.count)", @@ -761,76 +758,76 @@ class QueryToPipelineTests: FSTIntegrationTestCase { ) } -// private func verifyIDs(_ snapshot: Pipeline.Snapshot, -// _ expected: [String], -// enforceOrder: Bool = false, -// file: StaticString = #file, -// line: UInt = #line) { -// let results = snapshot.results.map { $0.ref!.documentID } -// if enforceOrder { -// XCTAssertEqual(results, expected, "Result IDs do not match or are not in order.", -// file: file, line: line) -// } else { -// XCTAssertEqual(Set(results), Set(expected), "Result ID sets do not match.", -// file: file, line: line) -// } -// } -// -// func testNotInRemovesExistenceFilter() async throws { -// let collRef = collectionRef(withDocuments: [ -// "doc1": ["field": 2], -// "doc2": ["field": 1], -// "doc3": [:], -// ]) -// let db = collRef.firestore -// -// let query = collRef.whereField("field", notIn: [1]) -// let pipeline = db.pipeline().create(from: query) -// let snapshot = try await pipeline.execute() -// -// verifyIDs(snapshot, ["doc1", "doc3"]) -// } -// -// func testNotEqualRemovesExistenceFilter() async throws { -// let collRef = collectionRef(withDocuments: [ -// "doc1": ["field": 2], -// "doc2": ["field": 1], -// "doc3": [:], -// ]) -// let db = collRef.firestore -// -// let query = collRef.whereField("field", isNotEqualTo: 1) -// let pipeline = db.pipeline().create(from: query) -// let snapshot = try await pipeline.execute() -// -// verifyIDs(snapshot, ["doc1", "doc3"]) -// } -// -// func testInequalityMaintainsExistenceFilter() async throws { -// let collRef = collectionRef(withDocuments: [ -// "doc1": ["field": 0], -// "doc2": [:], -// ]) -// let db = collRef.firestore -// -// let query = collRef.whereField("field", isLessThan: 1) -// let pipeline = db.pipeline().create(from: query) -// let snapshot = try await pipeline.execute() -// -// verifyIDs(snapshot, ["doc1"]) -// } -// -// func testExplicitOrderMaintainsExistenceFilter() async throws { -// let collRef = collectionRef(withDocuments: [ -// "doc1": ["field": 1], -// "doc2": [:], -// ]) -// let db = collRef.firestore -// -// let query = collRef.order(by: "field") -// let pipeline = db.pipeline().create(from: query) -// let snapshot = try await pipeline.execute() -// -// verifyIDs(snapshot, ["doc1"]) -// } + private func verifyIDs(_ snapshot: Pipeline.Snapshot, + _ expected: [String], + enforceOrder: Bool = false, + file: StaticString = #file, + line: UInt = #line) { + let results = snapshot.results.map { $0.ref!.documentID } + if enforceOrder { + XCTAssertEqual(results, expected, "Result IDs do not match or are not in order.", + file: file, line: line) + } else { + XCTAssertEqual(Set(results), Set(expected), "Result ID sets do not match.", + file: file, line: line) + } + } + + func testNotInRemovesExistenceFilter() async throws { + let collRef = collectionRef(withDocuments: [ + "doc1": ["field": 2], + "doc2": ["field": 1], + "doc3": [:], + ]) + let db = collRef.firestore + + let query = collRef.whereField("field", notIn: [1]) + let pipeline = db.pipeline().create(from: query) + let snapshot = try await pipeline.execute() + + verifyIDs(snapshot, ["doc1", "doc3"]) + } + + func testNotEqualRemovesExistenceFilter() async throws { + let collRef = collectionRef(withDocuments: [ + "doc1": ["field": 2], + "doc2": ["field": 1], + "doc3": [:], + ]) + let db = collRef.firestore + + let query = collRef.whereField("field", isNotEqualTo: 1) + let pipeline = db.pipeline().create(from: query) + let snapshot = try await pipeline.execute() + + verifyIDs(snapshot, ["doc1", "doc3"]) + } + + func testInequalityMaintainsExistenceFilter() async throws { + let collRef = collectionRef(withDocuments: [ + "doc1": ["field": 0], + "doc2": [:], + ]) + let db = collRef.firestore + + let query = collRef.whereField("field", isLessThan: 1) + let pipeline = db.pipeline().create(from: query) + let snapshot = try await pipeline.execute() + + verifyIDs(snapshot, ["doc1"]) + } + + func testExplicitOrderMaintainsExistenceFilter() async throws { + let collRef = collectionRef(withDocuments: [ + "doc1": ["field": 1], + "doc2": [:], + ]) + let db = collRef.firestore + + let query = collRef.order(by: "field") + let pipeline = db.pipeline().create(from: query) + let snapshot = try await pipeline.execute() + + verifyIDs(snapshot, ["doc1"]) + } } diff --git a/Firestore/core/src/core/pipeline_util.cc b/Firestore/core/src/core/pipeline_util.cc index 2b1d70b4bfe..c400a654182 100644 --- a/Firestore/core/src/core/pipeline_util.cc +++ b/Firestore/core/src/core/pipeline_util.cc @@ -712,6 +712,17 @@ std::vector> ToPipelineStages( "exists", std::vector>{ std::make_shared(core_order_by.field())})); } + + if (!exists_exprs.empty()) { + std::shared_ptr final_exists_expr; + if (exists_exprs.size() == 1) { + final_exists_expr = exists_exprs[0]; + } else { + final_exists_expr = + std::make_shared("and", exists_exprs); + } + stages.push_back(std::make_shared(final_exists_expr)); + } } // 4. Orderings, Cursors, Limit From e1c26fec4a5052a0b4ee91025b4441a84de88a7d Mon Sep 17 00:00:00 2001 From: cherylEnkidu Date: Tue, 20 Jan 2026 12:06:58 -0500 Subject: [PATCH 6/6] improve code logic --- Firestore/core/src/core/pipeline_util.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Firestore/core/src/core/pipeline_util.cc b/Firestore/core/src/core/pipeline_util.cc index c400a654182..53950a4384d 100644 --- a/Firestore/core/src/core/pipeline_util.cc +++ b/Firestore/core/src/core/pipeline_util.cc @@ -767,9 +767,7 @@ std::vector> ToPipelineStages( stages.push_back(std::make_shared(api_orderings)); } } else { - if (!api_orderings.empty()) { - stages.push_back(std::make_shared(api_orderings)); - } + stages.push_back(std::make_shared(api_orderings)); } return stages;