diff --git a/.changeset/heavy-starfishes-count.md b/.changeset/heavy-starfishes-count.md new file mode 100644 index 00000000000..cda5a38dd8b --- /dev/null +++ b/.changeset/heavy-starfishes-count.md @@ -0,0 +1,6 @@ +--- +"@firebase/firestore-compat": feat +"@firebase/firestore": feat +--- + +Relaxing query validation performed by the SDK diff --git a/packages/firestore-compat/test/validation.test.ts b/packages/firestore-compat/test/validation.test.ts index 26d45888193..0b94df7950e 100644 --- a/packages/firestore-compat/test/validation.test.ts +++ b/packages/firestore-compat/test/validation.test.ts @@ -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 @@ -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') @@ -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') @@ -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( @@ -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."); } ); @@ -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." ); diff --git a/packages/firestore/src/core/query.ts b/packages/firestore/src/core/query.ts index 310f7f6ac4b..9045a470e2c 100644 --- a/packages/firestore/src/core/query.ts +++ b/packages/firestore/src/core/query.ts @@ -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, @@ -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 diff --git a/packages/firestore/src/lite-api/query.ts b/packages/firestore/src/lite-api/query.ts index 0b3de2268d0..30e88927742 100644 --- a/packages/firestore/src/lite-api/query.ts +++ b/packages/firestore/src/lite-api/query.ts @@ -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, diff --git a/packages/firestore/test/integration/api/query.test.ts b/packages/firestore/test/integration/api/query.test.ts index f76baf19b9f..a92eef8443c 100644 --- a/packages/firestore/test/integration/api/query.test.ts +++ b/packages/firestore/test/integration/api/query.test.ts @@ -1574,6 +1574,248 @@ apiDescribe('Queries', (persistence: boolean) => { ); }); }); + + // eslint-disable-next-line no-restricted-properties + it('supports multiple in ops', () => { + const testDocs = { + doc1: { a: 1, b: 0 }, + doc2: { b: 1 }, + doc3: { a: 3, b: 2 }, + doc4: { a: 1, b: 3 }, + doc5: { a: 1 }, + doc6: { a: 2 } + }; + + return withTestCollection(persistence, testDocs, async coll => { + // Two IN operations on different fields with disjunction. + await checkOnlineAndOfflineResultsMatch( + query( + coll, + or(where('a', 'in', [2, 3]), where('b', 'in', [0, 2])), + orderBy('a') + ), + 'doc1', + 'doc6', + 'doc3' + ); + + // Two IN operations on different fields with conjunction. + await checkOnlineAndOfflineResultsMatch( + query( + coll, + and(where('a', 'in', [2, 3]), where('b', 'in', [0, 2])), + orderBy('a') + ), + 'doc3' + ); + + // Two IN operations on the same field. + // a IN [1,2,3] && a IN [0,1,4] should result in "a==1". + await checkOnlineAndOfflineResultsMatch( + query( + coll, + and(where('a', 'in', [1, 2, 3]), where('a', 'in', [0, 1, 4])) + ), + 'doc1', + 'doc4', + 'doc5' + ); + + // a IN [2,3] && a IN [0,1,4] is never true and so the result should be an + // empty set. + await checkOnlineAndOfflineResultsMatch( + query( + coll, + and(where('a', 'in', [2, 3]), where('a', 'in', [0, 1, 4])) + ) + ); + + // a IN [0,3] || a IN [0,2] should union them (similar to: a IN [0,2,3]). + await checkOnlineAndOfflineResultsMatch( + query(coll, or(where('a', 'in', [0, 3]), where('a', 'in', [0, 2]))), + 'doc3', + 'doc6' + ); + + // Nested composite filter on the same field. + await checkOnlineAndOfflineResultsMatch( + query( + coll, + and( + where('a', 'in', [1, 3]), + or( + where('a', 'in', [0, 2]), + and(where('b', '>=', 1), where('a', 'in', [1, 3])) + ) + ) + ), + 'doc3', + 'doc4' + ); + + // Nested composite filter on the different fields. + await checkOnlineAndOfflineResultsMatch( + query( + coll, + and( + where('b', 'in', [0, 3]), + or( + where('b', 'in', [1]), + and(where('b', 'in', [2, 3]), where('a', 'in', [1, 3])) + ) + ) + ), + 'doc4' + ); + }); + }); + + // eslint-disable-next-line no-restricted-properties + it('supports using in with array contains any', () => { + const testDocs = { + 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 } + }; + + return withTestCollection(persistence, testDocs, async coll => { + await checkOnlineAndOfflineResultsMatch( + query( + coll, + or( + where('a', 'in', [2, 3]), + where('b', 'array-contains-any', [0, 7]) + ) + ), + 'doc1', + 'doc3', + 'doc4', + 'doc6' + ); + + await checkOnlineAndOfflineResultsMatch( + query( + coll, + and( + where('a', 'in', [2, 3]), + where('b', 'array-contains-any', [0, 7]) + ) + ), + 'doc3' + ); + + await checkOnlineAndOfflineResultsMatch( + query( + coll, + or( + and(where('a', 'in', [2, 3]), where('c', '==', 10)), + where('b', 'array-contains-any', [0, 7]) + ) + ), + 'doc1', + 'doc3', + 'doc4' + ); + + await checkOnlineAndOfflineResultsMatch( + query( + coll, + and( + where('a', 'in', [2, 3]), + or(where('b', 'array-contains-any', [0, 7]), where('c', '==', 20)) + ) + ), + 'doc3', + 'doc6' + ); + }); + }); + + // eslint-disable-next-line no-restricted-properties + it('supports using in with array contains', () => { + const testDocs = { + 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 } + }; + + return withTestCollection(persistence, testDocs, async coll => { + await checkOnlineAndOfflineResultsMatch( + query( + coll, + or(where('a', 'in', [2, 3]), where('b', 'array-contains', 3)) + ), + 'doc3', + 'doc4', + 'doc6' + ); + + await checkOnlineAndOfflineResultsMatch( + query( + coll, + and(where('a', 'in', [2, 3]), where('b', 'array-contains', 7)) + ), + 'doc3' + ); + + await checkOnlineAndOfflineResultsMatch( + query( + coll, + or( + where('a', 'in', [2, 3]), + and(where('b', 'array-contains', 3), where('a', '==', 1)) + ) + ), + 'doc3', + 'doc4', + 'doc6' + ); + + await checkOnlineAndOfflineResultsMatch( + query( + coll, + and( + where('a', 'in', [2, 3]), + or(where('b', 'array-contains', 7), where('a', '==', 1)) + ) + ), + 'doc3' + ); + }); + }); + + // eslint-disable-next-line no-restricted-properties + it('supports order by equality', () => { + const testDocs = { + 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 } + }; + + return withTestCollection(persistence, testDocs, async coll => { + await checkOnlineAndOfflineResultsMatch( + query(coll, where('a', '==', 1), orderBy('a')), + 'doc1', + 'doc4', + 'doc5' + ); + + await checkOnlineAndOfflineResultsMatch( + query(coll, where('a', 'in', [2, 3]), orderBy('a')), + 'doc6', + 'doc3' + ); + }); + }); }); // Reproduces https://github.com/firebase/firebase-js-sdk/issues/5873 diff --git a/packages/firestore/test/integration/api/validation.test.ts b/packages/firestore/test/integration/api/validation.test.ts index c118e485310..8ec14d1ae3e 100644 --- a/packages/firestore/test/integration/api/validation.test.ts +++ b/packages/firestore/test/integration/api/validation.test.ts @@ -894,51 +894,6 @@ apiDescribe('Validation:', (persistence: boolean) => { } ); - validationIt(persistence, 'with multiple array filters fail', db => { - expect(() => - query( - collection(db, '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(() => - query( - collection(db, '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(() => - query( - collection(db, '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(() => - query( - collection(db, '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(() => query( @@ -962,14 +917,6 @@ apiDescribe('Validation:', (persistence: boolean) => { }); validationIt(persistence, 'with multiple disjunctive filters fail', db => { - expect(() => - query( - collection(db, 'test'), - where('foo', 'in', [1, 2]), - where('foo', 'in', [2, 3]) - ) - ).to.throw("Invalid query. You cannot use more than one 'in' filter."); - expect(() => query( collection(db, 'test'), @@ -980,39 +927,6 @@ apiDescribe('Validation:', (persistence: boolean) => { "Invalid query. You cannot use more than one 'not-in' filter." ); - expect(() => - query( - collection(db, '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(() => - query( - collection(db, '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(() => - query( - collection(db, '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(() => query( collection(db, 'test'), @@ -1054,55 +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(() => - query( - collection(db, '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(() => - query( - collection(db, '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(() => - query( - collection(db, '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(() => - query( - collection(db, '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( @@ -1124,26 +989,6 @@ apiDescribe('Validation:', (persistence: boolean) => { where('foo', 'array-contains', 1) ) ).not.to.throw(); - - expect(() => - query( - collection(db, '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(() => - query( - collection(db, '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."); } ); @@ -1164,31 +1009,6 @@ apiDescribe('Validation:', (persistence: boolean) => { "'array-contains-any' filters." ); - expect(() => - query( - collection(db, '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(() => - query( - collection(db, '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(() => query(collection(db, 'test'), where('foo', 'in', [])) ).to.throw(