Skip to content

Collection Group Queries w/ indexing #1440

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.

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 26 commits into from
Mar 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
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
14 changes: 14 additions & 0 deletions packages/firebase/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6006,6 +6006,20 @@ declare namespace firebase.firestore {
*/
doc(documentPath: string): DocumentReference;

// TODO(b/116617988): Uncomment method and change jsdoc comment to "/**"
// once backend support is ready.
/*
* Creates and returns a new Query that includes all documents in the
* database that are contained in a collection or subcollection with the
* given collectionId.
*
* @param collectionId Identifies the collections to query over. Every
* collection or subcollection with this ID as the last segment of its path
* will be included. Cannot contain a slash.
* @return The created Query.
*/
//collectionGroup(collectionId: string): Query;

/**
* Executes the given `updateFunction` and then attempts to commit the changes
* applied within the transaction. If any document read within the transaction
Expand Down
14 changes: 14 additions & 0 deletions packages/firestore-types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,20 @@ export class FirebaseFirestore {
*/
doc(documentPath: string): DocumentReference;

// TODO(b/116617988): Uncomment method and change jsdoc comment to "/**"
// once backend support is ready.
/*
* Creates and returns a new Query that includes all documents in the
* database that are contained in a collection or subcollection with the
* given collectionId.
*
* @param collectionId Identifies the collections to query over. Every
* collection or subcollection with this ID as the last segment of its path
* will be included. Cannot contain a slash.
* @return The created Query.
*/
//collectionGroup(collectionId: string): Query;

/**
* Executes the given updateFunction and then attempts to commit the
* changes applied within the transaction. If any document read within the
Expand Down
81 changes: 58 additions & 23 deletions packages/firestore/src/api/database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -486,28 +486,38 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
collection(pathString: string): firestore.CollectionReference {
validateExactNumberOfArgs('Firestore.collection', arguments, 1);
validateArgType('Firestore.collection', 'non-empty string', 1, pathString);
if (!pathString) {
throw new FirestoreError(
Code.INVALID_ARGUMENT,
'Must provide a non-empty collection path to collection()'
);
}

this.ensureClientConfigured();
return new CollectionReference(ResourcePath.fromString(pathString), this);
}

doc(pathString: string): firestore.DocumentReference {
validateExactNumberOfArgs('Firestore.doc', arguments, 1);
validateArgType('Firestore.doc', 'non-empty string', 1, pathString);
if (!pathString) {
this.ensureClientConfigured();
return DocumentReference.forPath(ResourcePath.fromString(pathString), this);
}

// TODO(b/116617988): Fix name, uncomment d.ts definitions, and update CHANGELOG.md.
_collectionGroup(collectionId: string): firestore.Query {
validateExactNumberOfArgs('Firestore.collectionGroup', arguments, 1);
validateArgType(
'Firestore.collectionGroup',
'non-empty string',
1,
collectionId
);
if (collectionId.indexOf('/') >= 0) {
throw new FirestoreError(
Code.INVALID_ARGUMENT,
'Must provide a non-empty document path to doc()'
`Invalid collection ID '${collectionId}' passed to function ` +
`Firestore.collectionGroup(). Collection IDs must not contain '/'.`
);
}
this.ensureClientConfigured();
return DocumentReference.forPath(ResourcePath.fromString(pathString), this);
return new Query(
new InternalQuery(ResourcePath.EMPTY_PATH, collectionId),
this
);
}

runTransaction<T>(
Expand Down Expand Up @@ -1353,25 +1363,36 @@ export class Query implements firestore.Query {
);
}
if (typeof value === 'string') {
if (value.indexOf('/') !== -1) {
// TODO(dimond): Allow slashes once ancestor queries are supported
if (value === '') {
throw new FirestoreError(
Code.INVALID_ARGUMENT,
'Function Query.where() requires its third parameter to be a ' +
'valid document ID if the first parameter is ' +
'FieldPath.documentId(), but it contains a slash.'
'FieldPath.documentId(), but it was an empty string.'
);
}
if (value === '') {
if (
!this._query.isCollectionGroupQuery() &&
value.indexOf('/') !== -1
) {
throw new FirestoreError(
Code.INVALID_ARGUMENT,
'Function Query.where() requires its third parameter to be a ' +
'valid document ID if the first parameter is ' +
'FieldPath.documentId(), but it was an empty string.'
`Invalid third parameter to Query.where(). When querying a collection by ` +
`FieldPath.documentId(), the value provided must be a plain document ID, but ` +
`'${value}' contains a slash.`
);
}
const path = this._query.path.child(ResourcePath.fromString(value));
if (!DocumentKey.isDocumentKey(path)) {
throw new FirestoreError(
Code.INVALID_ARGUMENT,
`Invalid third parameter to Query.where(). When querying a collection group by ` +
`FieldPath.documentId(), the value provided must result in a valid document path, ` +
`but '${path}' is not because it has an odd number of segments (${
path.length
}).`
);
}
const path = this._query.path.child(new ResourcePath([value]));
assert(path.length % 2 === 0, 'Path should be a document key');
fieldValue = new RefValue(
this.firestore._databaseId,
new DocumentKey(path)
Expand Down Expand Up @@ -1636,14 +1657,28 @@ export class Query implements firestore.Query {
`${methodName}(), but got a ${typeof rawValue}`
);
}
if (rawValue.indexOf('/') !== -1) {
if (
!this._query.isCollectionGroupQuery() &&
rawValue.indexOf('/') !== -1
) {
throw new FirestoreError(
Code.INVALID_ARGUMENT,
`Invalid query. When querying a collection and ordering by FieldPath.documentId(), ` +
`the value passed to ${methodName}() must be a plain document ID, but ` +
`'${rawValue}' contains a slash.`
);
}
const path = this._query.path.child(ResourcePath.fromString(rawValue));
if (!DocumentKey.isDocumentKey(path)) {
throw new FirestoreError(
Code.INVALID_ARGUMENT,
`Invalid query. Document ID '${rawValue}' contains a slash in ` +
`${methodName}()`
`Invalid query. When querying a collection group and ordering by ` +
`FieldPath.documentId(), the value passed to ${methodName}() must result in a ` +
`valid document path, but '${path}' is not because it contains an odd number ` +
`of segments.`
);
}
const key = new DocumentKey(this._query.path.child(rawValue));
const key = new DocumentKey(path);
components.push(new RefValue(this.firestore._databaseId, key));
} else {
const wrapped = this.firestore._dataConverter.parseQueryValue(
Expand Down
75 changes: 60 additions & 15 deletions packages/firestore/src/core/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,13 @@ export class Query {
private memoizedCanonicalId: string | null = null;
private memoizedOrderBy: OrderBy[] | null = null;

/**
* Initializes a Query with a path and optional additional query constraints.
* Path must currently be empty if this is a collection group query.
*/
constructor(
readonly path: ResourcePath,
readonly collectionGroup: string | null = null,
readonly explicitOrderBy: OrderBy[] = [],
readonly filters: Filter[] = [],
readonly limit: number | null = null,
Expand Down Expand Up @@ -111,13 +116,12 @@ export class Query {
'Query must only have one inequality field.'
);

assert(
!DocumentKey.isDocumentKey(this.path),
'No filtering allowed for document query'
);
assert(!this.isDocumentQuery(), 'No filtering allowed for document query');

const newFilters = this.filters.concat([filter]);
return new Query(
this.path,
this.collectionGroup,
this.explicitOrderBy.slice(),
newFilters,
this.limit,
Expand All @@ -127,15 +131,12 @@ export class Query {
}

addOrderBy(orderBy: OrderBy): Query {
assert(
!DocumentKey.isDocumentKey(this.path),
'No ordering allowed for document query'
);
assert(!this.startAt && !this.endAt, 'Bounds must be set after orderBy');
// TODO(dimond): validate that orderBy does not list the same key twice.
const newOrderBy = this.explicitOrderBy.concat([orderBy]);
return new Query(
this.path,
this.collectionGroup,
newOrderBy,
this.filters.slice(),
this.limit,
Expand All @@ -147,6 +148,7 @@ export class Query {
withLimit(limit: number | null): Query {
return new Query(
this.path,
this.collectionGroup,
this.explicitOrderBy.slice(),
this.filters.slice(),
limit,
Expand All @@ -158,6 +160,7 @@ export class Query {
withStartAt(bound: Bound): Query {
return new Query(
this.path,
this.collectionGroup,
this.explicitOrderBy.slice(),
this.filters.slice(),
this.limit,
Expand All @@ -169,6 +172,7 @@ export class Query {
withEndAt(bound: Bound): Query {
return new Query(
this.path,
this.collectionGroup,
this.explicitOrderBy.slice(),
this.filters.slice(),
this.limit,
Expand All @@ -177,12 +181,33 @@ export class Query {
);
}

/**
* Helper to convert a collection group query into a collection query at a
* specific path. This is used when executing collection group queries, since
* we have to split the query into a set of collection queries at multiple
* paths.
*/
asCollectionQueryAtPath(path: ResourcePath): Query {
return new Query(
path,
/*collectionGroup=*/ null,
this.explicitOrderBy.slice(),
this.filters.slice(),
this.limit,
this.startAt,
this.endAt
);
}

// TODO(b/29183165): This is used to get a unique string from a query to, for
// example, use as a dictionary key, but the implementation is subject to
// collisions. Make it collision-free.
canonicalId(): string {
if (this.memoizedCanonicalId === null) {
let canonicalId = this.path.canonicalString();
if (this.isCollectionGroupQuery()) {
canonicalId += '|cg:' + this.collectionGroup;
}
canonicalId += '|f:';
for (const filter of this.filters) {
canonicalId += filter.canonicalId();
Expand Down Expand Up @@ -213,6 +238,9 @@ export class Query {

toString(): string {
let str = 'Query(' + this.path.canonicalString();
if (this.isCollectionGroupQuery()) {
str += ' collectionGroup=' + this.collectionGroup;
}
if (this.filters.length > 0) {
str += `, filters: [${this.filters.join(', ')}]`;
}
Expand Down Expand Up @@ -257,6 +285,10 @@ export class Query {
}
}

if (this.collectionGroup !== other.collectionGroup) {
return false;
}

if (!this.path.isEqual(other.path)) {
return false;
}
Expand Down Expand Up @@ -291,7 +323,7 @@ export class Query {

matches(doc: Document): boolean {
return (
this.matchesAncestor(doc) &&
this.matchesPathAndCollectionGroup(doc) &&
this.matchesOrderBy(doc) &&
this.matchesFilters(doc) &&
this.matchesBounds(doc)
Expand Down Expand Up @@ -328,19 +360,32 @@ export class Query {
}

isDocumentQuery(): boolean {
return DocumentKey.isDocumentKey(this.path) && this.filters.length === 0;
return (
DocumentKey.isDocumentKey(this.path) &&
this.collectionGroup === null &&
this.filters.length === 0
);
}

isCollectionGroupQuery(): boolean {
return this.collectionGroup !== null;
}

private matchesAncestor(doc: Document): boolean {
private matchesPathAndCollectionGroup(doc: Document): boolean {
const docPath = doc.key.path;
if (DocumentKey.isDocumentKey(this.path)) {
if (this.collectionGroup !== null) {
// NOTE: this.path is currently always empty since we don't expose Collection
// Group queries rooted at a document path yet.
return (
doc.key.hasCollectionId(this.collectionGroup) &&
this.path.isPrefixOf(docPath)
);
} else if (DocumentKey.isDocumentKey(this.path)) {
// exact match for document queries
return this.path.isEqual(docPath);
} else {
// shallow ancestor queries by default
return (
this.path.isPrefixOf(docPath) && this.path.length === docPath.length - 1
);
return this.path.isImmediateParentOf(docPath);
}
}

Expand Down
51 changes: 51 additions & 0 deletions packages/firestore/src/local/index_manager.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/**
* Copyright 2019 Google Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { ResourcePath } from '../model/path';
import { PersistenceTransaction } from './persistence';
import { PersistencePromise } from './persistence_promise';

/**
* Represents a set of indexes that are used to execute queries efficiently.
*
* Currently the only index is a [collection id] => [parent path] index, used
* to execute Collection Group queries.
*/
export interface IndexManager {
/**
* Creates an index entry mapping the collectionId (last segment of the path)
* to the parent path (either the containing document location or the empty
* path for root-level collections). Index entries can be retrieved via
* getCollectionParents().
*
* NOTE: Currently we don't remove index entries. If this ends up being an
* issue we can devise some sort of GC strategy.
*/
addToCollectionParentIndex(
transaction: PersistenceTransaction,
collectionPath: ResourcePath
): PersistencePromise<void>;

/**
* Retrieves all parent locations containing the given collectionId, as a
* list of paths (each path being either a document location or the empty
* path for a root-level collection).
*/
getCollectionParents(
transaction: PersistenceTransaction,
collectionId: string
): PersistencePromise<ResourcePath[]>;
}
Loading