Skip to content

Remove some query restrictions and add integration tests #10449

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 17, 2023
Merged

Remove some query restrictions and add integration tests #10449

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ce8e27e
Adding test
cherylEnkidu Nov 1, 2022
640ee32
Remove more restrictions
cherylEnkidu Feb 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
180 changes: 180 additions & 0 deletions Firestore/Example/Tests/Integration/API/FIRQueryTests.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1020,4 +1020,184 @@ - (void)testOrQueriesWithArrayMembership {
matchesResult:@[ @"doc1", @"doc4", @"doc6" ]];
}

- (void)testMultipleInOps {
FIRCollectionReference *collRef = [self collectionRefWithDocuments:@{
@"doc1" : @{@"a" : @1, @"b" : @0},
@"doc2" : @{@"b" : @1},
@"doc3" : @{@"a" : @3, @"b" : @2},
@"doc4" : @{@"a" : @1, @"b" : @3},
@"doc5" : @{@"a" : @1},
@"doc6" : @{@"a" : @2}
}];

// Two IN operations on different fields with disjunction.
FIRFilter *filter1 = [FIRFilter orFilterWithFilters:@[
[FIRFilter filterWhereField:@"a" in:@[ @2, @3 ]], [FIRFilter filterWhereField:@"b"
in:@[ @0, @2 ]]
]];
[self checkOnlineAndOfflineQuery:[[collRef queryWhereFilter:filter1] queryOrderedByField:@"a"]
matchesResult:@[ @"doc1", @"doc6", @"doc3" ]];

// Two IN operations on different fields with conjunction.
FIRFilter *filter2 = [FIRFilter andFilterWithFilters:@[
[FIRFilter filterWhereField:@"a" in:@[ @2, @3 ]], [FIRFilter filterWhereField:@"b"
in:@[ @0, @2 ]]
]];
[self checkOnlineAndOfflineQuery:[[collRef queryWhereFilter:filter2] queryOrderedByField:@"a"]
matchesResult:@[ @"doc3" ]];

// Two IN operations on the same field.
// a IN [1,2,3] && a IN [0,1,4] should result in "a==1".
FIRFilter *filter3 = [FIRFilter andFilterWithFilters:@[
[FIRFilter filterWhereField:@"a" in:@[ @1, @2, @3 ]],
[FIRFilter filterWhereField:@"a" in:@[ @0, @1, @4 ]]
]];
[self checkOnlineAndOfflineQuery:[collRef queryWhereFilter:filter3]
matchesResult:@[ @"doc1", @"doc4", @"doc5" ]];

// a IN [2,3] && a IN [0,1,4] is never true and so the result should be an empty set.
FIRFilter *filter4 = [FIRFilter andFilterWithFilters:@[
[FIRFilter filterWhereField:@"a" in:@[ @2, @3 ]], [FIRFilter filterWhereField:@"a"
in:@[ @0, @1, @4 ]]
]];
[self checkOnlineAndOfflineQuery:[collRef queryWhereFilter:filter4] matchesResult:@[]];

// a IN [0,3] || a IN [0,2] should union them (similar to: a IN [0,2,3]).
FIRFilter *filter5 = [FIRFilter orFilterWithFilters:@[
[FIRFilter filterWhereField:@"a" in:@[ @0, @3 ]], [FIRFilter filterWhereField:@"a"
in:@[ @0, @2 ]]
]];
[self checkOnlineAndOfflineQuery:[collRef queryWhereFilter:filter5]
matchesResult:@[ @"doc3", @"doc6" ]];

// Nested composite filter on the same field.
FIRFilter *filter6 = [FIRFilter andFilterWithFilters:@[
[FIRFilter filterWhereField:@"a" in:@[ @1, @3 ]], [FIRFilter orFilterWithFilters:@[
[FIRFilter filterWhereField:@"a" in:@[ @0, @2 ]], [FIRFilter andFilterWithFilters:@[
[FIRFilter filterWhereField:@"b" isGreaterThanOrEqualTo:@1],
[FIRFilter filterWhereField:@"a" in:@[ @1, @3 ]]
]]
]]
]];
[self checkOnlineAndOfflineQuery:[collRef queryWhereFilter:filter6]
matchesResult:@[ @"doc3", @"doc4" ]];

// Nested composite filter on different fields.
FIRFilter *filter7 = [FIRFilter andFilterWithFilters:@[
[FIRFilter filterWhereField:@"b" in:@[ @0, @3 ]], [FIRFilter orFilterWithFilters:@[
[FIRFilter filterWhereField:@"b" in:@[ @1 ]], [FIRFilter andFilterWithFilters:@[
[FIRFilter filterWhereField:@"b" in:@[ @2, @3 ]], [FIRFilter filterWhereField:@"a"
in:@[ @1, @3 ]]
]]
]]
]];
[self checkOnlineAndOfflineQuery:[collRef queryWhereFilter:filter7] matchesResult:@[ @"doc4" ]];
}

