-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Firestore: add support for CollectionGroup queries. #7758
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
Changes from all commits
564da9d
84a3684
bf36c61
796e59c
c79aba4
b606d88
10df4e9
89b97b4
4a643a2
e699f33
3d9b901
115234c
8925d62
2689108
1223f34
412aade
7297f10
2ee7bbb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -111,6 +111,10 @@ class Query(object): | |
| any matching documents will be included in the result set. | ||
| When the query is formed, the document values | ||
| will be used in the order given by ``orders``. | ||
| all_descendants (Optional[bool]): When false, selects only collections | ||
| that are immediate children of the `parent` specified in the | ||
| containing `RunQueryRequest`. When true, selects all descendant | ||
| collections. | ||
| """ | ||
|
|
||
| ASCENDING = "ASCENDING" | ||
|
|
@@ -128,6 +132,7 @@ def __init__( | |
| offset=None, | ||
| start_at=None, | ||
| end_at=None, | ||
| all_descendants=False, | ||
| ): | ||
| self._parent = parent | ||
| self._projection = projection | ||
|
|
@@ -137,6 +142,7 @@ def __init__( | |
| self._offset = offset | ||
| self._start_at = start_at | ||
| self._end_at = end_at | ||
| self._all_descendants = all_descendants | ||
|
|
||
| def __eq__(self, other): | ||
| if not isinstance(other, self.__class__): | ||
|
|
@@ -150,6 +156,7 @@ def __eq__(self, other): | |
| and self._offset == other._offset | ||
| and self._start_at == other._start_at | ||
| and self._end_at == other._end_at | ||
| and self._all_descendants == other._all_descendants | ||
| ) | ||
|
|
||
| @property | ||
|
|
@@ -203,6 +210,7 @@ def select(self, field_paths): | |
| offset=self._offset, | ||
| start_at=self._start_at, | ||
| end_at=self._end_at, | ||
| all_descendants=self._all_descendants, | ||
| ) | ||
|
|
||
| def where(self, field_path, op_string, value): | ||
|
|
@@ -270,6 +278,7 @@ def where(self, field_path, op_string, value): | |
| offset=self._offset, | ||
| start_at=self._start_at, | ||
| end_at=self._end_at, | ||
| all_descendants=self._all_descendants, | ||
| ) | ||
|
|
||
| @staticmethod | ||
|
|
@@ -321,6 +330,7 @@ def order_by(self, field_path, direction=ASCENDING): | |
| offset=self._offset, | ||
| start_at=self._start_at, | ||
| end_at=self._end_at, | ||
| all_descendants=self._all_descendants, | ||
| ) | ||
|
|
||
| def limit(self, count): | ||
|
|
@@ -346,6 +356,7 @@ def limit(self, count): | |
| offset=self._offset, | ||
| start_at=self._start_at, | ||
| end_at=self._end_at, | ||
| all_descendants=self._all_descendants, | ||
| ) | ||
|
|
||
| def offset(self, num_to_skip): | ||
|
|
@@ -372,6 +383,7 @@ def offset(self, num_to_skip): | |
| offset=num_to_skip, | ||
| start_at=self._start_at, | ||
| end_at=self._end_at, | ||
| all_descendants=self._all_descendants, | ||
| ) | ||
|
|
||
| def _cursor_helper(self, document_fields, before, start): | ||
|
|
@@ -418,6 +430,7 @@ def _cursor_helper(self, document_fields, before, start): | |
| "orders": self._orders, | ||
| "limit": self._limit, | ||
| "offset": self._offset, | ||
| "all_descendants": self._all_descendants, | ||
| } | ||
| if start: | ||
| query_kwargs["start_at"] = cursor_pair | ||
|
|
@@ -679,7 +692,7 @@ def _to_protobuf(self): | |
| "select": projection, | ||
| "from": [ | ||
| query_pb2.StructuredQuery.CollectionSelector( | ||
| collection_id=self._parent.id | ||
| collection_id=self._parent.id, all_descendants=self._all_descendants | ||
| ) | ||
| ], | ||
| "where": self._filters_pb(), | ||
|
|
@@ -739,9 +752,14 @@ def stream(self, transaction=None): | |
| ) | ||
|
|
||
| for response in response_iterator: | ||
| snapshot = _query_response_to_snapshot( | ||
| response, self._parent, expected_prefix | ||
| ) | ||
| if self._all_descendants: | ||
| snapshot = _collection_group_query_response_to_snapshot( | ||
| response, self._parent | ||
| ) | ||
| else: | ||
| snapshot = _query_response_to_snapshot( | ||
| response, self._parent, expected_prefix | ||
| ) | ||
| if snapshot is not None: | ||
| yield snapshot | ||
|
|
||
|
|
@@ -968,3 +986,32 @@ def _query_response_to_snapshot(response_pb, collection, expected_prefix): | |
| update_time=response_pb.document.update_time, | ||
| ) | ||
| return snapshot | ||
|
|
||
|
|
||
| def _collection_group_query_response_to_snapshot(response_pb, collection): | ||
| """Parse a query response protobuf to a document snapshot. | ||
|
|
||
| Args: | ||
| response_pb (google.cloud.proto.firestore.v1.\ | ||
| firestore_pb2.RunQueryResponse): A | ||
| collection (~.firestore_v1.collection.CollectionReference): A | ||
| reference to the collection that initiated the query. | ||
|
|
||
| Returns: | ||
| Optional[~.firestore.document.DocumentSnapshot]: A | ||
| snapshot of the data returned in the query. If ``response_pb.document`` | ||
| is not set, the snapshot will be :data:`None`. | ||
| """ | ||
| if not response_pb.HasField("document"): | ||
| return None | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does Python not return empty DocumentSnapshots for non-existing documents? In either case, this is a query and this condition should never trigger (fingers crossed).
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @schmidt-sebastian There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My bad: It looks like the expected behavior is that it can get skipped (see https://cs.corp.google.com/piper///depot/google3/google/firestore/v1beta1/firestore.proto?q=runqueryrequest+firestore&dr=CSs&l=619) |
||
| reference = collection._client.document(response_pb.document.name) | ||
| data = _helpers.decode_dict(response_pb.document.fields, collection._client) | ||
| snapshot = document.DocumentSnapshot( | ||
| reference, | ||
| data, | ||
| exists=True, | ||
| read_time=response_pb.read_time, | ||
| create_time=response_pb.document.create_time, | ||
| update_time=response_pb.document.update_time, | ||
| ) | ||
| return snapshot | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am a little surprised that this is a different method from
_query_response_to_snapshot.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was related to expectations built into this method. @tseaver made this to address that. It is possible we can combine them by refactoring that code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@schmidt-sebastian The implementation of
_query_response_to_snapshotmandates (via_helpers.get_doc_id) that the result document is a direct child of the query's parent collection. I'm unsure whether this restriction is "important", or just an artifact of prior development iterations. If it is an artifact, then we could just use the new implementation in_collection_group_query_response_to_snapshotfor all queries.