diff --git a/common/api-review/firestore-lite.api.md b/common/api-review/firestore-lite.api.md index a96f3ada5cb..957216013d2 100644 --- a/common/api-review/firestore-lite.api.md +++ b/common/api-review/firestore-lite.api.md @@ -230,7 +230,7 @@ export abstract class QueryConstraint { } // @public -export type QueryConstraintType = 'where' | 'orderBy' | 'limit' | 'limitToLast' | 'startAt' | 'startAfter' | 'endAt' | 'endBefore'; +export type QueryConstraintType = 'where' | 'orderBy' | 'limit' | 'limitToLast' | 'startAt' | 'startAfter' | 'endAt' | 'endBefore' | 'and' | 'or'; // @public export class QueryDocumentSnapshot extends DocumentSnapshot { diff --git a/common/api-review/firestore.api.md b/common/api-review/firestore.api.md index db36b60482c..e13eb1dc716 100644 --- a/common/api-review/firestore.api.md +++ b/common/api-review/firestore.api.md @@ -359,7 +359,7 @@ export abstract class QueryConstraint { } // @public -export type QueryConstraintType = 'where' | 'orderBy' | 'limit' | 'limitToLast' | 'startAt' | 'startAfter' | 'endAt' | 'endBefore'; +export type QueryConstraintType = 'where' | 'orderBy' | 'limit' | 'limitToLast' | 'startAt' | 'startAfter' | 'endAt' | 'endBefore' | 'and' | 'or'; // @public export class QueryDocumentSnapshot extends DocumentSnapshot { diff --git a/packages/firestore/src/core/query.ts b/packages/firestore/src/core/query.ts index 2b4fca74c3a..eeabb93fb9b 100644 --- a/packages/firestore/src/core/query.ts +++ b/packages/firestore/src/core/query.ts @@ -24,16 +24,15 @@ import { Bound, canonifyTarget, Direction, - FieldFilter, Filter, newTarget, - Operator, OrderBy, boundSortsBeforeDocument, stringifyTarget, Target, targetEquals, - boundSortsAfterDocument + boundSortsAfterDocument, + CompositeFilter } from './target'; export const enum LimitType { @@ -166,6 +165,13 @@ 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 @@ -174,34 +180,12 @@ export function getFirstOrderByField(query: Query): FieldPath | null { export function getInequalityFilterField(query: Query): FieldPath | null { for (const filter of query.filters) { - debugAssert( - filter instanceof FieldFilter, - 'Only FieldFilters are supported' - ); - if (filter.isInequality()) { - return filter.field; + const result = filter.getFirstInequalityField(); + if (result !== null) { + return result; } } - return null; -} -/** - * Checks if any of the provided Operators are included in the query and - * returns the first one that is, or null if none are. - */ -export function findFilterOperator( - query: Query, - operators: Operator[] -): Operator | null { - for (const filter of query.filters) { - debugAssert( - filter instanceof FieldFilter, - 'Only FieldFilters are supported' - ); - if (operators.indexOf(filter.op) >= 0) { - return filter.op; - } - } return null; } @@ -337,11 +321,13 @@ export function queryToTarget(query: Query): Target { } export function queryWithAddedFilter(query: Query, filter: Filter): Query { + const newInequalityField = filter.getFirstInequalityField(); + const queryInequalityField = getInequalityFilterField(query); + debugAssert( - getInequalityFilterField(query) == null || - !(filter instanceof FieldFilter) || - !filter.isInequality() || - filter.field.isEqual(getInequalityFilterField(query)!), + queryInequalityField == null || + newInequalityField == null || + newInequalityField.isEqual(queryInequalityField), 'Query must only have one inequality field.' ); diff --git a/packages/firestore/src/core/target.ts b/packages/firestore/src/core/target.ts index 72a760a4529..8f6a78fbb9a 100644 --- a/packages/firestore/src/core/target.ts +++ b/packages/firestore/src/core/target.ts @@ -536,6 +536,12 @@ export function targetGetSegmentCount(target: Target): number { export abstract class Filter { abstract matches(doc: Document): boolean; + + abstract getFlattenedFilters(): readonly FieldFilter[]; + + abstract getFilters(): readonly Filter[]; + + abstract getFirstInequalityField(): FieldPath | null; } export const enum Operator { @@ -551,6 +557,11 @@ export const enum Operator { ARRAY_CONTAINS_ANY = 'array-contains-any' } +export const enum CompositeOperator { + OR = 'or', + AND = 'and' +} + /** * The direction of sorting in an order by. */ @@ -559,11 +570,12 @@ export const enum Direction { DESCENDING = 'desc' } +// TODO(orquery) move Filter classes to a new file, e.g. filter.ts export class FieldFilter extends Filter { protected constructor( - public field: FieldPath, - public op: Operator, - public value: ProtoValue + public readonly field: FieldPath, + public readonly op: Operator, + public readonly value: ProtoValue ) { super(); } @@ -685,21 +697,117 @@ export class FieldFilter extends Filter { ].indexOf(this.op) >= 0 ); } + + getFlattenedFilters(): readonly FieldFilter[] { + return [this]; + } + + getFilters(): readonly Filter[] { + return [this]; + } + + getFirstInequalityField(): FieldPath | null { + if (this.isInequality()) { + return this.field; + } + return null; + } +} + +export class CompositeFilter extends Filter { + private memoizedFlattenedFilters: FieldFilter[] | null = null; + + protected constructor( + public readonly filters: readonly Filter[], + public readonly op: CompositeOperator + ) { + super(); + } + + /** + * Creates a filter based on the provided arguments. + */ + static create(filters: Filter[], op: CompositeOperator): CompositeFilter { + return new CompositeFilter(filters, op); + } + + matches(doc: Document): boolean { + if (this.isConjunction()) { + // For conjunctions, all filters must match, so return false if any filter doesn't match. + return this.filters.find(filter => !filter.matches(doc)) === undefined; + } else { + // For disjunctions, at least one filter should match. + return this.filters.find(filter => filter.matches(doc)) !== undefined; + } + } + + getFlattenedFilters(): readonly FieldFilter[] { + if (this.memoizedFlattenedFilters !== null) { + return this.memoizedFlattenedFilters; + } + + this.memoizedFlattenedFilters = this.filters.reduce((result, subfilter) => { + return result.concat(subfilter.getFlattenedFilters()); + }, [] as FieldFilter[]); + + return this.memoizedFlattenedFilters; + } + + getFilters(): readonly Filter[] { + return 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; + } + + isConjunction(): boolean { + return this.op === CompositeOperator.AND; + } } export function canonifyFilter(filter: Filter): string { debugAssert( - filter instanceof FieldFilter, - 'canonifyFilter() only supports FieldFilters' - ); - // TODO(b/29183165): Technically, this won't be unique if two values have - // the same description, such as the int 3 and the string "3". So we should - // add the types in here somehow, too. - return ( - filter.field.canonicalString() + - filter.op.toString() + - canonicalId(filter.value) + filter instanceof FieldFilter || filter instanceof CompositeFilter, + 'canonifyFilter() only supports FieldFilters and CompositeFilters' ); + + if (filter instanceof FieldFilter) { + // TODO(b/29183165): Technically, this won't be unique if two values have + // the same description, such as the int 3 and the string "3". So we should + // add the types in here somehow, too. + return ( + filter.field.canonicalString() + + filter.op.toString() + + canonicalId(filter.value) + ); + } else { + // filter instanceof CompositeFilter + const canonicalIdsString = filter.filters + .map(filter => canonifyFilter(filter)) + .join(','); + return `${filter.op}(${canonicalIdsString})`; + } } export function filterEquals(f1: Filter, f2: Filter): boolean { diff --git a/packages/firestore/src/lite-api/query.ts b/packages/firestore/src/lite-api/query.ts index 0a762918b33..14b657d923f 100644 --- a/packages/firestore/src/lite-api/query.ts +++ b/packages/firestore/src/lite-api/query.ts @@ -19,7 +19,6 @@ import { getModularInstance } from '@firebase/util'; import { DatabaseId } from '../core/database_info'; import { - findFilterOperator, getFirstOrderByField, getInequalityFilterField, isCollectionGroupQuery, @@ -38,7 +37,9 @@ import { FieldFilter, Filter, Operator, - OrderBy + CompositeOperator, + OrderBy, + CompositeFilter } from '../core/target'; import { Document } from '../model/document'; import { DocumentKey } from '../model/document_key'; @@ -46,7 +47,6 @@ import { FieldPath as InternalFieldPath, ResourcePath } from '../model/path'; import { isServerTimestamp } from '../model/server_timestamps'; import { refValue } from '../model/values'; import { Value as ProtoValue } from '../protos/firestore_proto_api'; -import { debugAssert } from '../util/assert'; import { Code, FirestoreError } from '../util/error'; import { validatePositiveNumber, @@ -85,7 +85,9 @@ export type QueryConstraintType = | 'startAt' | 'startAfter' | 'endAt' - | 'endBefore'; + | 'endBefore' + | 'and' + | 'or'; /** * A `QueryConstraint` is used to narrow the set of documents returned by a @@ -137,6 +139,16 @@ class QueryFilterConstraint extends QueryConstraint { } _apply(query: Query): Query { + const filter = this._parse(query); + validateNewFieldFilter(query._query, filter); + return new Query( + query.firestore, + query.converter, + queryWithAddedFilter(query._query, filter) + ); + } + + _parse(query: Query): FieldFilter { const reader = newUserDataReader(query.firestore); const filter = newQueryFilter( query._query, @@ -147,11 +159,7 @@ class QueryFilterConstraint extends QueryConstraint { this._op, this._value ); - return new Query( - query.firestore, - query.converter, - queryWithAddedFilter(query._query, filter) - ); + return filter; } } @@ -181,7 +189,7 @@ export type WhereFilterOp = * @param opStr - The operation string (e.g "<", "<=", "==", "<", * "<=", "!="). * @param value - The value for comparison - * @returns The created {@link Query}. + * @returns The created {@link QueryConstraint}. */ export function where( fieldPath: string | FieldPath, @@ -193,6 +201,92 @@ export function where( return new QueryFilterConstraint(field, op, value); } +class QueryCompositeFilterConstraint extends QueryConstraint { + constructor( + readonly type: 'or' | 'and', + private readonly _queryConstraints: QueryFilterConstraint[] + ) { + super(); + } + + _parse(query: Query): Filter { + const parsedFilters = this._queryConstraints + .map(queryConstraint => { + return queryConstraint._parse(query); + }) + .filter(parsedFilter => parsedFilter.getFilters().length > 0); + + if (parsedFilters.length === 1) { + return parsedFilters[0]; + } + + return CompositeFilter.create(parsedFilters, this.getOperator()); + } + + _apply(query: Query): Query { + const parsedFilter = this._parse(query); + if (parsedFilter.getFilters().length === 0) { + // Return the existing query if not adding any more filters (e.g. an empty composite filter). + return query; + } + validateNewFilter(query._query, parsedFilter); + + return new Query( + query.firestore, + query.converter, + queryWithAddedFilter(query._query, parsedFilter) + ); + } + + getQueryConstraints(): QueryConstraint[] { + return this._queryConstraints; + } + + getOperator(): CompositeOperator { + return this.type === 'and' ? CompositeOperator.AND : CompositeOperator.OR; + } +} + +/** + * Creates a {@link QueryConstraint} that performs a logical OR + * of all the provided `queryConstraints` + * + * @param queryConstraints - Optional. The {@link queryConstraints} for OR operation. These must be + * created with calls to {@link where}, {@link or}, or {@link and}. + * @returns The created {@link QueryConstraint}. + */ +export function or(...queryConstraints: QueryConstraint[]): QueryConstraint { + // Only support QueryFilterConstraints + queryConstraints.forEach(queryConstraint => + validateQueryFilterConstraint('or', queryConstraint) + ); + + return new QueryCompositeFilterConstraint( + CompositeOperator.OR, + queryConstraints as QueryFilterConstraint[] + ); +} + +/** + * Creates a {@link QueryConstraint} that performs a logical AND + * of all the provided `queryConstraints` + * + * @param queryConstraints - Optional. The {@link queryConstraints} for AND operation. These must be + * created with calls to {@link where}, {@link or}, or {@link and}. + * @returns The created {@link QueryConstraint}. + */ +export function and(...queryConstraints: QueryConstraint[]): QueryConstraint { + // Only support QueryFilterConstraints + queryConstraints.forEach(queryConstraint => + validateQueryFilterConstraint('and', queryConstraint) + ); + + return new QueryCompositeFilterConstraint( + CompositeOperator.AND, + queryConstraints as QueryFilterConstraint[] + ); +} + class QueryOrderByConstraint extends QueryConstraint { readonly type = 'orderBy'; @@ -518,7 +612,6 @@ export function newQueryFilter( ); } const filter = FieldFilter.create(fieldPath, op, fieldValue); - validateNewFilter(query, filter); return filter; } @@ -792,46 +885,84 @@ function conflictingOps(op: Operator): Operator[] { } } -function validateNewFilter(query: InternalQuery, filter: Filter): void { - debugAssert(filter instanceof FieldFilter, 'Only FieldFilters are supported'); +function validateNewFieldFilter( + query: InternalQuery, + fieldFilter: FieldFilter +): void { + if (fieldFilter.isInequality()) { + const existingInequality = getInequalityFilterField(query); + const newInequality = fieldFilter.field; - if (filter.isInequality()) { - const existingField = getInequalityFilterField(query); - if (existingField !== null && !existingField.isEqual(filter.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 '${existingField.toString()}'` + - ` and '${filter.field.toString()}'` + ` inequality filters on '${existingInequality.toString()}'` + + ` and '${newInequality.toString()}'` ); } const firstOrderByField = getFirstOrderByField(query); if (firstOrderByField !== null) { - validateOrderByAndInequalityMatch(query, filter.field, firstOrderByField); + validateOrderByAndInequalityMatch( + query, + newInequality, + firstOrderByField + ); } } - const conflictingOp = findFilterOperator(query, conflictingOps(filter.op)); + const conflictingOp = findOpInsideFilters( + query.filters, + conflictingOps(fieldFilter.op) + ); if (conflictingOp !== null) { // Special case when it's a duplicate op to give a slightly clearer error message. - if (conflictingOp === filter.op) { + if (conflictingOp === fieldFilter.op) { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid query. You cannot use more than one ' + - `'${filter.op.toString()}' filter.` + `'${fieldFilter.op.toString()}' filter.` ); } else { throw new FirestoreError( Code.INVALID_ARGUMENT, - `Invalid query. You cannot use '${filter.op.toString()}' filters ` + + `Invalid query. You cannot use '${fieldFilter.op.toString()}' filters ` + `with '${conflictingOp.toString()}' filters.` ); } } } +function validateNewFilter(query: InternalQuery, filter: Filter): void { + let testQuery = query; + const subFilters = filter.getFlattenedFilters(); + for (const subFilter of subFilters) { + validateNewFieldFilter(testQuery, subFilter); + testQuery = queryWithAddedFilter(testQuery, subFilter); + } +} + +// Checks if any of the provided filter operators are included in the given list of filters and +// returns the first one that is, or null if none are. +function findOpInsideFilters( + filters: Filter[], + operators: Operator[] +): Operator | null { + for (const filter of filters) { + for (const fieldFilter of filter.getFlattenedFilters()) { + if (operators.indexOf(fieldFilter.op) >= 0) { + return fieldFilter.op; + } + } + } + return null; +} + function validateNewOrderBy(query: InternalQuery, orderBy: OrderBy): void { if (getFirstOrderByField(query) === null) { // This is the first order by. It must match any inequality. @@ -858,3 +989,15 @@ function validateOrderByAndInequalityMatch( ); } } + +export function validateQueryFilterConstraint( + functionName: string, + queryConstraint: QueryConstraint +): void { + if (!(queryConstraint instanceof QueryFilterConstraint)) { + throw new FirestoreError( + Code.INVALID_ARGUMENT, + `Function ${functionName}() requires QueryContraints created with a call to 'where(...)'.` + ); + } +} diff --git a/packages/firestore/src/local/query_engine.ts b/packages/firestore/src/local/query_engine.ts index f6b89aed66d..d968155363c 100644 --- a/packages/firestore/src/local/query_engine.ts +++ b/packages/firestore/src/local/query_engine.ts @@ -19,6 +19,7 @@ import { LimitType, newQueryComparator, Query, + queryContainsCompositeFilters, queryMatches, queryMatchesAllDocuments, queryToTarget, @@ -138,7 +139,10 @@ export class QueryEngine { return PersistencePromise.resolve(null); } - if (queryMatchesAllDocuments(query)) { + if ( + queryMatchesAllDocuments(query) || + queryContainsCompositeFilters(query) + ) { // Queries that match all documents don't benefit from using // key-based lookups. It is more efficient to scan all documents in a // collection, rather than to perform individual lookups. @@ -227,7 +231,10 @@ export class QueryEngine { remoteKeys: DocumentKeySet, lastLimboFreeSnapshotVersion: SnapshotVersion ): PersistencePromise { - if (queryMatchesAllDocuments(query)) { + if ( + queryMatchesAllDocuments(query) || + queryContainsCompositeFilters(query) + ) { // Queries that match all documents don't benefit from using // key-based lookups. It is more efficient to scan all documents in a // collection, rather than to perform individual lookups.