- (void)testUseInWithArrayContainsAny {
FIRCollectionReference *collRef = [self collectionRefWithDocuments:@{
@"doc1" : @{@"a" : @1, @"b" : @[ @0 ]},
@"doc2" : @{@"b" : @[ @1 ]},
@"doc3" : @{@"a" : @3, @"b" : @[ @2, @7 ], @"c" : @10},
@"doc4" : @{@"a" : @1, @"b" : @[ @3, @7 ]},
@"doc5" : @{@"a" : @1},
@"doc6" : @{@"a" : @2, @"c" : @20}
}];

FIRFilter *filter1 = [FIRFilter orFilterWithFilters:@[
[FIRFilter filterWhereField:@"a" in:@[ @2, @3 ]], [FIRFilter filterWhereField:@"b"
arrayContainsAny:@[ @0, @7 ]]
]];
[self checkOnlineAndOfflineQuery:[collRef queryWhereFilter:filter1]
matchesResult:@[ @"doc1", @"doc3", @"doc4", @"doc6" ]];

FIRFilter *filter2 = [FIRFilter andFilterWithFilters:@[
[FIRFilter filterWhereField:@"a" in:@[ @2, @3 ]], [FIRFilter filterWhereField:@"b"
arrayContainsAny:@[ @0, @7 ]]
]];
[self checkOnlineAndOfflineQuery:[collRef queryWhereFilter:filter2] matchesResult:@[ @"doc3" ]];

FIRFilter *filter3 = [FIRFilter orFilterWithFilters:@[
[FIRFilter andFilterWithFilters:@[
[FIRFilter filterWhereField:@"a" in:@[ @2, @3 ]], [FIRFilter filterWhereField:@"c"
isEqualTo:@10]
]],
[FIRFilter filterWhereField:@"b" arrayContainsAny:@[ @0, @7 ]]
]];
[self checkOnlineAndOfflineQuery:[collRef queryWhereFilter:filter3]
matchesResult:@[ @"doc1", @"doc3", @"doc4" ]];

FIRFilter *filter4 = [FIRFilter andFilterWithFilters:@[
[FIRFilter filterWhereField:@"a" in:@[ @2, @3 ]], [FIRFilter orFilterWithFilters:@[
[FIRFilter filterWhereField:@"b" arrayContainsAny:@[ @0, @7 ]],
[FIRFilter filterWhereField:@"c" isEqualTo:@20]
]]
]];
[self checkOnlineAndOfflineQuery:[collRef queryWhereFilter:filter4]
matchesResult:@[ @"doc3", @"doc6" ]];
}

