Skip to content

OR queries - relaxing in restrictions #7024

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 6 commits into from
Feb 23, 2023
Merged

OR queries - relaxing in restrictions #7024

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
89a67be
Relaxing in restrictions.
MarkDuckworth Nov 4, 2022
8c0f205
Additional testing.
MarkDuckworth Feb 9, 2023
4e7c5af
Relaxing query restrictions.
MarkDuckworth Feb 17, 2023
88a32a8
Code cleanup.
MarkDuckworth Feb 17, 2023
045c664
Removing compat tests that perform validation on the old query restri…
MarkDuckworth Feb 21, 2023
7adf26c
Create heavy-starfishes-count.md
MarkDuckworth Feb 22, 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
6 changes: 6 additions & 0 deletions .changeset/heavy-starfishes-count.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@firebase/firestore-compat": feat
"@firebase/firestore": feat
---

Relaxing query validation performed by the SDK
164 changes: 0 additions & 164 deletions packages/firestore-compat/test/validation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -901,47 +901,6 @@ apiDescribe('Validation:', (persistence: boolean) => {
}
);

validationIt(persistence, 'with multiple array filters fail', db => {
expect(() =>
db
.collection('test')
.where('foo', 'array-contains', 1)
.where('foo', 'array-contains', 2)
).to.throw(
"Invalid query. You cannot use more than one 'array-contains' filter."
);

expect(() =>
db
.collection('test')
.where('foo', 'array-contains', 1)
.where('foo', 'array-contains-any', [2, 3])
).to.throw(
"Invalid query. You cannot use 'array-contains-any' filters with " +
"'array-contains' filters."
);

expect(() =>
db
.collection('test')
.where('foo', 'array-contains-any', [2, 3])
.where('foo', 'array-contains', 1)
).to.throw(
"Invalid query. You cannot use 'array-contains' filters with " +
"'array-contains-any' filters."
);

expect(() =>
db
.collection('test')
.where('foo', 'not-in', [2, 3])
.where('foo', 'array-contains', 1)
).to.throw(
"Invalid query. You cannot use 'array-contains' filters with " +
"'not-in' filters."
);
});

