diff --git a/packages/firestore-compat/test/validation.test.ts b/packages/firestore-compat/test/validation.test.ts index 0b94df7950e..99ab4b63db5 100644 --- a/packages/firestore-compat/test/validation.test.ts +++ b/packages/firestore-compat/test/validation.test.ts @@ -825,15 +825,11 @@ apiDescribe('Validation:', (persistence: boolean) => { } ); - validationIt(persistence, 'with different inequality fields fail', db => { + validationIt(persistence, 'can have different inequality fields', db => { const collection = db.collection('test'); expect(() => collection.where('x', '>=', 32).where('y', '<', 'cat') - ).to.throw( - 'Invalid query. All where filters with an ' + - 'inequality (<, <=, !=, not-in, >, or >=) must be on the same field.' + - ` But you have inequality filters on 'x' and 'y'` - ); + ).not.to.throw(); }); validationIt(persistence, 'with more than one != query fail', db => { @@ -845,59 +841,46 @@ apiDescribe('Validation:', (persistence: boolean) => { validationIt( persistence, - 'with != and inequality queries on different fields fail', + 'can have != and inequality queries on different fields', db => { const collection = db.collection('test'); expect(() => collection.where('y', '>', 32).where('x', '!=', 33) - ).to.throw( - 'Invalid query. All where filters with an ' + - 'inequality (<, <=, !=, not-in, >, or >=) must be on the same field.' + - ` But you have inequality filters on 'y' and 'x` - ); + ).not.to.throw(); } ); validationIt( persistence, - 'with != and inequality queries on different fields fail', + 'can have NOT-IN and inequality queries on different fields', db => { const collection = db.collection('test'); expect(() => collection.where('y', '>', 32).where('x', 'not-in', [33]) - ).to.throw( - 'Invalid query. All where filters with an ' + - 'inequality (<, <=, !=, not-in, >, or >=) must be on the same field.' + - ` But you have inequality filters on 'y' and 'x` - ); + ).not.to.throw(); } ); validationIt( persistence, - 'with inequality different than first orderBy fail.', + 'can have inequality different than first orderBy', db => { const collection = db.collection('test'); - const reason = - `Invalid query. You have a where filter with an ` + - `inequality (<, <=, !=, not-in, >, or >=) on field 'x' and so you must also ` + - `use 'x' as your first argument to Query.orderBy(), but your first ` + - `orderBy() is on field 'y' instead.`; - expect(() => collection.where('x', '>', 32).orderBy('y')).to.throw( - reason - ); - expect(() => collection.orderBy('y').where('x', '>', 32)).to.throw( - reason - ); + expect(() => + collection.where('x', '>', 32).orderBy('y') + ).not.to.throw(); + expect(() => + collection.orderBy('y').where('x', '>', 32) + ).not.to.throw(); expect(() => collection.where('x', '>', 32).orderBy('y').orderBy('x') - ).to.throw(reason); + ).not.to.throw(); expect(() => collection.orderBy('y').orderBy('x').where('x', '>', 32) - ).to.throw(reason); - expect(() => collection.where('x', '!=', 32).orderBy('y')).to.throw( - reason - ); + ).not.to.throw(); + expect(() => + collection.where('x', '!=', 32).orderBy('y') + ).not.to.throw(); } ); diff --git a/packages/firestore/src/core/filter.ts b/packages/firestore/src/core/filter.ts index 16bb627f2dc..06e2740c315 100644 --- a/packages/firestore/src/core/filter.ts +++ b/packages/firestore/src/core/filter.ts @@ -56,8 +56,6 @@ export abstract class Filter { abstract getFlattenedFilters(): readonly FieldFilter[]; abstract getFilters(): Filter[]; - - abstract getFirstInequalityField(): FieldPath | null; } export class FieldFilter extends Filter { @@ -194,13 +192,6 @@ export class FieldFilter extends Filter { getFilters(): Filter[] { return [this]; } - - getFirstInequalityField(): FieldPath | null { - if (this.isInequality()) { - return this.field; - } - return null; - } } export class CompositeFilter extends Filter { @@ -246,30 +237,6 @@ export class CompositeFilter extends Filter { getFilters(): Filter[] { return Object.assign([], this.filters); } - - getFirstInequalityField(): FieldPath | null { - const found = this.findFirstMatchingFilter(filter => filter.isInequality()); - - if (found !== null) { - return found.field; - } - return null; - } - - // Performs a depth-first search to find and return the first FieldFilter in the composite filter - // that satisfies the predicate. Returns `null` if none of the FieldFilters satisfy the - // predicate. - private findFirstMatchingFilter( - predicate: (filter: FieldFilter) => boolean - ): FieldFilter | null { - for (const fieldFilter of this.getFlattenedFilters()) { - if (predicate(fieldFilter)) { - return fieldFilter; - } - } - - return null; - } } export function compositeFilterIsConjunction( diff --git a/packages/firestore/src/core/query.ts b/packages/firestore/src/core/query.ts index 098f57f1927..b13296ad7ee 100644 --- a/packages/firestore/src/core/query.ts +++ b/packages/firestore/src/core/query.ts @@ -19,13 +19,14 @@ import { compareDocumentsByField, Document } from '../model/document'; import { DocumentKey } from '../model/document_key'; import { FieldPath, ResourcePath } from '../model/path'; import { debugAssert, debugCast, fail } from '../util/assert'; +import { SortedSet } from '../util/sorted_set'; import { Bound, boundSortsAfterDocument, boundSortsBeforeDocument } from './bound'; -import { Filter } from './filter'; +import { FieldFilter, Filter } from './filter'; import { Direction, OrderBy } from './order_by'; import { canonifyTarget, @@ -172,21 +173,18 @@ export function queryMatchesAllDocuments(query: Query): boolean { ); } -export function getFirstOrderByField(query: Query): FieldPath | null { - return query.explicitOrderBy.length > 0 - ? query.explicitOrderBy[0].field - : null; -} - -export function getInequalityFilterField(query: Query): FieldPath | null { - for (const filter of query.filters) { - const result = filter.getFirstInequalityField(); - if (result !== null) { - return result; - } - } - - return null; +// Returns the sorted set of inequality filter fields used in this query. +export function getInequalityFilterFields(query: Query): SortedSet { + let result = new SortedSet(FieldPath.comparator); + query.filters.forEach((filter: Filter) => { + const subFilters = filter.getFlattenedFilters(); + subFilters.forEach((filter: FieldFilter) => { + if (filter.isInequality()) { + result = result.add(filter.field); + } + }); + }); + return result; } /** @@ -228,45 +226,43 @@ export function queryNormalizedOrderBy(query: Query): OrderBy[] { const queryImpl = debugCast(query, QueryImpl); if (queryImpl.memoizedNormalizedOrderBy === null) { queryImpl.memoizedNormalizedOrderBy = []; + const fieldsNormalized = new Set(); - const inequalityField = getInequalityFilterField(queryImpl); - const firstOrderByField = getFirstOrderByField(queryImpl); - if (inequalityField !== null && firstOrderByField === null) { - // 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 (!inequalityField.isKeyField()) { - queryImpl.memoizedNormalizedOrderBy.push(new OrderBy(inequalityField)); + // Any explicit order by fields should be added as is. + for (const orderBy of queryImpl.explicitOrderBy) { + queryImpl.memoizedNormalizedOrderBy.push(orderBy); + fieldsNormalized.add(orderBy.field.canonicalString()); + } + + // The order of the implicit ordering always matches the last explicit order by. + const lastDirection = + queryImpl.explicitOrderBy.length > 0 + ? queryImpl.explicitOrderBy[queryImpl.explicitOrderBy.length - 1].dir + : Direction.ASCENDING; + + // 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: `SortedSet` sorts the key field before other fields. However, we want the key + // field to be sorted last. + const inequalityFields: SortedSet = + getInequalityFilterFields(queryImpl); + inequalityFields.forEach(field => { + if ( + !fieldsNormalized.has(field.canonicalString()) && + !field.isKeyField() + ) { + queryImpl.memoizedNormalizedOrderBy!.push( + new OrderBy(field, lastDirection) + ); } + }); + + // Add the document key field to the last if it is not explicitly ordered. + if (!fieldsNormalized.has(FieldPath.keyField().canonicalString())) { queryImpl.memoizedNormalizedOrderBy.push( - new OrderBy(FieldPath.keyField(), Direction.ASCENDING) - ); - } else { - debugAssert( - inequalityField === null || - (firstOrderByField !== null && - inequalityField.isEqual(firstOrderByField)), - 'First orderBy should match inequality field.' + new OrderBy(FieldPath.keyField(), lastDirection) ); - let foundKeyOrdering = false; - for (const orderBy of queryImpl.explicitOrderBy) { - queryImpl.memoizedNormalizedOrderBy.push(orderBy); - if (orderBy.field.isKeyField()) { - foundKeyOrdering = true; - } - } - if (!foundKeyOrdering) { - // The order of the implicit key ordering always matches the last - // explicit order-by - const lastDirection = - queryImpl.explicitOrderBy.length > 0 - ? queryImpl.explicitOrderBy[queryImpl.explicitOrderBy.length - 1] - .dir - : Direction.ASCENDING; - queryImpl.memoizedNormalizedOrderBy.push( - new OrderBy(FieldPath.keyField(), lastDirection) - ); - } } } return queryImpl.memoizedNormalizedOrderBy; @@ -350,16 +346,6 @@ function _queryToTarget(queryImpl: QueryImpl, orderBys: OrderBy[]): Target { } export function queryWithAddedFilter(query: Query, filter: Filter): Query { - const newInequalityField = filter.getFirstInequalityField(); - const queryInequalityField = getInequalityFilterField(query); - - debugAssert( - queryInequalityField == null || - newInequalityField == null || - newInequalityField.isEqual(queryInequalityField), - 'Query must only have one inequality field.' - ); - debugAssert( !isDocumentQuery(query), 'No filtering allowed for document query' diff --git a/packages/firestore/src/lite-api/query.ts b/packages/firestore/src/lite-api/query.ts index 2283c92b3e1..f0a357b828c 100644 --- a/packages/firestore/src/lite-api/query.ts +++ b/packages/firestore/src/lite-api/query.ts @@ -28,8 +28,6 @@ import { } from '../core/filter'; import { Direction, OrderBy } from '../core/order_by'; import { - getFirstOrderByField, - getInequalityFilterField, isCollectionGroupQuery, LimitType, Query as InternalQuery, @@ -868,7 +866,6 @@ export function newQueryOrderBy( ); } const orderBy = new OrderBy(fieldPath, direction); - validateNewOrderBy(query, orderBy); return orderBy; } @@ -1100,33 +1097,6 @@ function validateNewFieldFilter( query: InternalQuery, fieldFilter: FieldFilter ): void { - if (fieldFilter.isInequality()) { - const existingInequality = getInequalityFilterField(query); - const newInequality = fieldFilter.field; - - if ( - existingInequality !== null && - !existingInequality.isEqual(newInequality) - ) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - 'Invalid query. All where filters with an inequality' + - ' (<, <=, !=, not-in, >, or >=) must be on the same field. But you have' + - ` inequality filters on '${existingInequality.toString()}'` + - ` and '${newInequality.toString()}'` - ); - } - - const firstOrderByField = getFirstOrderByField(query); - if (firstOrderByField !== null) { - validateOrderByAndInequalityMatch( - query, - newInequality, - firstOrderByField - ); - } - } - const conflictingOp = findOpInsideFilters( query.filters, conflictingOps(fieldFilter.op) @@ -1174,33 +1144,6 @@ function findOpInsideFilters( return null; } -function validateNewOrderBy(query: InternalQuery, orderBy: OrderBy): void { - if (getFirstOrderByField(query) === null) { - // This is the first order by. It must match any inequality. - const inequalityField = getInequalityFilterField(query); - if (inequalityField !== null) { - validateOrderByAndInequalityMatch(query, inequalityField, orderBy.field); - } - } -} - -function validateOrderByAndInequalityMatch( - baseQuery: InternalQuery, - inequality: InternalFieldPath, - orderBy: InternalFieldPath -): void { - if (!orderBy.isEqual(inequality)) { - throw new FirestoreError( - Code.INVALID_ARGUMENT, - `Invalid query. You have a where filter with an inequality ` + - `(<, <=, !=, not-in, >, or >=) on field '${inequality.toString()}' ` + - `and so you must also use '${inequality.toString()}' ` + - `as your first argument to orderBy(), but your first orderBy() ` + - `is on field '${orderBy.toString()}' instead.` - ); - } -} - export function validateQueryFilterConstraint( functionName: string, queryConstraint: AppliableConstraint diff --git a/packages/firestore/src/local/indexeddb_index_manager.ts b/packages/firestore/src/local/indexeddb_index_manager.ts index 71c5c9a70bc..33da3faa41d 100644 --- a/packages/firestore/src/local/indexeddb_index_manager.ts +++ b/packages/firestore/src/local/indexeddb_index_manager.ts @@ -275,10 +275,10 @@ export class IndexedDbIndexManager implements IndexManager { return this.getIndexType(transaction, subTarget).next(type => { if (type === IndexType.NONE || type === IndexType.PARTIAL) { const targetIndexMatcher = new TargetIndexMatcher(subTarget); - return this.addFieldIndex( - transaction, - targetIndexMatcher.buildTargetIndex() - ); + const fieldIndex = targetIndexMatcher.buildTargetIndex(); + if (fieldIndex != null) { + return this.addFieldIndex(transaction, fieldIndex); + } } }); } diff --git a/packages/firestore/src/model/target_index_matcher.ts b/packages/firestore/src/model/target_index_matcher.ts index e274ed7ba05..df80ffa419a 100644 --- a/packages/firestore/src/model/target_index_matcher.ts +++ b/packages/firestore/src/model/target_index_matcher.ts @@ -18,7 +18,7 @@ import { FieldFilter, Operator } from '../core/filter'; import { Direction, OrderBy } from '../core/order_by'; import { Target } from '../core/target'; -import { debugAssert, hardAssert } from '../util/assert'; +import { hardAssert } from '../util/assert'; import { SortedSet } from '../util/sorted_set'; import { @@ -54,8 +54,13 @@ import { FieldPath } from './path'; export class TargetIndexMatcher { // The collection ID (or collection group) of the query target. private readonly collectionId: string; - // The single inequality filter of the target (if it exists). - private readonly inequalityFilter?: FieldFilter; + // The inequality filters of the target (if it exists). + // Note: The sort on FieldFilters is not required. Using SortedSet here just to utilize the custom + // comparator. + private inequalityFilters = new SortedSet((lhs, rhs) => + FieldPath.comparator(lhs.field, rhs.field) + ); + // The list of equality filters of the target. private readonly equalityFilters: FieldFilter[]; // The list of orderBys of the target. @@ -68,22 +73,20 @@ export class TargetIndexMatcher { : target.path.lastSegment(); this.orderBys = target.orderBy; this.equalityFilters = []; - for (const filter of target.filters) { const fieldFilter = filter as FieldFilter; if (fieldFilter.isInequality()) { - debugAssert( - !this.inequalityFilter || - this.inequalityFilter.field.isEqual(fieldFilter.field), - 'Only a single inequality is supported' - ); - this.inequalityFilter = fieldFilter; + this.inequalityFilters = this.inequalityFilters.add(fieldFilter); } else { this.equalityFilters.push(fieldFilter); } } } + get hasMultipleInequality(): boolean { + return this.inequalityFilters.size > 1; + } + /** * Returns whether the index can be used to serve the TargetIndexMatcher's * target. @@ -111,6 +114,12 @@ export class TargetIndexMatcher { 'Collection IDs do not match' ); + if (this.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 arraySegment = fieldIndexGetArraySegment(index); if ( @@ -148,16 +157,17 @@ export class TargetIndexMatcher { return true; } - if (this.inequalityFilter !== undefined) { + if (this.inequalityFilters.size > 0) { + // Only a single inequality is currently supported. Get the only entry in the set. + const inequalityFilter = this.inequalityFilters.getIterator().getNext(); // 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 ( - !equalitySegments.has(this.inequalityFilter.field.canonicalString()) - ) { + if (!equalitySegments.has(inequalityFilter.field.canonicalString())) { const segment = segments[segmentIndex]; + if ( - !this.matchesFilter(this.inequalityFilter, segment) || + !this.matchesFilter(inequalityFilter, segment) || !this.matchesOrderBy(this.orderBys[orderBysIndex++], segment) ) { return false; @@ -182,8 +192,15 @@ export class TargetIndexMatcher { return true; } - /** Returns a full matched field index for this target. */ - buildTargetIndex(): FieldIndex { + /** + * Returns a full matched field index for this target. Currently multiple + * inequality query is not supported so function returns null. + */ + buildTargetIndex(): FieldIndex | null { + if (this.hasMultipleInequality) { + return null; + } + // 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/packages/firestore/test/integration/api/database.test.ts b/packages/firestore/test/integration/api/database.test.ts index 2b5faff54e5..34119cdc39c 100644 --- a/packages/firestore/test/integration/api/database.test.ts +++ b/packages/firestore/test/integration/api/database.test.ts @@ -658,6 +658,22 @@ apiDescribe('Database', persistence => { }); }); + it('inequality on multiple fields works', () => { + return withTestCollection(persistence, {}, async coll => { + expect(() => + query(coll, where('x', '>=', 32), where('y', '!=', 'cat')) + ).not.to.throw(); + }); + }); + + it('inequality with key fields works', () => { + return withTestCollection(persistence, {}, async coll => { + expect(() => + query(coll, where(documentId(), '>=', 'aa'), where('x', '>=', 32)) + ).not.to.throw(); + }); + }); + it('inequality and array-contains on different fields works', () => { return withTestCollection(persistence, {}, async coll => { expect(() => @@ -666,6 +682,44 @@ apiDescribe('Database', persistence => { }); }); + it('inequality and array-contains-any on different fields works', () => { + return withTestCollection(persistence, {}, async coll => { + expect(() => + query( + coll, + where('x', '>=', 32), + where('y', 'array-contains-any', [1, 2]) + ) + ).not.to.throw(); + }); + }); + + it('multiple inequality and array-contains on different fields works', () => { + return withTestCollection(persistence, {}, async coll => { + expect(() => + query( + coll, + where('x', '>=', 32), + where('y', 'array-contains', 'cat'), + where('z', '<=', 42) + ) + ).not.to.throw(); + }); + }); + + it('multiple inequality and array-contains-any on different fields works', () => { + return withTestCollection(persistence, {}, async coll => { + expect(() => + query( + coll, + where('x', '>=', 32), + where('y', 'array-contains-any', [1, 2]), + where('z', '<=', 42) + ) + ).not.to.throw(); + }); + }); + it('inequality and IN on different fields works', () => { return withTestCollection(persistence, {}, async coll => { expect(() => @@ -674,13 +728,35 @@ apiDescribe('Database', persistence => { }); }); - it('inequality and array-contains-any on different fields works', () => { + it('multiple inequality and IN on different fields works', () => { return withTestCollection(persistence, {}, async coll => { expect(() => query( coll, where('x', '>=', 32), - where('y', 'array-contains-any', [1, 2]) + where('y', 'in', [1, 2]), + where('z', '<=', 42) + ) + ).not.to.throw(); + }); + }); + + it('inequality and NOT IN on different fields works', () => { + return withTestCollection(persistence, {}, async coll => { + expect(() => + query(coll, where('x', '>=', 32), where('y', 'not-in', [1, 2])) + ).not.to.throw(); + }); + }); + + it('multiple inequality and NOT IN on different fields works', () => { + return withTestCollection(persistence, {}, async coll => { + expect(() => + query( + coll, + where('x', '>=', 32), + where('y', 'not-in', [1, 2]), + where('z', '<=', 42) ) ).not.to.throw(); }); @@ -708,32 +784,48 @@ apiDescribe('Database', persistence => { }); }); - it('inequality same as first orderBy works.', () => { + it('equality different than orderBy works', () => { return withTestCollection(persistence, {}, async coll => { expect(() => - query(coll, where('x', '>', 32), orderBy('x'), orderBy('y')) - ).not.to.throw(); - expect(() => - query(coll, orderBy('x'), where('x', '>', 32), orderBy('y')) + query(coll, orderBy('x'), where('y', '==', 'cat')) ).not.to.throw(); }); }); - it('!= same as first orderBy works.', () => { + it('inequality different than orderBy works.', () => { return withTestCollection(persistence, {}, async coll => { expect(() => - query(coll, where('x', '!=', 32), orderBy('x'), orderBy('y')) + query(coll, where('x', '>', 32), orderBy('y')) + ).not.to.throw(); + expect(() => + query(coll, orderBy('y'), where('x', '>', 32)) ).not.to.throw(); expect(() => - query(coll, orderBy('x'), where('x', '!=', 32), orderBy('y')) + query(coll, where('x', '>', 32), orderBy('y'), orderBy('z')) + ).not.to.throw(); + expect(() => + query(coll, orderBy('y'), where('x', '>', 32), orderBy('x')) ).not.to.throw(); }); }); - it('equality different than orderBy works', () => { + it('multiple inequality different from orderBy works.', () => { return withTestCollection(persistence, {}, async coll => { expect(() => - query(coll, orderBy('x'), where('y', '==', 'cat')) + query( + coll, + where('x', '>', 32), + where('y', '!=', 'cat'), + orderBy('z') + ) + ).not.to.throw(); + expect(() => + query( + coll, + orderBy('z'), + where('x', '>', 32), + where('y', '!=', 'cat') + ) ).not.to.throw(); }); }); @@ -1051,6 +1143,33 @@ apiDescribe('Database', persistence => { }); }); + it('can compare multiple inequality Query instances with isEqual().', () => { + return withTestDb(persistence, async firestore => { + const query1 = query( + collection(firestore, 'foo'), + where('x', '>=', 42), + where('y', '!=', 42), + orderBy('z') + ); + const query2 = query( + collection(firestore, 'foo'), + where('x', '>=', 42), + where('y', '!=', 42), + orderBy('z') + ); + expect(queryEqual(query1, query2)).to.be.true; + + // Inequality fields in different order + const query3 = query( + collection(firestore, 'foo'), + where('y', '!=', 42), + where('x', '>=', 42), + orderBy('z') + ); + expect(queryEqual(query1, query3)).to.be.false; + }); + }); + it('can traverse collections and documents.', () => { return withTestDb(persistence, async db => { const expected = 'a/b/c/d'; diff --git a/packages/firestore/test/integration/api/query.test.ts b/packages/firestore/test/integration/api/query.test.ts index 9c842cf2cd7..3a0e7c1c2e8 100644 --- a/packages/firestore/test/integration/api/query.test.ts +++ b/packages/firestore/test/integration/api/query.test.ts @@ -23,9 +23,11 @@ import { EventsAccumulator } from '../util/events_accumulator'; import { addDoc, and, + average, Bytes, collection, collectionGroup, + count, deleteDoc, disableNetwork, doc, @@ -36,7 +38,10 @@ import { enableNetwork, endAt, endBefore, + FieldPath, GeoPoint, + getAggregateFromServer, + getCountFromServer, getDocs, getDocsFromCache, getDocsFromServer, @@ -51,6 +56,7 @@ import { setDoc, startAfter, startAt, + sum, Timestamp, updateDoc, where, @@ -1340,6 +1346,576 @@ apiDescribe('Queries', persistence => { }); }); + // eslint-disable-next-line no-restricted-properties + (USE_EMULATOR ? describe : describe.skip)('Multiple Inequality', () => { + it('can use multiple inequality filters', async () => { + const testDocs = { + 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 } + }; + return withTestCollection(persistence, testDocs, async coll => { + // Multiple inequality fields + const snapshot1 = await getDocs( + query( + coll, + where('key', '!=', 'a'), + where('sort', '<=', 2), + where('v', '>', 2) + ) + ); + expect(toIds(snapshot1)).to.deep.equal(['doc3']); + + // Duplicate inequality fields + const snapshot2 = await getDocs( + query( + coll, + where('key', '!=', 'a'), + where('sort', '<=', 2), + where('sort', '>', 1) + ) + ); + expect(toIds(snapshot2)).to.deep.equal(['doc4']); + + // With multiple IN + const snapshot3 = await getDocs( + query( + coll, + where('key', '>=', 'a'), + where('sort', '<=', 2), + where('v', 'in', [2, 3, 4]), + where('sort', 'in', [2, 3]) + ) + ); + expect(toIds(snapshot3)).to.deep.equal(['doc4']); + + // With NOT-IN + const snapshot4 = await getDocs( + query( + coll, + where('key', '>=', 'a'), + where('sort', '<=', 2), + where('v', 'not-in', [2, 4, 5]) + ) + ); + expect(toIds(snapshot4)).to.deep.equal(['doc1', 'doc3']); + + // With orderby + const snapshot5 = await getDocs( + query( + coll, + where('key', '>=', 'a'), + where('sort', '<=', 2), + orderBy('v', 'desc') + ) + ); + expect(toIds(snapshot5)).to.deep.equal(['doc3', 'doc4', 'doc1']); + + // With limit + const snapshot6 = await getDocs( + query( + coll, + where('key', '>=', 'a'), + where('sort', '<=', 2), + orderBy('v', 'desc'), + limit(2) + ) + ); + expect(toIds(snapshot6)).to.deep.equal(['doc3', 'doc4']); + + // With limitToLast + const snapshot7 = await getDocs( + query( + coll, + where('key', '>=', 'a'), + where('sort', '<=', 2), + orderBy('v', 'desc'), + limitToLast(2) + ) + ); + expect(toIds(snapshot7)).to.deep.equal(['doc4', 'doc1']); + }); + }); + + it('can use on special values', async () => { + const testDocs = { + doc1: { key: 'a', sort: 0, v: 0 }, + doc2: { key: 'b', sort: NaN, v: 1 }, + doc3: { key: 'c', sort: null, v: 3 }, + doc4: { key: 'd', v: 0 }, + doc5: { key: 'e', sort: 1 }, + doc6: { key: 'f', sort: 1, v: 1 } + }; + return withTestCollection(persistence, testDocs, async coll => { + const snapshot1 = await getDocs( + query(coll, where('key', '!=', 'a'), where('sort', '<=', 2)) + ); + expect(toIds(snapshot1)).to.deep.equal(['doc5', 'doc6']); + + const snapshot2 = await getDocs( + query( + coll, + where('key', '!=', 'a'), + where('sort', '<=', 2), + where('v', '<=', 1) + ) + ); + expect(toIds(snapshot2)).to.deep.equal(['doc6']); + }); + }); + it('can use with array membership', async () => { + const testDocs = { + 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: [null] } + }; + return withTestCollection(persistence, testDocs, async coll => { + const snapshot1 = await getDocs( + query( + coll, + where('key', '!=', 'a'), + where('sort', '>=', 1), + where('v', 'array-contains', 0) + ) + ); + expect(toIds(snapshot1)).to.deep.equal(['doc2']); + + const snapshot2 = await getDocs( + query( + coll, + where('key', '!=', 'a'), + where('sort', '>=', 1), + where('v', 'array-contains-any', [0, 1]) + ) + ); + expect(toIds(snapshot2)).to.deep.equal(['doc2', 'doc4']); + }); + }); + + it('can use with nested field', () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const testData = (n?: number): any => { + n = n || 1; + return { + name: 'room ' + n, + metadata: { + createdAt: n + }, + field: 'field ' + n, + 'field.dot': n, + 'field\\slash': n + }; + }; + + const testDocs = { + 'doc1': testData(400), + 'doc2': testData(200), + 'doc3': testData(100), + 'doc4': testData(300) + }; + + return withTestCollection(persistence, testDocs, async coll => { + const snapshot1 = await getDocs( + query( + coll, + where('metadata.createdAt', '<=', 500), + where('metadata.createdAt', '>', 100), + where('name', '!=', 'room 200'), + orderBy('name') + ) + ); + expect(toIds(snapshot1)).to.deep.equal(['doc4', 'doc1']); + + const snapshot2 = await getDocs( + query( + coll, + where('field', '>=', 'field 100'), + where(new FieldPath('field.dot'), '!=', 300), + where('field\\slash', '<', 400), + orderBy('name', 'desc') + ) + ); + expect(toIds(snapshot2)).to.deep.equal(['doc2', 'doc3']); + }); + }); + + it('can use with nested composite filters', async () => { + const testDocs = { + 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 } + }; + return withTestCollection(persistence, testDocs, async coll => { + const snapshot1 = await getDocs( + query( + coll, + or( + and(where('key', '==', 'b'), where('sort', '<=', 2)), + and(where('key', '!=', 'b'), where('v', '>', 4)) + ) + ) + ); + // Implicitly ordered by: 'key' asc, 'sort' asc, 'v' asc, __name__ asc + expect(toIds(snapshot1)).to.deep.equal([ + 'doc1', + 'doc6', + 'doc5', + 'doc4' + ]); + + const snapshot2 = await getDocs( + query( + coll, + or( + and(where('key', '==', 'b'), where('sort', '<=', 2)), + and(where('key', '!=', 'b'), where('v', '>', 4)) + ), + orderBy('sort', 'desc'), + orderBy('key') + ) + ); + // Ordered by: 'sort' desc, 'key' asc, 'v' asc, __name__ asc + expect(toIds(snapshot2)).to.deep.equal([ + 'doc5', + 'doc4', + 'doc1', + 'doc6' + ]); + + const snapshot3 = await getDocs( + query( + coll, + and( + or( + and(where('key', '==', 'b'), where('sort', '<=', 4)), + and(where('key', '!=', 'b'), where('v', '>=', 4)) + ), + or( + and(where('key', '>', 'b'), where('sort', '>=', 1)), + and(where('key', '<', 'b'), where('v', '>', 0)) + ) + ) + ) + ); + // Implicitly ordered by: 'key' asc, 'sort' asc, 'v' asc, __name__ asc + expect(toIds(snapshot3)).to.deep.equal(['doc1', 'doc2']); + }); + }); + + it('inequality fields will be implicitly ordered lexicographically', async () => { + const testDocs = { + 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 } + }; + return withTestCollection(persistence, testDocs, async coll => { + // Implicitly ordered by: 'key' asc, 'sort' asc, __name__ asc + const snapshot1 = await getDocs( + query( + coll, + where('key', '!=', 'a'), + where('sort', '>', 1), + where('v', 'in', [1, 2, 3, 4]) + ) + ); + expect(toIds(snapshot1)).to.deep.equal([ + 'doc2', + 'doc4', + 'doc5', + 'doc3' + ]); + + // Implicitly ordered by: 'key' asc, 'sort' asc,__name__ asc + const snapshot2 = await getDocs( + query( + coll, + where('sort', '>', 1), + where('key', '!=', 'a'), + where('v', 'in', [1, 2, 3, 4]) + ) + ); + expect(toIds(snapshot2)).to.deep.equal([ + 'doc2', + 'doc4', + 'doc5', + 'doc3' + ]); + }); + }); + + it('can use multiple explicit order by field', async () => { + const testDocs = { + 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 } + }; + + return withTestCollection(persistence, testDocs, async coll => { + // Ordered by: 'v' asc, 'key' asc, 'sort' asc, __name__ asc + const snapshot1 = await getDocs( + query( + coll, + where('key', '>', 'a'), + where('sort', '>=', 1), + orderBy('v') + ) + ); + expect(toIds(snapshot1)).to.deep.equal([ + 'doc2', + 'doc4', + 'doc3', + 'doc5' + ]); + + // Ordered by: 'v asc, 'sort' asc, 'key' asc, __name__ asc + const snapshot2 = await getDocs( + query( + coll, + where('key', '>', 'a'), + where('sort', '>=', 1), + orderBy('v'), + orderBy('sort') + ) + ); + expect(toIds(snapshot2)).to.deep.equal([ + 'doc2', + 'doc5', + 'doc4', + 'doc3' + ]); + + // Implicit order by matches the direction of last explicit order by. + // Ordered by: 'v' desc, 'key' desc, 'sort' desc, __name__ desc + const snapshot3 = await getDocs( + query( + coll, + where('key', '>', 'a'), + where('sort', '>=', 1), + orderBy('v', 'desc') + ) + ); + expect(toIds(snapshot3)).to.deep.equal([ + 'doc5', + 'doc3', + 'doc4', + 'doc2' + ]); + + // Ordered by: 'v desc, 'sort' asc, 'key' asc, __name__ asc + const snapshot4 = await getDocs( + query( + coll, + where('key', '>', 'a'), + where('sort', '>=', 1), + orderBy('v', 'desc'), + orderBy('sort') + ) + ); + expect(toIds(snapshot4)).to.deep.equal([ + 'doc5', + 'doc4', + 'doc3', + 'doc2' + ]); + }); + }); + + it('can use in aggregate query', async () => { + const testDocs = { + 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 } + }; + + return withTestCollection(persistence, testDocs, async coll => { + const snapshot1 = await getCountFromServer( + query( + coll, + where('key', '>', 'a'), + where('sort', '>=', 1), + orderBy('v') + ) + ); + expect(snapshot1.data().count).to.equal(4); + + const snapshot2 = await getAggregateFromServer( + query( + coll, + where('key', '>', 'a'), + where('sort', '>=', 1), + where('v', '!=', 0) + ), + { + count: count(), + sum: sum('sort'), + avg: average('v') + } + ); + expect(snapshot2.data().count).to.equal(3); + expect(snapshot2.data().sum).to.equal(6); + expect(snapshot2.data().avg).to.equal(1); + }); + }); + + it('can use document ID im multiple inequality query', () => { + const testDocs = { + 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 } + }; + return withTestCollection(persistence, testDocs, async coll => { + const snapshot1 = await getDocs( + query( + coll, + where('sort', '>=', 1), + where('key', '!=', 'a'), + where(documentId(), '<', 'doc5') + ) + ); + // Document Key in inequality field will implicitly ordered to the last. + // Implicitly ordered by: 'key' asc, 'sort' asc, __name__ asc + expect(toIds(snapshot1)).to.deep.equal(['doc2', 'doc4', 'doc3']); + + const snapshot2 = await getDocs( + query( + coll, + where(documentId(), '<', 'doc5'), + where('sort', '>=', 1), + where('key', '!=', 'a') + ) + ); + // Changing filters order will not effect implicit order. + // Implicitly ordered by: 'key' asc, 'sort' asc, __name__ asc + expect(toIds(snapshot2)).to.deep.equal(['doc2', 'doc4', 'doc3']); + + const snapshot3 = await getDocs( + query( + coll, + where(documentId(), '<', 'doc5'), + where('sort', '>=', 1), + where('key', '!=', 'a'), + orderBy('sort', 'desc') + ) + ); + // Ordered by: 'sort' desc,'key' desc, __name__ desc + expect(toIds(snapshot3)).to.deep.equal(['doc2', 'doc3', 'doc4']); + }); + }); + + it('can get documents while offline', () => { + const testDocs = { + doc1: { key: 'a', sort: 1 }, + doc2: { key: 'aa', sort: 4 }, + doc3: { key: 'b', sort: 3 }, + doc4: { key: 'b', sort: 2 } + }; + return withTestCollection( + persistence.toLruGc(), + testDocs, + async (coll, db) => { + const query_ = query( + coll, + where('key', '!=', 'a'), + where('sort', '<=', 3) + ); + //populate the cache. + const snapshot1 = await getDocs(query_); + expect(snapshot1.size).to.equal(2); + + await disableNetwork(db); + + const snapshot2 = await getDocs(query_); + expect(snapshot2.metadata.fromCache).to.be.true; + expect(snapshot2.metadata.hasPendingWrites).to.be.false; + // Implicitly ordered by: 'key' asc, 'sort' asc, __name__ asc + expect(toIds(snapshot2)).to.deep.equal(['doc4', 'doc3']); + } + ); + }); + + // eslint-disable-next-line no-restricted-properties + (persistence.gc === 'lru' ? it : it.skip)( + 'can get same result from server and cache', + () => { + const testDocs = { + 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 } + }; + + return withTestCollection(persistence, testDocs, async coll => { + // implicit AND: a != 1 && b < 2 + await checkOnlineAndOfflineResultsMatch( + query(coll, where('a', '!=', 1), where('b', '<', 2)), + 'doc2' + ); + + // explicit AND: a != 1 && b < 2 + await checkOnlineAndOfflineResultsMatch( + query(coll, and(where('a', '!=', 1), where('b', '<', 2))), + 'doc2' + ); + + // explicit AND: a < 3 && b not-in [2, 3] + // Implicitly ordered by: a asc, b asc, __name__ asc + await checkOnlineAndOfflineResultsMatch( + query(coll, and(where('a', '<', 3), where('b', 'not-in', [2, 3]))), + 'doc1', + 'doc5', + 'doc2' + ); + + // a <3 && b != 0, implicitly ordered by: a asc, b asc, __name__ asc + await checkOnlineAndOfflineResultsMatch( + query(coll, where('b', '!=', 0), where('a', '<', 3), limit(2)), + 'doc5', + 'doc4' + ); + + // a <3 && b != 0, ordered by: b desc, a desc, __name__ desc + await checkOnlineAndOfflineResultsMatch( + query( + coll, + where('a', '<', 3), + where('b', '!=', 0), + orderBy('b', 'desc'), + limit(2) + ), + 'doc4', + 'doc2' + ); + + // explicit OR: multiple inequality: a>2 || b<1. + await checkOnlineAndOfflineResultsMatch( + query(coll, or(where('a', '>', 2), where('b', '<', 1))), + 'doc1', + 'doc3' + ); + }); + } + ); + }); + // OR Query tests only run when the SDK's local cache is configured to use // LRU garbage collection (rather than eager garbage collection) because // they validate that the result from server and cache match. diff --git a/packages/firestore/test/integration/api/validation.test.ts b/packages/firestore/test/integration/api/validation.test.ts index d36f49147b6..8260b53bb4f 100644 --- a/packages/firestore/test/integration/api/validation.test.ts +++ b/packages/firestore/test/integration/api/validation.test.ts @@ -813,17 +813,6 @@ apiDescribe('Validation:', persistence => { } ); - validationIt(persistence, 'with different inequality fields fail', db => { - const coll = collection(db, 'test'); - expect(() => - query(coll, where('x', '>=', 32), where('y', '<', 'cat')) - ).to.throw( - 'Invalid query. All where filters with an ' + - 'inequality (<, <=, !=, not-in, >, or >=) must be on the same field.' + - ` But you have inequality filters on 'x' and 'y'` - ); - }); - validationIt(persistence, 'with more than one != query fail', db => { const coll = collection(db, 'test'); expect(() => @@ -831,64 +820,6 @@ apiDescribe('Validation:', persistence => { ).to.throw("Invalid query. You cannot use more than one '!=' filter."); }); - validationIt( - persistence, - 'with != and inequality queries on different fields fail', - db => { - const coll = collection(db, 'test'); - expect(() => - query(coll, where('y', '>', 32), where('x', '!=', 33)) - ).to.throw( - 'Invalid query. All where filters with an ' + - 'inequality (<, <=, !=, not-in, >, or >=) must be on the same field.' + - ` But you have inequality filters on 'y' and 'x` - ); - } - ); - - validationIt( - persistence, - 'with != and inequality queries on different fields fail', - db => { - const coll = collection(db, 'test'); - expect(() => - query(coll, where('y', '>', 32), where('x', 'not-in', [33])) - ).to.throw( - 'Invalid query. All where filters with an ' + - 'inequality (<, <=, !=, not-in, >, or >=) must be on the same field.' + - ` But you have inequality filters on 'y' and 'x` - ); - } - ); - - validationIt( - persistence, - 'with inequality different than first orderBy fail.', - db => { - const coll = collection(db, 'test'); - const reason = - `Invalid query. You have a where filter with an ` + - `inequality (<, <=, !=, not-in, >, or >=) on field 'x' and so you must also ` + - `use 'x' as your first argument to orderBy(), but your first ` + - `orderBy() is on field 'y' instead.`; - expect(() => query(coll, where('x', '>', 32), orderBy('y'))).to.throw( - reason - ); - expect(() => query(coll, orderBy('y'), where('x', '>', 32))).to.throw( - reason - ); - expect(() => - query(coll, where('x', '>', 32), orderBy('y'), orderBy('x')) - ).to.throw(reason); - expect(() => - query(coll, orderBy('y'), orderBy('x'), where('x', '>', 32)) - ).to.throw(reason); - expect(() => query(coll, where('x', '!=', 32), orderBy('y'))).to.throw( - reason - ); - } - ); - validationIt(persistence, 'with != and not-in filters fail', db => { expect(() => query( @@ -1136,36 +1067,7 @@ apiDescribe('Validation:', persistence => { }); validationIt(persistence, 'invalid query filters fail', db => { - // Multiple inequalities, one of which is inside a nested composite filter. const coll = collection(db, 'test'); - expect(() => - query( - coll, - and( - or( - and(where('a', '==', 'b'), where('c', '>', 'd')), - and(where('e', '==', 'f'), where('g', '==', 'h')) - ), - where('r', '>', 's') - ) - ) - ).to.throw( - "Invalid query. All where filters with an inequality (<, <=, !=, not-in, >, or >=) must be on the same field. But you have inequality filters on 'c' and 'r'" - ); - - // OrderBy and inequality on different fields. Inequality inside a nested composite filter. - expect(() => - query( - coll, - or( - and(where('a', '==', 'b'), where('c', '>', 'd')), - and(where('e', '==', 'f'), where('g', '==', 'h')) - ), - orderBy('r') - ) - ).to.throw( - "Invalid query. You have a where filter with an inequality (<, <=, !=, not-in, >, or >=) on field 'c' and so you must also use 'c' as your first argument to orderBy(), but your first orderBy() is on field 'r' instead." - ); // Conflicting operations within a composite filter. expect(() => @@ -1276,6 +1178,66 @@ apiDescribe('Validation:', persistence => { } } ); + + validationIt( + persistence, + 'conflicting operators inside a nested composite filter', + db => { + const coll = collection(db, 'test'); + // Composite queries can validate conflicting operators. + expect(() => + query( + coll, + and( + or( + and(where('a', '!=', 'b'), where('c', '>=', 'd')), + and(where('e', '==', 'f'), where('g', '!=', 'h')) + ), + or( + and(where('i', '==', 'j'), where('k', '>', 'l')), + and(where('m', '<=', 'n'), where('o', '<', 'p')) + ) + ) + ) + ).to.throw("Invalid query. You cannot use more than one '!=' filter."); + + expect(() => + query( + coll, + and( + or( + and(where('a', '==', 'b'), where('c', '>=', 'd')), + and(where('e', '==', 'f'), where('g', '!=', 'h')) + ), + or( + and(where('i', '==', 'j'), where('k', '>', 'l')), + and(where('m', '<=', 'n'), where('o', 'not-in', ['p'])) + ) + ) + ) + ).to.throw( + "Invalid query. You cannot use 'not-in' filters with '!=' filters." + ); + + expect(() => + query( + coll, + and( + or( + and(where('a', '==', 'b'), where('c', '>=', 'd')), + and(where('e', '==', 'f'), where('g', 'not-in', ['h'])) + ), + or( + and(where('i', '==', 'j'), where('k', '>', 'l')), + and(where('m', '<=', 'n'), where('o', 'not-in', ['p'])) + ) + ) + ) + ).to.throw( + "Invalid query. You cannot use more than one 'not-in' filter." + ); + } + ); }); }); diff --git a/packages/firestore/test/unit/core/query.test.ts b/packages/firestore/test/unit/core/query.test.ts index cfd5d99adeb..fd0e6884c66 100644 --- a/packages/firestore/test/unit/core/query.test.ts +++ b/packages/firestore/test/unit/core/query.test.ts @@ -778,6 +778,144 @@ describe('Query', () => { ); }); + it("generates the correct implicit order by's for multiple inequality", () => { + assertImplicitOrderBy( + query( + 'foo', + filter('a', '<', 5), + filter('a', '>=', 5), + filter('aa', '>', 5), + filter('b', '>', 5), + filter('A', '>', 5) + ), + orderBy('A'), + orderBy('a'), + orderBy('aa'), + orderBy('b'), + orderBy(DOCUMENT_KEY_NAME) + ); + + // numbers + assertImplicitOrderBy( + query( + 'foo', + filter('a', '<', 5), + filter('1', '>', 5), + filter('19', '>', 5), + filter('2', '>', 5) + ), + orderBy('1'), + orderBy('19'), + orderBy('2'), + orderBy('a'), + + orderBy(DOCUMENT_KEY_NAME) + ); + + // nested fields + assertImplicitOrderBy( + query( + 'foo', + filter('a', '<', 5), + filter('aa', '>', 5), + filter('a.a', '>', 5) + ), + orderBy('a'), + orderBy('a.a'), + orderBy('aa'), + orderBy(DOCUMENT_KEY_NAME) + ); + + // special characters + assertImplicitOrderBy( + query( + 'foo', + filter('a', '<', 5), + filter('_a', '>', 5), + filter('a.a', '>', 5) + ), + orderBy('_a'), + orderBy('a'), + orderBy('a.a'), + orderBy(DOCUMENT_KEY_NAME) + ); + + // field name with dot + assertImplicitOrderBy( + query( + 'foo', + filter('a', '<', 5), + filter('a.z', '>', 5), // nested field + filter('`a.a`', '>', 5) // field name with dot + ), + orderBy('a'), + orderBy('a.z'), + orderBy('`a.a`'), + orderBy(DOCUMENT_KEY_NAME) + ); + + // composite filter + assertImplicitOrderBy( + query( + 'foo', + filter('a', '<', 5), + andFilter( + orFilter(filter('b', '>=', 1), filter('c', '<=', 1)), + orFilter(filter('d', '>', 1), filter('e', '==', 1)) + ) + ), + orderBy('a'), + orderBy('b'), + orderBy('c'), + orderBy('d'), + orderBy(DOCUMENT_KEY_NAME) + ); + + // OrderBy + assertImplicitOrderBy( + query( + 'foo', + filter('b', '<', 5), + filter('a', '>', 5), + filter('z', '>', 5), + orderBy('z') + ), + orderBy('z'), + orderBy('a'), + orderBy('b'), + orderBy(DOCUMENT_KEY_NAME) + ); + + // last explicit order by direction + assertImplicitOrderBy( + query( + 'foo', + filter('b', '<', 5), + filter('a', '>', 5), + orderBy('z', 'desc') + ), + orderBy('z', 'desc'), + orderBy('a', 'desc'), + orderBy('b', 'desc'), + orderBy(DOCUMENT_KEY_NAME, 'desc') + ); + + assertImplicitOrderBy( + query( + 'foo', + filter('b', '<', 5), + filter('a', '>', 5), + orderBy('z', 'desc'), + orderBy('c') + ), + orderBy('z', 'desc'), + orderBy('c'), + orderBy('a'), + orderBy('b'), + orderBy(DOCUMENT_KEY_NAME) + ); + }); + it('matchesAllDocuments() considers filters, orders and bounds', () => { const baseQuery = newQueryForPath(ResourcePath.fromString('collection')); expect(queryMatchesAllDocuments(baseQuery)).to.be.true; @@ -853,6 +991,41 @@ describe('Query', () => { assertQueryMatches(query5, [doc3], [doc1, doc2, doc4, doc5]); }); + it('matches composite queries with multiple inequality', () => { + const doc1 = doc('collection/1', 0, { a: 1, b: 0 }); + const doc2 = doc('collection/2', 0, { a: 2, b: 1 }); + const doc3 = doc('collection/3', 0, { a: 3, b: 2 }); + const doc4 = doc('collection/4', 0, { a: 1, b: 3 }); + const doc5 = doc('collection/5', 0, { a: 1, b: 1 }); + + // a>1 || b!=1. + const query1 = query( + 'collection', + orFilter(filter('a', '>', 1), filter('b', '!=', 1)) + ); + assertQueryMatches(query1, [doc1, doc2, doc3, doc4], [doc5]); + + // (a>=1 && b==0) || (a==1 && b!=1) + const query2 = query( + 'collection', + orFilter( + andFilter(filter('a', '>=', 1), filter('b', '==', 0)), + andFilter(filter('a', '==', 1), filter('b', '!=', 1)) + ) + ); + assertQueryMatches(query2, [doc1, doc4], [doc2, doc3, doc5]); + + // a<=2 && (a!=1 || b<3). + const query3 = query( + 'collection', + andFilter( + filter('a', '<=', 2), + orFilter(filter('a', '!=', 1), filter('b', '<', 3)) + ) + ); + assertQueryMatches(query3, [doc1, doc2, doc5], [doc3, doc4]); + }); + it('generates appropriate order-bys for aggregate and non-aggregate targets', () => { const col = newQueryForPath(ResourcePath.fromString('collection')); diff --git a/packages/firestore/test/unit/local/local_store_indexeddb.test.ts b/packages/firestore/test/unit/local/local_store_indexeddb.test.ts index 8da12ded95d..e88b8d6a978 100644 --- a/packages/firestore/test/unit/local/local_store_indexeddb.test.ts +++ b/packages/firestore/test/unit/local/local_store_indexeddb.test.ts @@ -892,4 +892,49 @@ describe('LocalStore w/ IndexedDB Persistence (Non generic)', () => { test.assertRemoteDocumentsRead(0, 1); test.assertQueryReturned('coll/a'); }); + + it('index auto creation does not work with multiple inequality', async () => { + const query_ = query( + 'coll', + filter('field1', '<', 5), + filter('field2', '<', 5) + ); + + const targetId = await test.allocateQuery(query_); + test.configureIndexAutoCreation({ + isEnabled: true, + indexAutoCreationMinCollectionSize: 0, + relativeIndexReadCostPerDocument: 2 + }); + + await test.applyRemoteEvents( + docAddedRemoteEvent(doc('coll/a', 10, { field1: 1, field2: 2 }), [ + targetId + ]), + docAddedRemoteEvent(doc('coll/b', 10, { field1: 8, field2: 2 }), [ + targetId + ]), + docAddedRemoteEvent(doc('coll/c', 10, { field1: 'string', field2: 2 }), [ + targetId + ]), + docAddedRemoteEvent(doc('coll/d', 10, { field1: 1 }), [targetId]), + docAddedRemoteEvent(doc('coll/e', 10, { field1: 4, field2: 4 }), [ + targetId + ]) + ); + + // First time query runs 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. + await test.executeQuery(query_); + test.assertRemoteDocumentsRead(0, 2); + test.assertQueryReturned('coll/a', 'coll/e'); + + await test.backfillIndexes(); + await test.executeQuery(query_); + test.assertRemoteDocumentsRead(0, 2); + test.assertQueryReturned('coll/a', 'coll/e'); + }); }); diff --git a/packages/firestore/test/unit/model/target_index_matcher.test.ts b/packages/firestore/test/unit/model/target_index_matcher.test.ts index 7c7af606839..d35b25f82ef 100644 --- a/packages/firestore/test/unit/model/target_index_matcher.test.ts +++ b/packages/firestore/test/unit/model/target_index_matcher.test.ts @@ -25,7 +25,7 @@ import { newQueryForCollectionGroup } from '../../../src/core/query'; import { targetGetSegmentCount } from '../../../src/core/target'; -import { IndexKind } from '../../../src/model/field_index'; +import { FieldIndex, IndexKind } from '../../../src/model/field_index'; import { TargetIndexMatcher } from '../../../src/model/target_index_matcher'; import { fieldIndex, filter, orderBy, query } from '../../util/helpers'; @@ -956,12 +956,32 @@ describe('Target Bounds', () => { )); }); + describe('query with multiple inequality', () => { + it('returns null', () => { + const q = queryWithAddedFilter( + queryWithAddedFilter(query('collId'), filter('a', '>=', 1)), + filter('b', '<=', 10) + ); + const target = queryToTarget(q); + const targetIndexMatcher = new TargetIndexMatcher(target); + expect(targetIndexMatcher.hasMultipleInequality).is.true; + const actualIndex = targetIndexMatcher.buildTargetIndex(); + expect(actualIndex).is.null; + }); + }); + function validateBuildTargetIndexCreateFullMatchIndex(q: Query): void { const target = queryToTarget(q); const targetIndexMatcher = new TargetIndexMatcher(target); - const expectedIndex = targetIndexMatcher.buildTargetIndex(); - expect(targetIndexMatcher.servedByIndex(expectedIndex)).is.true; - expect(expectedIndex.fields.length >= targetGetSegmentCount(target)); + expect(targetIndexMatcher.hasMultipleInequality).is.false; + const actualIndex = targetIndexMatcher.buildTargetIndex(); + expect(actualIndex).is.not.null; + expect(targetIndexMatcher.servedByIndex(actualIndex as FieldIndex)).is + .true; + expect( + (actualIndex as FieldIndex).fields.length >= + targetGetSegmentCount(target) + ); } }); diff --git a/packages/firestore/test/util/helpers.ts b/packages/firestore/test/util/helpers.ts index 6cc59a5f779..344e4676777 100644 --- a/packages/firestore/test/util/helpers.ts +++ b/packages/firestore/test/util/helpers.ts @@ -227,7 +227,7 @@ export function path(path: string, offset?: number): ResourcePath { } export function field(path: string): FieldPath { - return new FieldPath(path.split('.')); + return FieldPath.fromServerFormat(path); } export function fieldIndex(