- (void)testUseInWithArrayContains {
FIRCollectionReference *collRef = [self collectionRefWithDocuments:@{
@"doc1" : @{@"a" : @1, @"b" : @[ @0 ]},
@"doc2" : @{@"b" : @[ @1 ]},
@"doc3" : @{@"a" : @3, @"b" : @[ @2, @7 ]},
@"doc4" : @{@"a" : @1, @"b" : @[ @3, @7 ]},
@"doc5" : @{@"a" : @1},
@"doc6" : @{@"a" : @2}
}];

FIRFilter *filter1 = [FIRFilter orFilterWithFilters:@[
[FIRFilter filterWhereField:@"a" in:@[ @2, @3 ]], [FIRFilter filterWhereField:@"b"
arrayContainsAny:@[ @3 ]]
]];
[self checkOnlineAndOfflineQuery:[collRef queryWhereFilter:filter1]
matchesResult:@[ @"doc3", @"doc4", @"doc6" ]];

FIRFilter *filter2 = [FIRFilter andFilterWithFilters:@[
[FIRFilter filterWhereField:@"a" in:@[ @2, @3 ]], [FIRFilter filterWhereField:@"b"
arrayContains:@7]
]];
[self checkOnlineAndOfflineQuery:[collRef queryWhereFilter:filter2] matchesResult:@[ @"doc3" ]];

FIRFilter *filter3 = [FIRFilter orFilterWithFilters:@[
[FIRFilter filterWhereField:@"a" in:@[ @2, @3 ]], [FIRFilter andFilterWithFilters:@[
[FIRFilter filterWhereField:@"b" arrayContains:@3], [FIRFilter filterWhereField:@"a"
isEqualTo:@1]
]]
]];
[self checkOnlineAndOfflineQuery:[collRef queryWhereFilter:filter3]
matchesResult:@[ @"doc3", @"doc4", @"doc6" ]];

FIRFilter *filter4 = [FIRFilter andFilterWithFilters:@[
[FIRFilter filterWhereField:@"a" in:@[ @2, @3 ]], [FIRFilter orFilterWithFilters:@[
[FIRFilter filterWhereField:@"b" arrayContains:@7], [FIRFilter filterWhereField:@"a"
isEqualTo:@1]
]]
]];
[self checkOnlineAndOfflineQuery:[collRef queryWhereFilter:filter4] matchesResult:@[ @"doc3" ]];
}

- (void)testOrderByEquality {
FIRCollectionReference *collRef = [self collectionRefWithDocuments:@{
@"doc1" : @{@"a" : @1, @"b" : @[ @0 ]},
@"doc2" : @{@"b" : @[ @1 ]},
@"doc3" : @{@"a" : @3, @"b" : @[ @2, @7 ], @"c" : @10},
@"doc4" : @{@"a" : @1, @"b" : @[ @3, @7 ]},
@"doc5" : @{@"a" : @1},
@"doc6" : @{@"a" : @2, @"c" : @20}
}];

[self checkOnlineAndOfflineQuery:[[collRef queryWhereFilter:[FIRFilter filterWhereField:@"a"
isEqualTo:@1]]
queryOrderedByField:@"a"]
matchesResult:@[ @"doc1", @"doc4", @"doc5" ]];

[self checkOnlineAndOfflineQuery:[[collRef
queryWhereFilter:[FIRFilter filterWhereField:@"a"
in:@[ @2, @3 ]]]
queryOrderedByField:@"a"]
matchesResult:@[ @"doc6", @"doc3" ]];
}

@end
80 changes: 0 additions & 80 deletions Firestore/Example/Tests/Integration/API/FIRValidationTests.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -774,23 +774,6 @@ - (void)testQueriesWithMultipleNotEqualAndInequalitiesFail {
"the same field. But you have inequality filters on 'x' and 'y'");
}

- (void)testQueriesWithMultipleArrayFiltersFail {
FIRCollectionReference *coll = [self.db collectionWithPath:@"collection"];
FSTAssertThrows([[coll queryWhereField:@"foo" arrayContains:@1] queryWhereField:@"foo"
arrayContains:@2],
@"Invalid Query. You cannot use more than one 'arrayContains' filter.");

FSTAssertThrows(
[[coll queryWhereField:@"foo" arrayContains:@1] queryWhereField:@"foo"
arrayContainsAny:@[ @2 ]],
@"Invalid Query. You cannot use 'arrayContainsAny' filters with 'arrayContains' filters.");

FSTAssertThrows(
[[coll queryWhereField:@"foo" arrayContainsAny:@[ @1 ]] queryWhereField:@"foo"
arrayContains:@2],
@"Invalid Query. You cannot use 'arrayContains' filters with 'arrayContainsAny' filters.");
}