validationIt(persistence, 'with != and not-in filters fail', db => {
expect(() =>
db
Expand All @@ -963,13 +922,6 @@ apiDescribe('Validation:', (persistence: boolean) => {
});

validationIt(persistence, 'with multiple disjunctive filters fail', db => {
expect(() =>
db
.collection('test')
.where('foo', 'in', [1, 2])
.where('foo', 'in', [2, 3])
).to.throw("Invalid query. You cannot use more than one 'in' filter.");

expect(() =>
db
.collection('test')
Expand All @@ -979,36 +931,6 @@ apiDescribe('Validation:', (persistence: boolean) => {
"Invalid query. You cannot use more than one 'not-in' filter."
);

expect(() =>
db
.collection('test')
.where('foo', 'array-contains-any', [1, 2])
.where('foo', 'array-contains-any', [2, 3])
).to.throw(
"Invalid query. You cannot use more than one 'array-contains-any'" +
' filter.'
);

expect(() =>
db
.collection('test')
.where('foo', 'array-contains-any', [2, 3])
.where('foo', 'in', [2, 3])
).to.throw(
"Invalid query. You cannot use 'in' filters with " +
"'array-contains-any' filters."
);

expect(() =>
db
.collection('test')
.where('foo', 'in', [2, 3])
.where('foo', 'array-contains-any', [2, 3])
).to.throw(
"Invalid query. You cannot use 'array-contains-any' filters with " +
"'in' filters."
);

expect(() =>
db
.collection('test')
Expand Down Expand Up @@ -1046,51 +968,6 @@ apiDescribe('Validation:', (persistence: boolean) => {
).to.throw(
"Invalid query. You cannot use 'not-in' filters with 'in' filters."
);

// This is redundant with the above tests, but makes sure our validation
// doesn't get confused.
expect(() =>
db
.collection('test')
.where('foo', 'in', [2, 3])
.where('foo', 'array-contains', 1)
.where('foo', 'array-contains-any', [2])
).to.throw(
"Invalid query. You cannot use 'array-contains-any' filters with 'in' filters."
);

expect(() =>
db
.collection('test')
.where('foo', 'array-contains', 1)
.where('foo', 'in', [2, 3])
.where('foo', 'array-contains-any', [2])
).to.throw(
"Invalid query. You cannot use 'array-contains-any' filters with " +
"'array-contains' filters."
);

expect(() =>
db
.collection('test')
.where('foo', 'not-in', [2, 3])
.where('foo', 'array-contains', 2)
.where('foo', 'array-contains-any', [2])
).to.throw(
"Invalid query. You cannot use 'array-contains' filters with " +
"'not-in' filters."
);

expect(() =>
db
.collection('test')
.where('foo', 'array-contains', 2)
.where('foo', 'in', [2])
.where('foo', 'not-in', [2, 3])
).to.throw(
"Invalid query. You cannot use 'not-in' filters with " +
"'array-contains' filters."
);
});

validationIt(
Expand All @@ -1110,24 +987,6 @@ apiDescribe('Validation:', (persistence: boolean) => {
.where('foo', 'in', [2, 3])
.where('foo', 'array-contains', 1)
).not.to.throw();

expect(() =>
db
.collection('test')
.where('foo', 'in', [2, 3])
.where('foo', 'array-contains', 1)
.where('foo', 'array-contains', 2)
).to.throw(
"Invalid query. You cannot use more than one 'array-contains' filter."
);

expect(() =>
db
.collection('test')
.where('foo', 'array-contains', 1)
.where('foo', 'in', [2, 3])
.where('foo', 'in', [2, 3])
).to.throw("Invalid query. You cannot use more than one 'in' filter.");
}
);

Expand All @@ -1146,29 +1005,6 @@ apiDescribe('Validation:', (persistence: boolean) => {
"'array-contains-any' filters."
);

expect(() =>
db
.collection('test')
// The 10 element max includes duplicates.
.where('foo', 'in', [1, 2, 3, 4, 5, 6, 7, 8, 9, 9, 9])
).to.throw(
"Invalid Query. 'in' filters support a maximum of 10 elements in " +
'the value array.'
);

expect(() =>
db
.collection('test')
.where(
'foo',
'array-contains-any',
[1, 2, 3, 4, 5, 6, 7, 8, 9, 9, 9]
)
).to.throw(
"Invalid Query. 'array-contains-any' filters support a maximum of " +
'10 elements in the value array.'
);

expect(() => db.collection('test').where('foo', 'in', [])).to.throw(
"Invalid Query. A non-empty array is required for 'in' filters."
);
Expand Down
9 changes: 1 addition & 8 deletions packages/firestore/src/core/query.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {
boundSortsAfterDocument,
boundSortsBeforeDocument
} from './bound';
import { CompositeFilter, Filter } from './filter';
import { Filter } from './filter';
import { Direction, OrderBy } from './order_by';
import {
canonifyTarget,
Expand Down Expand Up @@ -165,13 +165,6 @@ export function queryMatchesAllDocuments(query: Query): boolean {
);
}

export function queryContainsCompositeFilters(query: Query): boolean {
return (
query.filters.find(filter => filter instanceof CompositeFilter) !==
undefined
);
}

export function getFirstOrderByField(query: Query): FieldPath | null {
return query.explicitOrderBy.length > 0
? query.explicitOrderBy[0].field
Expand Down
36 changes: 7 additions & 29 deletions packages/firestore/src/lite-api/query.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1050,49 +1050,27 @@ function validateDisjunctiveFilterElements(
`'${operator.toString()}' filters.`
);
}
if (value.length > 10) {
throw new FirestoreError(
Code.INVALID_ARGUMENT,
`Invalid Query. '${operator.toString()}' filters support a ` +
'maximum of 10 elements in the value array.'
);
}
}

/**
* 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.
*/
function conflictingOps(op: Operator): Operator[] {
switch (op) {
case Operator.NOT_EQUAL:
return [Operator.NOT_EQUAL, Operator.NOT_IN];
case Operator.ARRAY_CONTAINS:
return [
Operator.ARRAY_CONTAINS,
Operator.ARRAY_CONTAINS_ANY,
Operator.NOT_IN
];
case Operator.IN:
return [Operator.ARRAY_CONTAINS_ANY, Operator.IN, Operator.NOT_IN];
case Operator.ARRAY_CONTAINS_ANY:
return [
Operator.ARRAY_CONTAINS,
Operator.ARRAY_CONTAINS_ANY,
Operator.IN,
Operator.NOT_IN
];
case Operator.IN:
return [Operator.NOT_IN];
case Operator.NOT_IN:
return [
Operator.ARRAY_CONTAINS,
Operator.ARRAY_CONTAINS_ANY,
Operator.IN,
Operator.NOT_IN,
Expand Down
Loading