diff --git a/Firestore/Example/Tests/Integration/API/FIRQueryTests.mm b/Firestore/Example/Tests/Integration/API/FIRQueryTests.mm index 7bc875eda50..c68ba764342 100644 --- a/Firestore/Example/Tests/Integration/API/FIRQueryTests.mm +++ b/Firestore/Example/Tests/Integration/API/FIRQueryTests.mm @@ -23,6 +23,10 @@ #import "Firestore/Example/Tests/Util/FSTIntegrationTestCase.h" #import "Firestore/Example/Tests/Util/FSTTestingHooks.h" +// TODO(MIEQ) update these imports with public imports when aggregate types are public +#import "Firestore/Source/API/FIRAggregateQuerySnapshot+Internal.h" +#import "Firestore/Source/API/FIRQuery+Internal.h" + @interface FIRQueryTests : FSTIntegrationTestCase @end @@ -1166,6 +1170,512 @@ - (void)testOrderByEquality { matchesResult:@[ @"doc6", @"doc3" ]]; } +// Multiple Inequality +- (void)testMultipleInequalityOnDifferentFields { + // TODO(MIEQ): Enable this test against production when possible. + XCTSkipIf(![FSTIntegrationTestCase isRunningAgainstEmulator], + "Skip this test if running against production because multiple inequality is " + "not supported yet."); + + FIRCollectionReference *collRef = [self collectionRefWithDocuments:@{ + @"doc1" : @{@"key" : @"a", @"sort" : @0, @"v" : @0}, + @"doc2" : @{@"key" : @"b", @"sort" : @3, @"v" : @1}, + @"doc3" : @{@"key" : @"c", @"sort" : @1, @"v" : @3}, + @"doc4" : @{@"key" : @"d", @"sort" : @2, @"v" : @2} + }]; + + // Multiple inequality fields + FIRQuerySnapshot *snapshot = [self + readDocumentSetForRef:[[[collRef queryWhereField:@"key" isNotEqualTo:@"a"] + queryWhereField:@"sort" + isLessThanOrEqualTo:@2] queryWhereField:@"v" isGreaterThan:@2]]; + XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(snapshot), (@[ @"doc3" ])); + + // Duplicate inequality fields + snapshot = [self readDocumentSetForRef:[[[collRef queryWhereField:@"key" isNotEqualTo:@"a"] + queryWhereField:@"sort" + isLessThanOrEqualTo:@2] queryWhereField:@"sort" + isGreaterThan:@1]]; + XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(snapshot), (@[ @"doc4" ])); + + // With multiple IN + snapshot = [self + readDocumentSetForRef:[[[[collRef queryWhereField:@"key" + isGreaterThanOrEqualTo:@"a"] queryWhereField:@"sort" + isLessThanOrEqualTo:@2] + queryWhereField:@"v" + in:@[ @2, @3, @4 ]] queryWhereField:@"sort" + in:@[ @2, @3 ]]]; + XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(snapshot), (@[ @"doc4" ])); + + // With NOT-IN + snapshot = [self readDocumentSetForRef:[[[collRef queryWhereField:@"key" + isGreaterThanOrEqualTo:@"a"] queryWhereField:@"sort" + isLessThanOrEqualTo:@2] + queryWhereField:@"v" + notIn:@[ @2, @4, @5 ]]]; + XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(snapshot), (@[ @"doc1", @"doc3" ])); + + // With orderby + snapshot = [self readDocumentSetForRef:[[[collRef queryWhereField:@"key" + isGreaterThanOrEqualTo:@"a"] queryWhereField:@"sort" + isLessThanOrEqualTo:@2] + queryOrderedByField:@"v" + descending:YES]]; + XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(snapshot), (@[ @"doc3", @"doc4", @"doc1" ])); + + // With limit + snapshot = [self readDocumentSetForRef:[[[[collRef queryWhereField:@"key" + isGreaterThanOrEqualTo:@"a"] queryWhereField:@"sort" + isLessThanOrEqualTo:@2] + queryOrderedByField:@"v" + descending:YES] queryLimitedTo:2]]; + XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(snapshot), (@[ @"doc3", @"doc4" ])); + + // With limitedToLast + snapshot = [self readDocumentSetForRef:[[[[collRef queryWhereField:@"key" + isGreaterThanOrEqualTo:@"a"] queryWhereField:@"sort" + isLessThanOrEqualTo:@2] + queryOrderedByField:@"v" + descending:YES] queryLimitedToLast:2]]; + XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(snapshot), (@[ @"doc4", @"doc1" ])); +} + +- (void)testMultipleInequalityOnSpecialValues { + // TODO(MIEQ): Enable this test against production when possible. + XCTSkipIf(![FSTIntegrationTestCase isRunningAgainstEmulator], + "Skip this test if running against production because multiple inequality is " + "not supported yet."); + + FIRCollectionReference *collRef = [self collectionRefWithDocuments:@{ + @"doc1" : @{@"key" : @"a", @"sort" : @0, @"v" : @0}, + @"doc2" : @{@"key" : @"b", @"sort" : @(NAN), @"v" : @1}, + @"doc3" : @{@"key" : @"c", @"sort" : [NSNull null], @"v" : @3}, + @"doc4" : @{@"key" : @"d", @"v" : @0}, + @"doc5" : @{@"key" : @"e", @"sort" : @1}, + @"doc6" : @{@"key" : @"f", @"sort" : @1, @"v" : @1} + }]; + + FIRQuerySnapshot *snapshot = + [self readDocumentSetForRef:[[collRef queryWhereField:@"key" + isNotEqualTo:@"a"] queryWhereField:@"sort" + isLessThanOrEqualTo:@2]]; + XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(snapshot), (@[ @"doc5", @"doc6" ])); + + snapshot = [self readDocumentSetForRef:[[[collRef queryWhereField:@"key" isNotEqualTo:@"a"] + queryWhereField:@"sort" + isLessThanOrEqualTo:@2] queryWhereField:@"v" + isLessThanOrEqualTo:@1]]; + XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(snapshot), (@[ @"doc6" ])); +} +- (void)testMultipleInequalityWithArrayMembership { + // TODO(MIEQ): Enable this test against production when possible. + XCTSkipIf(![FSTIntegrationTestCase isRunningAgainstEmulator], + "Skip this test if running against production because multiple inequality is " + "not supported yet."); + + FIRCollectionReference *collRef = [self collectionRefWithDocuments:@{ + @"doc1" : @{@"key" : @"a", @"sort" : @0, @"v" : @[ @0 ]}, + @"doc2" : @{@"key" : @"b", @"sort" : @1, @"v" : @[ @0, @1, @3 ]}, + @"doc3" : @{@"key" : @"c", @"sort" : @1, @"v" : @[]}, + @"doc4" : @{@"key" : @"d", @"sort" : @2, @"v" : @[ @1 ]}, + @"doc5" : @{@"key" : @"e", @"sort" : @3, @"v" : @[ @2, @4 ]}, + @"doc6" : @{@"key" : @"f", @"sort" : @4, @"v" : @[ @(NAN) ]}, + @"doc7" : @{@"key" : @"g", @"sort" : @4, @"v" : @[ [NSNull null] ]} + + }]; + + FIRQuerySnapshot *snapshot = [self + readDocumentSetForRef:[[[collRef queryWhereField:@"key" isNotEqualTo:@"a"] + queryWhereField:@"sort" + isGreaterThanOrEqualTo:@1] queryWhereField:@"v" arrayContains:@0]]; + XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(snapshot), (@[ @"doc2" ])); + + snapshot = [self readDocumentSetForRef:[[[collRef queryWhereField:@"key" + isNotEqualTo:@"a"] queryWhereField:@"sort" + isGreaterThanOrEqualTo:@1] + queryWhereField:@"v" + arrayContainsAny:@[ @0, @1 ]]]; + XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(snapshot), (@[ @"doc2", @"doc4" ])); +} + +- (NSDictionary *)nestedData:(int)number { + return @{ + @"name" : [NSString stringWithFormat:@"room %d", number], + @"metadata" : @{@"createdAt" : @(number)}, + @"field" : [NSString stringWithFormat:@"field %d", number], + @"field.dot" : @(number), + @"field\\slash" : @(number) + }; +} + +- (void)testMultipleInequalityWithNestedField { + // TODO(MIEQ): Enable this test against production when possible. + XCTSkipIf(![FSTIntegrationTestCase isRunningAgainstEmulator], + "Skip this test if running against production because multiple inequality is " + "not supported yet."); + + FIRCollectionReference *collRef = [self collectionRefWithDocuments:@{ + @"doc1" : [self nestedData:400], + @"doc2" : [self nestedData:200], + @"doc3" : [self nestedData:100], + @"doc4" : [self nestedData:300] + }]; + + FIRQuerySnapshot *snapshot = [self + readDocumentSetForRef:[[[[collRef queryWhereField:@"metadata.createdAt" + isLessThanOrEqualTo:@500] queryWhereField:@"metadata.createdAt" + isGreaterThan:@100] + queryWhereField:@"name" + isNotEqualTo:@"room 200"] queryOrderedByField:@"name" + descending:NO]]; + XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(snapshot), (@[ @"doc4", @"doc1" ])); + + snapshot = + [self readDocumentSetForRef:[[[[collRef queryWhereField:@"field" + isGreaterThanOrEqualTo:@"field 100"] + queryWhereFieldPath:[[FIRFieldPath alloc] + initWithFields:@[ @"field.dot" ]] + isNotEqualTo:@300] queryWhereField:@"field\\slash" + isLessThan:@400] + queryOrderedByField:@"name" + descending:YES]]; + XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(snapshot), (@[ @"doc2", @"doc3" ])); +} + +- (void)testMultipleInequalityWithCompositeFilters { + // TODO(MIEQ): Enable this test against production when possible. + XCTSkipIf(![FSTIntegrationTestCase isRunningAgainstEmulator], + "Skip this test if running against production because multiple inequality is " + "not supported yet."); + + FIRCollectionReference *collRef = [self collectionRefWithDocuments:@{ + @"doc1" : @{@"key" : @"a", @"sort" : @0, @"v" : @5}, + @"doc2" : @{@"key" : @"aa", @"sort" : @4, @"v" : @4}, + @"doc3" : @{@"key" : @"c", @"sort" : @3, @"v" : @3}, + @"doc4" : @{@"key" : @"b", @"sort" : @2, @"v" : @2}, + @"doc5" : @{@"key" : @"b", @"sort" : @2, @"v" : @1}, + @"doc6" : @{@"key" : @"b", @"sort" : @0, @"v" : @0} + }]; + + FIRQuerySnapshot *snapshot = [self + readDocumentSetForRef:[collRef queryWhereFilter:[FIRFilter orFilterWithFilters:@[ + [FIRFilter andFilterWithFilters:@[ + [FIRFilter filterWhereField:@"key" isEqualTo:@"b"], + [FIRFilter filterWhereField:@"sort" isLessThanOrEqualTo:@2] + ]], + [FIRFilter andFilterWithFilters:@[ + [FIRFilter filterWhereField:@"key" isNotEqualTo:@"b"], + [FIRFilter filterWhereField:@"v" isGreaterThan:@4] + ]] + ]]]]; + // Implicitly ordered by: 'key' asc, 'sort' asc, 'v' asc, __name__ asc + XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(snapshot), + (@[ @"doc1", @"doc6", @"doc5", @"doc4" ])); + + snapshot = [self + readDocumentSetForRef:[[[collRef queryWhereFilter:[FIRFilter orFilterWithFilters:@[ + [FIRFilter andFilterWithFilters:@[ + [FIRFilter filterWhereField:@"key" isEqualTo:@"b"], + [FIRFilter filterWhereField:@"sort" + isLessThanOrEqualTo:@2] + ]], + [FIRFilter andFilterWithFilters:@[ + [FIRFilter filterWhereField:@"key" isNotEqualTo:@"b"], + [FIRFilter filterWhereField:@"v" isGreaterThan:@4] + ]] + ]]] queryOrderedByField:@"sort" + descending:YES] queryOrderedByField:@"key"]]; + // Ordered by: 'sort' desc, 'key' asc, 'v' asc, __name__ asc + XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(snapshot), + (@[ @"doc5", @"doc4", @"doc1", @"doc6" ])); + + snapshot = [self + readDocumentSetForRef:[collRef + queryWhereFilter:[FIRFilter andFilterWithFilters:@[ + + [FIRFilter orFilterWithFilters:@[ + [FIRFilter andFilterWithFilters:@[ + [FIRFilter filterWhereField:@"key" isEqualTo:@"b"], + [FIRFilter filterWhereField:@"sort" isLessThanOrEqualTo:@4] + ]], + [FIRFilter andFilterWithFilters:@[ + [FIRFilter filterWhereField:@"key" isNotEqualTo:@"b"], + [FIRFilter filterWhereField:@"v" isGreaterThanOrEqualTo:@4] + ]] + ]], + + [FIRFilter orFilterWithFilters:@[ + [FIRFilter andFilterWithFilters:@[ + [FIRFilter filterWhereField:@"key" isGreaterThan:@"b"], + [FIRFilter filterWhereField:@"sort" isGreaterThanOrEqualTo:@1] + ]], + [FIRFilter andFilterWithFilters:@[ + [FIRFilter filterWhereField:@"key" isLessThan:@"b"], + [FIRFilter filterWhereField:@"v" isGreaterThan:@0] + ]] + ]] + + ]]]]; + // Implicitly ordered by: 'key' asc, 'sort' asc, 'v' asc, __name__ asc + XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(snapshot), (@[ @"doc1", @"doc2" ])); +} + +- (void)testMultipleInequalityFieldsWillBeImplicitlyOrderedLexicographically { + // TODO(MIEQ): Enable this test against production when possible. + XCTSkipIf(![FSTIntegrationTestCase isRunningAgainstEmulator], + "Skip this test if running against production because multiple inequality is " + "not supported yet."); + + FIRCollectionReference *collRef = [self collectionRefWithDocuments:@{ + @"doc1" : @{@"key" : @"a", @"sort" : @0, @"v" : @5}, + @"doc2" : @{@"key" : @"aa", @"sort" : @4, @"v" : @4}, + @"doc3" : @{@"key" : @"b", @"sort" : @3, @"v" : @3}, + @"doc4" : @{@"key" : @"b", @"sort" : @2, @"v" : @2}, + @"doc5" : @{@"key" : @"b", @"sort" : @2, @"v" : @1}, + @"doc6" : @{@"key" : @"b", @"sort" : @0, @"v" : @0} + }]; + + FIRQuerySnapshot *snapshot = [self + readDocumentSetForRef:[[[collRef queryWhereField:@"key" isNotEqualTo:@"a"] + queryWhereField:@"sort" + isGreaterThan:@1] queryWhereField:@"v" in:@[ @1, @2, @3, @4 ]]]; + // Implicitly ordered by: 'key' asc, 'sort' asc, __name__ asc + XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(snapshot), + (@[ @"doc2", @"doc4", @"doc5", @"doc3" ])); + + snapshot = [self readDocumentSetForRef:[[[collRef queryWhereField:@"sort" + isGreaterThan:@1] queryWhereField:@"key" + isNotEqualTo:@"a"] + queryWhereField:@"v" + in:@[ @1, @2, @3, @4 ]]]; + // Implicitly ordered by: 'key' asc, 'sort' asc, __name__ asc + XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(snapshot), + (@[ @"doc2", @"doc4", @"doc5", @"doc3" ])); +} + +- (void)testMultipleInequalityWithMultipleExplicitOrderBy { + // TODO(MIEQ): Enable this test against production when possible. + XCTSkipIf(![FSTIntegrationTestCase isRunningAgainstEmulator], + "Skip this test if running against production because multiple inequality is " + "not supported yet."); + + FIRCollectionReference *collRef = [self collectionRefWithDocuments:@{ + @"doc1" : @{@"key" : @"a", @"sort" : @5, @"v" : @0}, + @"doc2" : @{@"key" : @"aa", @"sort" : @4, @"v" : @0}, + @"doc3" : @{@"key" : @"b", @"sort" : @3, @"v" : @1}, + @"doc4" : @{@"key" : @"b", @"sort" : @2, @"v" : @1}, + @"doc5" : @{@"key" : @"bb", @"sort" : @1, @"v" : @1}, + @"doc6" : @{@"key" : @"c", @"sort" : @0, @"v" : @2} + }]; + + FIRQuerySnapshot *snapshot = [self + readDocumentSetForRef:[[[collRef queryWhereField:@"key" isGreaterThan:@"a"] + queryWhereField:@"sort" + isGreaterThanOrEqualTo:@1] queryOrderedByField:@"v" descending:NO]]; + // Ordered by: 'v' asc, 'key' asc, 'sort' asc, __name__ asc + XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(snapshot), + (@[ @"doc2", @"doc4", @"doc3", @"doc5" ])); + + snapshot = [self readDocumentSetForRef:[[[[collRef queryWhereField:@"key" + isGreaterThan:@"a"] queryWhereField:@"sort" + isGreaterThanOrEqualTo:@1] + queryOrderedByField:@"v" + descending:NO] queryOrderedByField:@"sort" + descending:NO]]; + // Ordered by: 'v asc, 'sort' asc, 'key' asc, __name__ asc + XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(snapshot), + (@[ @"doc2", @"doc5", @"doc4", @"doc3" ])); + + snapshot = [self readDocumentSetForRef:[[[collRef queryWhereField:@"key" isGreaterThan:@"a"] + queryWhereField:@"sort" + isGreaterThanOrEqualTo:@1] queryOrderedByField:@"v" + descending:YES]]; + // Implicit order by matches the direction of last explicit order by. + // Ordered by: 'v' desc, 'key' desc, 'sort' desc, __name__ desc + XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(snapshot), + (@[ @"doc5", @"doc3", @"doc4", @"doc2" ])); + + snapshot = [self readDocumentSetForRef:[[[[collRef queryWhereField:@"key" + isGreaterThan:@"a"] queryWhereField:@"sort" + isGreaterThanOrEqualTo:@1] + queryOrderedByField:@"v" + descending:YES] queryOrderedByField:@"sort" + descending:NO]]; + // Ordered by: 'v desc, 'sort' asc, 'key' asc, __name__ asc + XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(snapshot), + (@[ @"doc5", @"doc4", @"doc3", @"doc2" ])); +} + +- (void)testMultipleInequalityInAggregateQuery { + // TODO(MIEQ): Enable this test against production when possible. + XCTSkipIf(![FSTIntegrationTestCase isRunningAgainstEmulator], + "Skip this test if running against production because multiple inequality is " + "not supported yet."); + + FIRCollectionReference *collRef = [self collectionRefWithDocuments:@{ + @"doc1" : @{@"key" : @"a", @"sort" : @5, @"v" : @0}, + @"doc2" : @{@"key" : @"aa", @"sort" : @4, @"v" : @0}, + @"doc3" : @{@"key" : @"b", @"sort" : @3, @"v" : @1}, + @"doc4" : @{@"key" : @"b", @"sort" : @2, @"v" : @1}, + @"doc5" : @{@"key" : @"bb", @"sort" : @1, @"v" : @1}, + }]; + + FIRAggregateQuerySnapshot *snapshot = + [self readSnapshotForAggregate:[[[[collRef queryWhereField:@"key" isGreaterThan:@"a"] + queryWhereField:@"sort" + isGreaterThanOrEqualTo:@1] queryOrderedByField:@"v" + descending:NO] + aggregate:@[ + [FIRAggregateField aggregateFieldForCount], + [FIRAggregateField aggregateFieldForSumOfField:@"sort"], + [FIRAggregateField aggregateFieldForAverageOfField:@"v"] + ]]]; + XCTAssertEqual([snapshot count], [NSNumber numberWithLong:4L]); + + snapshot = [self + readSnapshotForAggregate:[[[[collRef queryWhereField:@"key" isGreaterThan:@"a"] + queryWhereField:@"sort" + isGreaterThanOrEqualTo:@1] queryWhereField:@"v" isNotEqualTo:@0] + aggregate:@[ + [FIRAggregateField aggregateFieldForCount], + [FIRAggregateField aggregateFieldForSumOfField:@"sort"], + [FIRAggregateField aggregateFieldForAverageOfField:@"v"], + ]]]; + XCTAssertEqual([snapshot valueForAggregation:[FIRAggregateField aggregateFieldForCount]], + [NSNumber numberWithLong:3L]); + XCTAssertEqual( + [[snapshot valueForAggregation:[FIRAggregateField aggregateFieldForSumOfField:@"sort"]] + longValue], + 6L); + XCTAssertEqual( + [snapshot valueForAggregation:[FIRAggregateField aggregateFieldForAverageOfField:@"v"]], + [NSNumber numberWithDouble:1.0]); +} + +- (void)testMultipleInequalityFieldsWithDocumentKey { + // TODO(MIEQ): Enable this test against production when possible. + XCTSkipIf(![FSTIntegrationTestCase isRunningAgainstEmulator], + "Skip this test if running against production because multiple inequality is " + "not supported yet."); + + FIRCollectionReference *collRef = [self collectionRefWithDocuments:@{ + @"doc1" : @{@"key" : @"a", @"sort" : @5}, + @"doc2" : @{@"key" : @"aa", @"sort" : @4}, + @"doc3" : @{@"key" : @"b", @"sort" : @3}, + @"doc4" : @{@"key" : @"b", @"sort" : @2}, + @"doc5" : @{@"key" : @"bb", @"sort" : @1} + }]; + + FIRQuerySnapshot *snapshot = [self + readDocumentSetForRef:[[[collRef queryWhereField:@"sort" + isGreaterThan:@1] queryWhereField:@"key" isNotEqualTo:@"a"] + queryWhereFieldPath:[FIRFieldPath documentID] + isLessThan:@"doc5"]]; + // Document Key in inequality field will implicitly ordered to the last. + // Implicitly ordered by: 'key' asc, 'sort' asc, __name__ asc + XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(snapshot), (@[ @"doc2", @"doc4", @"doc3" ])); + + snapshot = [self readDocumentSetForRef:[[[collRef queryWhereFieldPath:[FIRFieldPath documentID] + isLessThan:@"doc5"] + queryWhereField:@"sort" + isGreaterThan:@1] queryWhereField:@"key" + isNotEqualTo:@"a"]]; + // Changing filters order will not effect implicit order. + // Implicitly ordered by: 'key' asc, 'sort' asc, __name__ asc + XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(snapshot), (@[ @"doc2", @"doc4", @"doc3" ])); + + snapshot = [self + readDocumentSetForRef:[[[[collRef queryWhereFieldPath:[FIRFieldPath documentID] + isLessThan:@"doc5"] queryWhereField:@"sort" + isGreaterThan:@1] + queryWhereField:@"key" + isNotEqualTo:@"a"] queryOrderedByField:@"sort" descending:YES]]; + // Ordered by: 'sort' desc,'key' desc, __name__ desc + XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(snapshot), (@[ @"doc2", @"doc3", @"doc4" ])); +} + +- (void)testMultipleInequalityReadFromCacheWhenOffline { + // TODO(MIEQ): Enable this test against production when possible. + XCTSkipIf(![FSTIntegrationTestCase isRunningAgainstEmulator], + "Skip this test if running against production because multiple inequality is " + "not supported yet."); + + FIRCollectionReference *collRef = [self collectionRefWithDocuments:@{ + @"doc1" : @{@"key" : @"a", @"sort" : @1}, + @"doc2" : @{@"key" : @"aa", @"sort" : @4}, + @"doc3" : @{@"key" : @"b", @"sort" : @3}, + @"doc4" : @{@"key" : @"b", @"sort" : @2}, + }]; + + FIRQuery *query = [[collRef queryWhereField:@"key" isNotEqualTo:@"a"] queryWhereField:@"sort" + isLessThanOrEqualTo:@3]; + + // populate the cache. + FIRQuerySnapshot *snapshot = [self readDocumentSetForRef:query]; + XCTAssertEqual(snapshot.count, 2L); + XCTAssertEqual(snapshot.metadata.isFromCache, NO); + + [self disableNetwork]; + + snapshot = [self readDocumentSetForRef:query]; + XCTAssertEqual(snapshot.count, 2L); + XCTAssertEqual(snapshot.metadata.isFromCache, YES); + // Implicitly ordered by: 'key' asc, 'sort' asc, __name__ asc + XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(snapshot), (@[ @"doc4", @"doc3" ])); +} + +- (void)testMultipleInequalityFromCacheAndFromServer { + // TODO(MIEQ): Enable this test against production when possible. + XCTSkipIf(![FSTIntegrationTestCase isRunningAgainstEmulator], + "Skip this test if running against production because multiple inequality is " + "not supported yet."); + + FIRCollectionReference *collRef = [self collectionRefWithDocuments:@{ + @"doc1" : @{@"a" : @1, @"b" : @0}, + @"doc2" : @{@"a" : @2, @"b" : @1}, + @"doc3" : @{@"a" : @3, @"b" : @2}, + @"doc4" : @{@"a" : @1, @"b" : @3}, + @"doc5" : @{@"a" : @1, @"b" : @1}, + + }]; + + // implicit AND: a != 1 && b < 2 + FIRQuery *query = [[collRef queryWhereField:@"a" isNotEqualTo:@1] queryWhereField:@"b" + isLessThan:@2]; + [self checkOnlineAndOfflineQuery:query matchesResult:@[ @"doc2" ]]; + + // explicit AND: a != 1 && b < 2 + FIRFilter *filter = [FIRFilter andFilterWithFilters:@[ + [FIRFilter filterWhereField:@"a" isNotEqualTo:@1], [FIRFilter filterWhereField:@"b" + isLessThan:@2] + ]]; + [self checkOnlineAndOfflineQuery:[collRef queryWhereFilter:filter] matchesResult:@[ @"doc2" ]]; + + // explicit AND: a < 3 && b not-in [2, 3] + // Implicitly ordered by: a asc, b asc, __name__ asc + filter = [FIRFilter andFilterWithFilters:@[ + [FIRFilter filterWhereField:@"a" isLessThan:@3], [FIRFilter filterWhereField:@"b" + notIn:@[ @2, @3 ]] + ]]; + [self checkOnlineAndOfflineQuery:[collRef queryWhereFilter:filter] + matchesResult:@[ @"doc1", @"doc5", @"doc2" ]]; + + // a <3 && b != 0, ordered by: b desc, a desc, __name__ desc + query = [[[[collRef queryWhereField:@"a" isLessThan:@3] queryWhereField:@"b" isNotEqualTo:@0] + queryOrderedByField:@"b" + descending:YES] queryLimitedTo:2]; + [self checkOnlineAndOfflineQuery:query matchesResult:@[ @"doc4", @"doc2" ]]; + + // explicit OR: a>2 || b<1. + filter = [FIRFilter orFilterWithFilters:@[ + [FIRFilter filterWhereField:@"a" isGreaterThan:@2], [FIRFilter filterWhereField:@"b" + isLessThan:@1] + ]]; + [self checkOnlineAndOfflineQuery:[collRef queryWhereFilter:filter] + matchesResult:@[ @"doc1", @"doc3" ]]; +} + - (void)testResumingAQueryShouldUseBloomFilterToAvoidFullRequery { // TODO(b/291365820): Stop skipping this test when running against the Firestore emulator once // the emulator is improved to include a bloom filter in the existence filter messages that it diff --git a/Firestore/Example/Tests/Integration/API/FIRValidationTests.mm b/Firestore/Example/Tests/Integration/API/FIRValidationTests.mm index 327f7e82b2e..acc32c66be2 100644 --- a/Firestore/Example/Tests/Integration/API/FIRValidationTests.mm +++ b/Firestore/Example/Tests/Integration/API/FIRValidationTests.mm @@ -626,111 +626,72 @@ - (void)testQueriesUsingInAndDocumentIdMustHaveProperDocumentReferencesInArray { - (void)testInvalidQueryFilters { FIRCollectionReference *collection = [self collectionRef]; - - // Multiple inequalities, one of which is inside a nested composite filter. - NSString *reason = @"Invalid Query. All where filters with an inequality (notEqual, lessThan, " - "lessThanOrEqual, greaterThan, or greaterThanOrEqual) must be on the same " - "field. But you have inequality filters on 'c' and 'r'"; - + NSString *reason = @"Invalid Query. You cannot use 'notIn' filters with 'in' filters."; NSArray *array1 = @[ [FIRFilter andFilterWithFilters:@[ [FIRFilter filterWhereField:@"a" isEqualTo:@"b"], [FIRFilter filterWhereField:@"c" - isGreaterThan:@"d"] + in:@[ @"d", @"e" ]] ]], [FIRFilter andFilterWithFilters:@[ [FIRFilter filterWhereField:@"e" isEqualTo:@"f"], [FIRFilter filterWhereField:@"g" - isEqualTo:@"h"] + notIn:@[ @"h", @"i" ]] ]] ]; + FSTAssertThrows([collection queryWhereFilter:[FIRFilter orFilterWithFilters:array1]], reason); - FSTAssertThrows( - [[collection queryWhereFilter:[FIRFilter orFilterWithFilters:array1]] queryWhereField:@"r" - isGreaterThan:@"s"], - reason); - - // OrderBy and inequality on different fields. Inequality inside a nested composite filter. - reason = @"Invalid query. You have a where filter with an inequality (notEqual, lessThan, " - "lessThanOrEqual, greaterThan, or greaterThanOrEqual) on field 'c' and so you must " - "also use 'c' as your first queryOrderedBy field, but your first queryOrderedBy is " - "currently on field 'r' instead."; - - FSTAssertThrows([[collection queryWhereFilter:[FIRFilter orFilterWithFilters:array1]] - queryOrderedByField:@"r"], - reason); - - // Conflicting operations within a composite filter. - reason = @"Invalid Query. You cannot use 'notIn' filters with 'in' filters."; - + reason = @"Invalid Query. You cannot use 'notIn' filters with 'notEqual' filters."; NSArray *array2 = @[ [FIRFilter andFilterWithFilters:@[ [FIRFilter filterWhereField:@"a" isEqualTo:@"b"], [FIRFilter filterWhereField:@"c" - in:@[ @"d", @"e" ]] + isNotEqualTo:@"d"] ]], [FIRFilter andFilterWithFilters:@[ - [FIRFilter filterWhereField:@"e" isEqualTo:@"f"], [FIRFilter filterWhereField:@"c" - notIn:@[ @"f", @"g" ]] + [FIRFilter filterWhereField:@"e" isEqualTo:@"f"], [FIRFilter filterWhereField:@"g" + notIn:@[ @"h", @"i" ]] ]] ]; - FSTAssertThrows([collection queryWhereFilter:[FIRFilter orFilterWithFilters:array2]], reason); - // Conflicting operations between a field filter and a composite filter. + reason = @"Invalid Query. You cannot use more than one 'notIn' filter."; NSArray *array3 = @[ [FIRFilter andFilterWithFilters:@[ [FIRFilter filterWhereField:@"a" isEqualTo:@"b"], [FIRFilter filterWhereField:@"c" - in:@[ @"d", @"e" ]] + notIn:@[ @"d", @"e" ]] ]], [FIRFilter andFilterWithFilters:@[ [FIRFilter filterWhereField:@"e" isEqualTo:@"f"], [FIRFilter filterWhereField:@"g" - isEqualTo:@"h"] + notIn:@[ @"h", @"i" ]] ]] ]; + FSTAssertThrows([collection queryWhereFilter:[FIRFilter orFilterWithFilters:array3]], reason); - NSArray *array4 = @[ @"j", @"k" ]; - - FSTAssertThrows( - [[collection queryWhereFilter:[FIRFilter orFilterWithFilters:array3]] queryWhereField:@"i" - notIn:array4], - reason); - - // Conflicting operations between two composite filters. - NSArray *array5 = @[ + reason = @"Invalid Query. You cannot use more than one 'notEqual' filter."; + NSArray *array4 = @[ [FIRFilter andFilterWithFilters:@[ - [FIRFilter filterWhereField:@"i" isEqualTo:@"j"], [FIRFilter filterWhereField:@"l" - notIn:@[ @"m", @"n" ]] + [FIRFilter filterWhereField:@"a" isEqualTo:@"b"], [FIRFilter filterWhereField:@"c" + isNotEqualTo:@"d"] ]], [FIRFilter andFilterWithFilters:@[ - [FIRFilter filterWhereField:@"o" isEqualTo:@"p"], [FIRFilter filterWhereField:@"q" - isEqualTo:@"r"] + [FIRFilter filterWhereField:@"e" isEqualTo:@"f"], [FIRFilter filterWhereField:@"g" + isNotEqualTo:@"h"] ]] ]; - - FSTAssertThrows([[collection queryWhereFilter:[FIRFilter orFilterWithFilters:array3]] - queryWhereFilter:[FIRFilter orFilterWithFilters:array5]], - reason); + FSTAssertThrows([collection queryWhereFilter:[FIRFilter orFilterWithFilters:array4]], reason); } -- (void)testQueryInequalityFieldMustMatchFirstOrderByField { +- (void)testQueryInequalityFieldWithMultipleOrderByFields { FIRCollectionReference *coll = [self.db collectionWithPath:@"collection"]; FIRQuery *base = [coll queryWhereField:@"x" isGreaterThanOrEqualTo:@32]; - FSTAssertThrows([base queryWhereField:@"y" isLessThan:@"cat"], - @"Invalid Query. All where filters with an inequality (notEqual, lessThan, " - "lessThanOrEqual, greaterThan, or greaterThanOrEqual) must be on the same " - "field. But you have inequality filters on 'x' and 'y'"); + XCTAssertNoThrow([base queryWhereField:@"y" isLessThan:@"cat"]); - NSString *reason = - @"Invalid query. You have a where filter with " - "an inequality (notEqual, lessThan, lessThanOrEqual, greaterThan, or greaterThanOrEqual) " - "on field 'x' and so you must also use 'x' as your first queryOrderedBy field, " - "but your first queryOrderedBy is currently on field 'y' instead."; - FSTAssertThrows([base queryOrderedByField:@"y"], reason); - FSTAssertThrows([[coll queryOrderedByField:@"y"] queryWhereField:@"x" isGreaterThan:@32], reason); - FSTAssertThrows([[base queryOrderedByField:@"y"] queryOrderedByField:@"x"], reason); - FSTAssertThrows([[[coll queryOrderedByField:@"y"] queryOrderedByField:@"x"] queryWhereField:@"x" - isGreaterThan:@32], - reason); - FSTAssertThrows([[coll queryOrderedByField:@"y"] queryWhereField:@"x" isNotEqualTo:@32], reason); + XCTAssertNoThrow([base queryOrderedByField:@"y"]); + XCTAssertNoThrow([[coll queryOrderedByField:@"y"] queryWhereField:@"x" isGreaterThan:@32]); + XCTAssertNoThrow([[base queryOrderedByField:@"y"] queryOrderedByField:@"x"]); + XCTAssertNoThrow([[[coll queryOrderedByField:@"y"] queryOrderedByField:@"x"] + queryWhereField:@"x" + isGreaterThan:@32]); + XCTAssertNoThrow([[coll queryOrderedByField:@"y"] queryWhereField:@"x" isNotEqualTo:@32]); XCTAssertNoThrow([base queryWhereField:@"x" isLessThanOrEqualTo:@"cat"], @"Same inequality fields work"); @@ -758,20 +719,6 @@ - (void)testQueryInequalityFieldMustMatchFirstOrderByField { @"array_contains different than orderBy works."); } -- (void)testQueriesWithMultipleNotEqualAndInequalitiesFail { - FIRCollectionReference *coll = [self.db collectionWithPath:@"collection"]; - - FSTAssertThrows([[coll queryWhereField:@"x" isNotEqualTo:@1] queryWhereField:@"x" - isNotEqualTo:@2], - @"Invalid Query. You cannot use more than one 'notEqual' filter."); - - FSTAssertThrows([[coll queryWhereField:@"x" isNotEqualTo:@1] queryWhereField:@"y" - isNotEqualTo:@2], - @"Invalid Query. All where filters with an inequality (notEqual, lessThan, " - "lessThanOrEqual, greaterThan, or greaterThanOrEqual) must be on " - "the same field. But you have inequality filters on 'x' and 'y'"); -} - - (void)testQueriesWithNotEqualAndNotInFiltersFail { FIRCollectionReference *coll = [self.db collectionWithPath:@"collection"]; diff --git a/Firestore/core/src/api/query_core.cc b/Firestore/core/src/api/query_core.cc index a7e9d52f9f0..f66676dffe3 100644 --- a/Firestore/core/src/api/query_core.cc +++ b/Firestore/core/src/api/query_core.cc @@ -277,7 +277,6 @@ Query Query::OrderBy(FieldPath field_path, bool descending) const { } Query Query::OrderBy(FieldPath field_path, Direction direction) const { - ValidateNewOrderByPath(field_path); if (query_.start_at()) { ThrowInvalidArgument( "Invalid query. You must not specify a starting point " @@ -320,26 +319,6 @@ Query Query::EndAt(Bound bound) const { void Query::ValidateNewFieldFilter(const core::Query& query, const FieldFilter& field_filter) const { - if (field_filter.IsInequality()) { - const FieldPath* existing_inequality = query.InequalityFilterField(); - const FieldPath& new_inequality = field_filter.field(); - - if (existing_inequality && *existing_inequality != new_inequality) { - ThrowInvalidArgument( - "Invalid Query. All where filters with an inequality (notEqual, " - "lessThan, lessThanOrEqual, greaterThan, or greaterThanOrEqual) " - "must be on the same field. But you have inequality filters on " - "'%s' and '%s'", - existing_inequality->CanonicalString(), - new_inequality.CanonicalString()); - } - - const FieldPath* first_order_by_field = query.FirstOrderByField(); - if (first_order_by_field) { - ValidateOrderByField(*first_order_by_field, field_filter.field()); - } - } - Operator filter_op = field_filter.op(); absl::optional conflicting_op = query.FindOpInsideFilters(ConflictingOps(filter_op)); @@ -367,30 +346,6 @@ void Query::ValidateNewFilter(const Filter& filter) const { } } -void Query::ValidateNewOrderByPath(const FieldPath& field_path) const { - if (!query_.FirstOrderByField()) { - // This is the first order by. It must match any inequality. - const FieldPath* inequality_field = query_.InequalityFilterField(); - if (inequality_field) { - ValidateOrderByField(field_path, *inequality_field); - } - } -} - -void Query::ValidateOrderByField(const FieldPath& order_by_field, - const FieldPath& inequality_field) const { - if (order_by_field != inequality_field) { - ThrowInvalidArgument( - "Invalid query. You have a where filter with an inequality " - "(notEqual, lessThan, lessThanOrEqual, greaterThan, or " - "greaterThanOrEqual) on field '%s' and so you must also use '%s' as " - "your first queryOrderedBy field, but your first queryOrderedBy is " - "currently on field '%s' instead.", - inequality_field.CanonicalString(), inequality_field.CanonicalString(), - order_by_field.CanonicalString()); - } -} - void Query::ValidateHasExplicitOrderByForLimitToLast() const { if (query_.has_limit_to_last() && query_.explicit_order_bys().empty()) { ThrowInvalidArgument( diff --git a/Firestore/core/src/api/query_core.h b/Firestore/core/src/api/query_core.h index b85238a7eea..f6588cc7927 100644 --- a/Firestore/core/src/api/query_core.h +++ b/Firestore/core/src/api/query_core.h @@ -215,9 +215,6 @@ class Query { void ValidateNewFilter(const core::Filter& filter) const; void ValidateNewFieldFilter(const core::Query& query, const core::FieldFilter& filter) const; - void ValidateNewOrderByPath(const model::FieldPath& field_path) const; - void ValidateOrderByField(const model::FieldPath& order_by_field, - const model::FieldPath& inequality_field) const; void ValidateHasExplicitOrderByForLimitToLast() const; /** * Validates that the value passed into a disjunctive filter satisfies all diff --git a/Firestore/core/src/core/composite_filter.cc b/Firestore/core/src/core/composite_filter.cc index 5c917d4e2a1..7bbb81abc57 100644 --- a/Firestore/core/src/core/composite_filter.cc +++ b/Firestore/core/src/core/composite_filter.cc @@ -141,17 +141,6 @@ const FieldFilter* CompositeFilter::Rep::FindFirstMatchingFilter( return nullptr; } -const model::FieldPath* CompositeFilter::Rep::GetFirstInequalityField() const { - CheckFunction condition = [](const FieldFilter& field_filter) { - return field_filter.IsInequality(); - }; - const FieldFilter* found = FindFirstMatchingFilter(condition); - if (found) { - return &(found->field()); - } - return nullptr; -} - const std::vector& CompositeFilter::Rep::GetFlattenedFilters() const { return memoized_flattened_filters_->memoize([&]() { diff --git a/Firestore/core/src/core/composite_filter.h b/Firestore/core/src/core/composite_filter.h index d16975fdac2..24671d3e44e 100644 --- a/Firestore/core/src/core/composite_filter.h +++ b/Firestore/core/src/core/composite_filter.h @@ -140,8 +140,6 @@ class CompositeFilter : public Filter { const std::vector& GetFlattenedFilters() const override; - const model::FieldPath* GetFirstInequalityField() const override; - std::vector GetFilters() const override { return filters(); } diff --git a/Firestore/core/src/core/field_filter.cc b/Firestore/core/src/core/field_filter.cc index eb882e0db77..b68956c8a52 100644 --- a/Firestore/core/src/core/field_filter.cc +++ b/Firestore/core/src/core/field_filter.cc @@ -202,13 +202,6 @@ bool FieldFilter::Rep::Equals(const Filter::Rep& other) const { *value_rhs_ == *other_rep.value_rhs_; } -const model::FieldPath* FieldFilter::Rep::GetFirstInequalityField() const { - if (IsInequality()) { - return &field(); - } - return nullptr; -} - } // namespace core } // namespace firestore } // namespace firebase diff --git a/Firestore/core/src/core/field_filter.h b/Firestore/core/src/core/field_filter.h index a0fb692e44d..48219f222f2 100644 --- a/Firestore/core/src/core/field_filter.h +++ b/Firestore/core/src/core/field_filter.h @@ -117,8 +117,6 @@ class FieldFilter : public Filter { return false; } - const model::FieldPath* GetFirstInequalityField() const override; - const std::vector& GetFlattenedFilters() const override; std::vector GetFilters() const override; diff --git a/Firestore/core/src/core/filter.h b/Firestore/core/src/core/filter.h index 1da4888f826..ec20120daf6 100644 --- a/Firestore/core/src/core/filter.h +++ b/Firestore/core/src/core/filter.h @@ -95,14 +95,6 @@ class Filter { return rep_->IsEmpty(); } - /** - * Returns the first inequality filter contained within this filter. - * Returns nullptr if it does not contain any inequalities. - */ - const model::FieldPath* GetFirstInequalityField() const { - return rep_->GetFirstInequalityField(); - } - /** * Returns a list of all field filters that are contained within this filter. */ @@ -155,8 +147,6 @@ class Filter { virtual bool IsEmpty() const = 0; - virtual const model::FieldPath* GetFirstInequalityField() const = 0; - virtual const std::vector& GetFlattenedFilters() const = 0; virtual std::vector GetFilters() const = 0; diff --git a/Firestore/core/src/core/query.cc b/Firestore/core/src/core/query.cc index eb089ff2359..5b70ebc322f 100644 --- a/Firestore/core/src/core/query.cc +++ b/Firestore/core/src/core/query.cc @@ -67,14 +67,16 @@ bool Query::MatchesAllDocuments() const { explicit_order_bys_.front().field().IsKeyFieldPath())); } -const FieldPath* Query::InequalityFilterField() const { +const std::set Query::InequalityFilterFields() const { + std::set result; for (const Filter& filter : filters_) { - const FieldPath* found = filter.GetFirstInequalityField(); - if (found) { - return found; + for (const FieldFilter& subFilter : filter.GetFlattenedFilters()) { + if (subFilter.IsInequality()) { + result.emplace(subFilter.field()); + } } } - return nullptr; + return result; } absl::optional Query::FindOpInsideFilters( @@ -91,62 +93,44 @@ absl::optional Query::FindOpInsideFilters( const std::vector& Query::normalized_order_bys() const { return memoized_normalized_order_bys_->memoize([&]() { - std::vector result; - const FieldPath* inequality_field = InequalityFilterField(); - const FieldPath* first_order_by_field = FirstOrderByField(); - - if (inequality_field && !first_order_by_field) { - // In order to implicitly add key ordering, we must also add the - // inequality filter field for it to be a valid query. Note that the - // default inequality field and key ordering is ascending. - if (inequality_field->IsKeyFieldPath()) { - result = { - OrderBy(FieldPath::KeyFieldPath(), Direction::Ascending), - }; - } else { - result = { - OrderBy(*inequality_field, Direction::Ascending), - OrderBy(FieldPath::KeyFieldPath(), Direction::Ascending), - }; - } - } else { - HARD_ASSERT( - !inequality_field || *inequality_field == *first_order_by_field, - "First orderBy %s should match inequality field %s.", - first_order_by_field->CanonicalString(), - inequality_field->CanonicalString()); - - result = explicit_order_bys_; - bool found_explicit_key_order = false; - for (const OrderBy& order_by : explicit_order_bys_) { - if (order_by.field().IsKeyFieldPath()) { - found_explicit_key_order = true; - break; - } - } + // Any explicit order by fields should be added as is. + std::vector result = explicit_order_bys_; + std::set fieldsNormalized; + for (const OrderBy& order_by : explicit_order_bys_) { + fieldsNormalized.insert(order_by.field()); + } - if (!found_explicit_key_order) { - // The direction of the implicit key ordering always matches the - // direction of the last explicit sort order - Direction last_direction = explicit_order_bys_.empty() - ? Direction::Ascending - : explicit_order_bys_.back().direction(); - result.emplace_back(FieldPath::KeyFieldPath(), last_direction); + // The order of the implicit ordering always matches the last explicit order + // by. + Direction last_direction = explicit_order_bys_.empty() + ? Direction::Ascending + : explicit_order_bys_.back().direction(); + + // Any inequality fields not explicitly ordered should be implicitly ordered + // in a lexicographical order. When there are multiple inequality filters on + // the same field, the field should be added only once. Note: + // `std::set` sorts the key field before other fields. + // However, we want the key field to be sorted last. + const std::set inequality_fields = + InequalityFilterFields(); + + for (const model::FieldPath& field : inequality_fields) { + if (fieldsNormalized.find(field) == fieldsNormalized.end() && + !field.IsKeyFieldPath()) { + result.push_back(OrderBy(field, last_direction)); } } + // Add the document key field to the last if it is not explicitly ordered. + if (fieldsNormalized.find(FieldPath::KeyFieldPath()) == + fieldsNormalized.end()) { + result.push_back(OrderBy(FieldPath::KeyFieldPath(), last_direction)); + } + return result; }); } -const FieldPath* Query::FirstOrderByField() const { - if (explicit_order_bys_.empty()) { - return nullptr; - } - - return &explicit_order_bys_.front().field(); -} - LimitType Query::limit_type() const { return limit_type_; } @@ -162,16 +146,6 @@ int32_t Query::limit() const { Query Query::AddingFilter(Filter filter) const { HARD_ASSERT(!IsDocumentQuery(), "No filter is allowed for document query"); - const FieldPath* new_inequality_field = filter.GetFirstInequalityField(); - const FieldPath* query_inequality_field = InequalityFilterField(); - HARD_ASSERT(!query_inequality_field || !new_inequality_field || - *query_inequality_field == *new_inequality_field, - "Query must only have one inequality field."); - - HARD_ASSERT(explicit_order_bys_.empty() || !new_inequality_field || - explicit_order_bys_[0].field() == *new_inequality_field, - "First orderBy must match inequality field"); - std::vector filters_copy(filters_); filters_copy.push_back(std::move(filter)); @@ -188,12 +162,6 @@ Query Query::AddingFilter(Filter filter) const { Query Query::AddingOrderBy(OrderBy order_by) const { HARD_ASSERT(!IsDocumentQuery(), "No ordering is allowed for document query"); - if (explicit_order_bys_.empty()) { - const FieldPath* inequality = InequalityFilterField(); - HARD_ASSERT(inequality == nullptr || *inequality == order_by.field(), - "First OrderBy must match inequality field."); - } - std::vector order_bys_copy(explicit_order_bys_); order_bys_copy.push_back(std::move(order_by)); diff --git a/Firestore/core/src/core/query.h b/Firestore/core/src/core/query.h index 047f2e24bbb..23351a4de56 100644 --- a/Firestore/core/src/core/query.h +++ b/Firestore/core/src/core/query.h @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -118,6 +119,11 @@ class Query { */ const model::FieldPath* InequalityFilterField() const; + /** + * Returns the sorted set of inequality filter fields used in this query. + */ + const std::set InequalityFilterFields() const; + /** * Checks if any of the provided filter operators are included in the query * and returns the first one that is, or null if none are. @@ -144,9 +150,6 @@ class Query { */ const std::vector& normalized_order_bys() const; - /** Returns the first field in an order-by constraint, or nullptr if none. */ - const model::FieldPath* FirstOrderByField() const; - bool has_limit() const { return limit_ != Target::kNoLimit; } diff --git a/Firestore/core/src/local/leveldb_index_manager.cc b/Firestore/core/src/local/leveldb_index_manager.cc index ca8412744d0..eb41c79ad0b 100644 --- a/Firestore/core/src/local/leveldb_index_manager.cc +++ b/Firestore/core/src/local/leveldb_index_manager.cc @@ -480,7 +480,10 @@ void LevelDbIndexManager::CreateTargetIndexes(const core::Target& target) { if (type == IndexManager::IndexType::NONE || type == IndexManager::IndexType::PARTIAL) { TargetIndexMatcher targetIndexMatcher(subTarget); - AddFieldIndex(targetIndexMatcher.BuildTargetIndex()); + auto const field_index = targetIndexMatcher.BuildTargetIndex(); + if (field_index.has_value()) { + AddFieldIndex(field_index.value()); + } } } } diff --git a/Firestore/core/src/model/target_index_matcher.cc b/Firestore/core/src/model/target_index_matcher.cc index b26114f0f79..8a4d3aac851 100644 --- a/Firestore/core/src/model/target_index_matcher.cc +++ b/Firestore/core/src/model/target_index_matcher.cc @@ -36,15 +36,11 @@ TargetIndexMatcher::TargetIndexMatcher(const core::Target& target) { ? *target.collection_group() : target.path().last_segment(); order_bys_ = target.order_bys(); - inequality_filter_ = absl::nullopt; for (const Filter& filter : target.filters()) { FieldFilter field_filter(filter); if (field_filter.IsInequality()) { - HARD_ASSERT(!inequality_filter_.has_value() || - inequality_filter_->field() == field_filter.field(), - "Only a single inequality is supported"); - inequality_filter_ = field_filter; + inequality_filters_.insert(field_filter); } else { equality_filters_.push_back(field_filter); } @@ -55,6 +51,12 @@ bool TargetIndexMatcher::ServedByIndex(const model::FieldIndex& index) const { HARD_ASSERT(index.collection_group() == collection_id_, "Collection IDs do not match"); + if (HasMultipleInequality()) { + // Only single inequality is supported for now. + // TODO(Add support for multiple inequality query): b/298441043 + return false; + } + // If there is an array element, find a matching filter. const auto& array_segment = index.GetArraySegment(); if (array_segment.has_value() && @@ -90,13 +92,17 @@ bool TargetIndexMatcher::ServedByIndex(const model::FieldIndex& index) const { // `order_bys_` has at least one element. auto order_by_iter = order_bys_.begin(); - if (inequality_filter_.has_value()) { + if (!inequality_filters_.empty()) { + // Only a single inequality is currently supported. Get the only entry in + // the set. + const FieldFilter& inequality_filter = *inequality_filters_.begin(); + // If there is an inequality filter and the field was not in one of the // equality filters above, the next segment must match both the filter // and the first orderBy clause. - if (equality_segments.count( - inequality_filter_.value().field().CanonicalString()) == 0) { - if (!MatchesFilter(inequality_filter_, segments[segment_index]) || + if (equality_segments.count(inequality_filter.field().CanonicalString()) == + 0) { + if (!MatchesFilter(inequality_filter, segments[segment_index]) || !MatchesOrderBy(*(order_by_iter++), segments[segment_index])) { return false; } @@ -118,7 +124,11 @@ bool TargetIndexMatcher::ServedByIndex(const model::FieldIndex& index) const { return true; } -model::FieldIndex TargetIndexMatcher::BuildTargetIndex() const { +absl::optional TargetIndexMatcher::BuildTargetIndex() { + if (HasMultipleInequality()) { + return {}; + } + // We want to make sure only one segment created for one field. For example, // in case like a == 3 and a > 2, Index: {a ASCENDING} will only be created // once. diff --git a/Firestore/core/src/model/target_index_matcher.h b/Firestore/core/src/model/target_index_matcher.h index 7afedb43af2..d6c964ca1de 100644 --- a/Firestore/core/src/model/target_index_matcher.h +++ b/Firestore/core/src/model/target_index_matcher.h @@ -17,6 +17,7 @@ #ifndef FIRESTORE_CORE_SRC_MODEL_TARGET_INDEX_MATCHER_H_ #define FIRESTORE_CORE_SRC_MODEL_TARGET_INDEX_MATCHER_H_ +#include #include #include @@ -54,6 +55,10 @@ class TargetIndexMatcher { public: explicit TargetIndexMatcher(const core::Target& target); + inline bool HasMultipleInequality() const { + return inequality_filters_.size() > 1U; + } + /** * Returns whether the index can be used to serve the TargetIndexMatcher's * target. @@ -77,8 +82,11 @@ class TargetIndexMatcher { */ bool ServedByIndex(const model::FieldIndex& index) const; - /** Returns a full matched field index for this target. */ - model::FieldIndex BuildTargetIndex() const; + /** + * Returns a full matched field index for this target. Currently multiple + * inequality query is not supported so function returns null. + */ + absl::optional BuildTargetIndex(); private: bool HasMatchingEqualityFilter(const model::Segment& segment) const; @@ -91,10 +99,21 @@ class TargetIndexMatcher { bool MatchesOrderBy(const core::OrderBy& order_by, const model::Segment& segment) const; + // Custom comparator for FieldFilter based on its Field property. + struct FieldFilterComparator { + bool operator()(const core::FieldFilter& filter1, + const core::FieldFilter& filter2) const { + return filter1.field() < filter2.field(); + } + }; + // The collection ID (or collection group) of the query target. std::string collection_id_; - absl::optional inequality_filter_; + // The inequality filters of the target (if it exists). + // Note: The sort on FieldFilters is not required. We are using comparator to + // differentiate inequality Filters based on its field path only. + std::set inequality_filters_; std::vector equality_filters_; std::vector order_bys_; }; diff --git a/Firestore/core/test/unit/core/query_test.cc b/Firestore/core/test/unit/core/query_test.cc index aaf0dabc431..c71590a8f33 100644 --- a/Firestore/core/test/unit/core/query_test.cc +++ b/Firestore/core/test/unit/core/query_test.cc @@ -836,6 +836,127 @@ TEST(QueryTest, ImplicitOrderBy) { })); } +TEST(QueryTest, ImplicitOrderByInMultipleInequality) { + auto base_query = testutil::Query("foo"); + ASSERT_EQ(base_query.AddingFilter(testutil::Filter("a", "<", 5)) + .AddingFilter(testutil::Filter("a", ">=", 5)) + .AddingFilter(testutil::Filter("aa", ">", 5)) + .AddingFilter(testutil::Filter("b", "<=", 5)) + .AddingFilter(testutil::Filter("A", ">=", 5)) + .normalized_order_bys(), + (std::vector{ + testutil::OrderBy("A", "asc"), + testutil::OrderBy("a", "asc"), + testutil::OrderBy("aa", "asc"), + testutil::OrderBy("b", "asc"), + testutil::OrderBy(FieldPath::kDocumentKeyPath, "asc"), + })); + + // numbers + ASSERT_EQ(base_query.AddingFilter(testutil::Filter("a", "<", 5)) + .AddingFilter(testutil::Filter("1", ">", 5)) + .AddingFilter(testutil::Filter("19", "<=", 5)) + .AddingFilter(testutil::Filter("2", ">=", 5)) + .normalized_order_bys(), + (std::vector{ + testutil::OrderBy("1", "asc"), + testutil::OrderBy("19", "asc"), + testutil::OrderBy("2", "asc"), + testutil::OrderBy("a", "asc"), + testutil::OrderBy(FieldPath::kDocumentKeyPath, "asc"), + })); + + // nested fields + ASSERT_EQ(base_query.AddingFilter(testutil::Filter("a", "<", 5)) + .AddingFilter(testutil::Filter("aa", ">", 5)) + .AddingFilter(testutil::Filter("a.a", "<=", 5)) + .normalized_order_bys(), + (std::vector{ + testutil::OrderBy("a", "asc"), + testutil::OrderBy("a.a", "asc"), + testutil::OrderBy("aa", "asc"), + testutil::OrderBy(FieldPath::kDocumentKeyPath, "asc"), + })); + + // special characters + ASSERT_EQ(base_query.AddingFilter(testutil::Filter("a", "<", 5)) + .AddingFilter(testutil::Filter("_a", ">", 5)) + .AddingFilter(testutil::Filter("a.a", "<=", 5)) + .normalized_order_bys(), + (std::vector{ + testutil::OrderBy("_a", "asc"), + testutil::OrderBy("a", "asc"), + testutil::OrderBy("a.a", "asc"), + testutil::OrderBy(FieldPath::kDocumentKeyPath, "asc"), + })); + + // field name with dot + ASSERT_EQ(base_query.AddingFilter(testutil::Filter("a", "<", 5)) + .AddingFilter(testutil::Filter("a.z", ">", 5)) + .AddingFilter(testutil::Filter(("`a.a`"), "<=", 5)) + .normalized_order_bys(), + (std::vector{ + testutil::OrderBy("a", "asc"), + testutil::OrderBy("a.z", "asc"), + testutil::OrderBy(("`a.a`"), "asc"), + testutil::OrderBy(FieldPath::kDocumentKeyPath, "asc"), + })); + + // composite filter + ASSERT_EQ(base_query.AddingFilter(testutil::Filter("a", "<", 5)) + .AddingFilter( + AndFilters({OrFilters({testutil::Filter("b", ">=", 1), + testutil::Filter("c", "<=", 0)}), + OrFilters({testutil::Filter("d", ">", 3), + testutil::Filter("e", "==", 2)})})) + .normalized_order_bys(), + (std::vector{ + testutil::OrderBy("a", "asc"), + testutil::OrderBy("b", "asc"), + testutil::OrderBy("c", "asc"), + testutil::OrderBy("d", "asc"), + testutil::OrderBy(FieldPath::kDocumentKeyPath, "asc"), + })); + + // OrderBy + ASSERT_EQ(base_query.AddingFilter(testutil::Filter("b", "<", 5)) + .AddingFilter(testutil::Filter("a", ">", 5)) + .AddingFilter(testutil::Filter(("z"), "<=", 5)) + .AddingOrderBy(testutil::OrderBy("z")) + .normalized_order_bys(), + (std::vector{ + testutil::OrderBy("z", "asc"), + testutil::OrderBy("a", "asc"), + testutil::OrderBy("b", "asc"), + testutil::OrderBy(FieldPath::kDocumentKeyPath, "asc"), + })); + + // last explicit order by direction + ASSERT_EQ(base_query.AddingFilter(testutil::Filter("b", "<", 5)) + .AddingFilter(testutil::Filter("a", ">", 5)) + .AddingOrderBy(testutil::OrderBy("z", "desc")) + .normalized_order_bys(), + (std::vector{ + testutil::OrderBy("z", "desc"), + testutil::OrderBy("a", "desc"), + testutil::OrderBy("b", "desc"), + testutil::OrderBy(FieldPath::kDocumentKeyPath, "desc"), + })); + + ASSERT_EQ(base_query.AddingFilter(testutil::Filter("b", "<", 5)) + .AddingFilter(testutil::Filter("a", ">", 5)) + .AddingOrderBy(testutil::OrderBy("z", "desc")) + .AddingOrderBy(testutil::OrderBy("c")) + .normalized_order_bys(), + (std::vector{ + testutil::OrderBy("z", "desc"), + testutil::OrderBy("c", "asc"), + testutil::OrderBy("a", "asc"), + testutil::OrderBy("b", "asc"), + testutil::OrderBy(FieldPath::kDocumentKeyPath, "asc"), + })); +} + MATCHER_P(HasCanonicalId, expected, "") { const std::string& actual = arg.CanonicalId(); *result_listener << "which has CanonicalId " << actual; diff --git a/Firestore/core/test/unit/local/leveldb_local_store_test.cc b/Firestore/core/test/unit/local/leveldb_local_store_test.cc index fb233f12fd3..85e4286698b 100644 --- a/Firestore/core/test/unit/local/leveldb_local_store_test.cc +++ b/Firestore/core/test/unit/local/leveldb_local_store_test.cc @@ -764,6 +764,43 @@ TEST_F(LevelDbLocalStoreTest, IndexAutoCreationWorksWithMutation) { FSTAssertQueryReturned("coll/a", "coll/f"); } +TEST_F(LevelDbLocalStoreTest, + IndexAutoCreationDoesnotWorkWithMultipleInequality) { + core::Query query = testutil::Query("coll") + .AddingFilter(Filter("field1", "<", 5)) + .AddingFilter(Filter("field2", "<", 5)); + int target_id = AllocateQuery(query); + + SetIndexAutoCreationEnabled(true); + SetMinCollectionSizeToAutoCreateIndex(0); + SetRelativeIndexReadCostPerDocument(2); + + ApplyRemoteEvent(AddedRemoteEvent( + Doc("coll/a", 10, Map("field1", 1, "field2", 2)), {target_id})); + ApplyRemoteEvent(AddedRemoteEvent( + Doc("coll/b", 10, Map("field1", 8, "field2", 2)), {target_id})); + ApplyRemoteEvent(AddedRemoteEvent( + Doc("coll/c", 10, Map("field1", "string", "field2", 2)), {target_id})); + ApplyRemoteEvent( + AddedRemoteEvent(Doc("coll/d", 10, Map("field1", 2)), {target_id})); + ApplyRemoteEvent(AddedRemoteEvent( + Doc("coll/e", 10, Map("field1", 4, "field2", 4)), {target_id})); + + // First time query is running without indexes. + // Based on current heuristic, collection document counts (5) > 2 * resultSize + // (2). Full matched index will not be created since FieldIndex does not + // support multiple inequality. + ExecuteQuery(query); + FSTAssertRemoteDocumentsRead(/* byKey= */ 0, /* byCollection= */ 2); + FSTAssertQueryReturned("coll/a", "coll/e"); + + BackfillIndexes(); + + ExecuteQuery(query); + FSTAssertRemoteDocumentsRead(/* byKey= */ 0, /* byCollection= */ 2); + FSTAssertQueryReturned("coll/a", "coll/e"); +} + } // namespace local } // namespace firestore } // namespace firebase diff --git a/Firestore/core/test/unit/model/target_index_matcher_test.cc b/Firestore/core/test/unit/model/target_index_matcher_test.cc index f07b7bfc66d..f94644882e3 100644 --- a/Firestore/core/test/unit/model/target_index_matcher_test.cc +++ b/Firestore/core/test/unit/model/target_index_matcher_test.cc @@ -139,10 +139,13 @@ void ValidateDoesNotServeTarget(const core::Query& query, void ValidateBuildTargetIndexCreateFullMatchIndex(const core::Query& query) { const core::Target& target = query.ToTarget(); TargetIndexMatcher matcher(target); - FieldIndex expected_index = matcher.BuildTargetIndex(); - EXPECT_TRUE(matcher.ServedByIndex(expected_index)); + EXPECT_FALSE(matcher.HasMultipleInequality()); + absl::optional actual_index = matcher.BuildTargetIndex(); + ASSERT_TRUE(actual_index.has_value()); + EXPECT_TRUE(matcher.ServedByIndex(actual_index.value())); // Check the index created is a FULL MATCH index - EXPECT_TRUE(expected_index.segments().size() >= target.GetSegmentCount()); + EXPECT_TRUE(actual_index.value().segments().size() >= + target.GetSegmentCount()); } TEST(TargetIndexMatcher, CanUseMergeJoin) { @@ -728,6 +731,17 @@ TEST(TargetIndexMatcher, BuildTargetIndexWithInAndOrderBySameField) { ValidateBuildTargetIndexCreateFullMatchIndex(query); } +TEST(TargetIndexMatcher, BuildTargetIndexReturnsNullForMultipleInequality) { + auto query = testutil::Query("collId") + .AddingFilter(Filter("a", ">=", 1)) + .AddingFilter(Filter("b", "<=", 10)); + const core::Target& target = query.ToTarget(); + TargetIndexMatcher matcher(target); + EXPECT_TRUE(matcher.HasMultipleInequality()); + absl::optional actual_index = matcher.BuildTargetIndex(); + EXPECT_FALSE(actual_index.has_value()); +} + } // namespace } // namespace model } // namespace firestore