- (void)testQueriesWithNotEqualAndNotInFiltersFail {
FIRCollectionReference *coll = [self.db collectionWithPath:@"collection"];

Expand All @@ -805,25 +788,11 @@ - (void)testQueriesWithNotEqualAndNotInFiltersFail {

- (void)testQueriesWithMultipleDisjunctiveFiltersFail {
FIRCollectionReference *coll = [self.db collectionWithPath:@"collection"];
FSTAssertThrows([[coll queryWhereField:@"foo" in:@[ @1 ]] queryWhereField:@"foo" in:@[ @2 ]],
@"Invalid Query. You cannot use more than one 'in' filter.");

FSTAssertThrows([[coll queryWhereField:@"foo" arrayContainsAny:@[ @1 ]] queryWhereField:@"foo"
arrayContainsAny:@[ @2 ]],
@"Invalid Query. You cannot use more than one 'arrayContainsAny' filter.");

FSTAssertThrows([[coll queryWhereField:@"foo" notIn:@[ @1 ]] queryWhereField:@"foo"
notIn:@[ @2 ]],
@"Invalid Query. You cannot use more than one 'notIn' filter.");

FSTAssertThrows([[coll queryWhereField:@"foo" arrayContainsAny:@[ @1 ]] queryWhereField:@"foo"
in:@[ @2 ]],
@"Invalid Query. You cannot use 'in' filters with 'arrayContainsAny' filters.");

FSTAssertThrows([[coll queryWhereField:@"foo" in:@[ @1 ]] queryWhereField:@"foo"
arrayContainsAny:@[ @2 ]],
@"Invalid Query. You cannot use 'arrayContainsAny' filters with 'in' filters.");

FSTAssertThrows(
[[coll queryWhereField:@"foo" arrayContainsAny:@[ @1 ]] queryWhereField:@"foo" notIn:@[ @2 ]],
@"Invalid Query. You cannot use 'notIn' filters with 'arrayContainsAny' filters.");
Expand All @@ -837,31 +806,6 @@ - (void)testQueriesWithMultipleDisjunctiveFiltersFail {

FSTAssertThrows([[coll queryWhereField:@"foo" notIn:@[ @1 ]] queryWhereField:@"foo" in:@[ @2 ]],
@"Invalid Query. You cannot use 'in' filters with 'notIn' filters.");

// This is redundant with the above tests, but makes sure our validation doesn't get confused.
FSTAssertThrows([[[coll queryWhereField:@"foo"
in:@[ @1 ]] queryWhereField:@"foo"
arrayContains:@2] queryWhereField:@"foo"
arrayContainsAny:@[ @2 ]],
@"Invalid Query. You cannot use 'arrayContainsAny' filters with 'in' filters.");

FSTAssertThrows(
[[[coll queryWhereField:@"foo"
arrayContains:@1] queryWhereField:@"foo" in:@[ @2 ]] queryWhereField:@"foo"
arrayContainsAny:@[ @2 ]],
@"Invalid Query. You cannot use 'arrayContainsAny' filters with 'arrayContains' filters.");

FSTAssertThrows([[[coll queryWhereField:@"foo"
notIn:@[ @1 ]] queryWhereField:@"foo"
arrayContains:@2] queryWhereField:@"foo"
arrayContainsAny:@[ @2 ]],
@"Invalid Query. You cannot use 'arrayContains' filters with 'notIn' filters.");

FSTAssertThrows([[[coll queryWhereField:@"foo"
arrayContains:@1] queryWhereField:@"foo"
in:@[ @2 ]] queryWhereField:@"foo"
notIn:@[ @2 ]],
@"Invalid Query. You cannot use 'notIn' filters with 'arrayContains' filters.");
}

- (void)testQueriesCanUseInWithArrayContain {
Expand All @@ -873,18 +817,6 @@ - (void)testQueriesCanUseInWithArrayContain {
XCTAssertNoThrow([[coll queryWhereField:@"foo" in:@[ @1 ]] queryWhereField:@"foo"
arrayContains:@2],
@"IN with arrayContains works.");

FSTAssertThrows([[[coll queryWhereField:@"foo"
in:@[ @1 ]] queryWhereField:@"foo"
arrayContains:@2] queryWhereField:@"foo"
arrayContains:@3],
@"Invalid Query. You cannot use more than one 'arrayContains' filter.");

FSTAssertThrows([[[coll queryWhereField:@"foo"
arrayContains:@1] queryWhereField:@"foo"
in:@[ @2 ]] queryWhereField:@"foo"
in:@[ @3 ]],
@"Invalid Query. You cannot use more than one 'in' filter.");
}

- (void)testQueriesInAndArrayContainsAnyArrayRules {
Expand All @@ -898,18 +830,6 @@ - (void)testQueriesInAndArrayContainsAnyArrayRules {

FSTAssertThrows([coll queryWhereField:@"foo" arrayContainsAny:@[]],
@"Invalid Query. A non-empty array is required for 'arrayContainsAny' filters.");

// The 10 element max includes duplicates.
NSArray *values = @[ @1, @2, @3, @4, @5, @6, @7, @8, @9, @9, @9 ];
FSTAssertThrows(
[coll queryWhereField:@"foo" in:values],
@"Invalid Query. 'in' filters support a maximum of 10 elements in the value array.");
FSTAssertThrows([coll queryWhereField:@"foo" arrayContainsAny:values],
@"Invalid Query. 'arrayContainsAny' filters support a maximum of 10 elements"
" in the value array.");
FSTAssertThrows(
[coll queryWhereField:@"foo" notIn:values],
@"Invalid Query. 'notIn' filters support a maximum of 10 elements in the value array.");
}

#pragma mark - GeoPoint Validation
Expand Down
32 changes: 10 additions & 22 deletions Firestore/core/src/api/query_core.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,30 +77,24 @@ namespace {
* Given an operator, returns the set of operators that cannot be used with
* it.
*
* Operators in a query must adhere to the following set of rules:
* 1. Only one array operator is allowed.
* 2. Only one disjunctive operator is allowed.
* 3. NOT_EQUAL cannot be used with another NOT_EQUAL operator.
* 4. NOT_IN cannot be used with array, disjunctive, or NOT_EQUAL operators.
* This is not a comprehensive check, and this function should be removed in the
* long term.
* Validations should occur in the Firestore backend.
*
* Array operators: ARRAY_CONTAINS, ARRAY_CONTAINS_ANY
* Disjunctive operators: IN, ARRAY_CONTAINS_ANY, NOT_IN
* Operators in a query must adhere to the following set of rules:
* 1. Only one inequality per query.
* 2. NOT_IN cannot be used with array, disjunctive, or NOT_EQUAL operators.
*/
static std::vector<Operator> ConflictingOps(Operator op) {
switch (op) {
case Operator::NotEqual:
return {Operator::NotEqual, Operator::NotIn};
case Operator::ArrayContains:
return {Operator::ArrayContains, Operator::ArrayContainsAny,
Operator::NotIn};
case Operator::In:
return {Operator::ArrayContainsAny, Operator::In, Operator::NotIn};
case Operator::ArrayContainsAny:
return {Operator::ArrayContains, Operator::ArrayContainsAny, Operator::In,
Operator::NotIn};
case Operator::In:
return {Operator::NotIn};
case Operator::NotIn:
return {Operator::ArrayContains, Operator::ArrayContainsAny, Operator::In,
Operator::NotIn, Operator::NotEqual};
return {Operator::ArrayContainsAny, Operator::In, Operator::NotIn,
Operator::NotEqual};
default:
return {};
}
Expand Down Expand Up @@ -413,12 +407,6 @@ void Query::ValidateDisjunctiveFilterElements(
" filters.",
Describe(op));
}
if (value.array_value.values_count > 10) {
ThrowInvalidArgument(
"Invalid Query. '%s' filters support a maximum of 10"
" elements in the value array.",
Describe(op));
}
}

Message<google_firestore_v1_Value> Query::ParseExpectedReferenceValue(
Expand Down