-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(firestore): literals pipeline stage #16028
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 23 commits
2a8213c
28acee9
a2e11ed
a97cb13
cc1b737
3861223
7bad13c
36e45bf
816498e
09f59f6
11c22ee
941101f
ccb159a
6aae503
fa30d5f
db98e44
4a85ee6
5627c75
0dbfc77
837e52c
fc2880d
a515903
eee827e
c4c8398
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 |
|---|---|---|
|
|
@@ -21,14 +21,15 @@ | |
|
|
||
| from abc import ABC, abstractmethod | ||
| from enum import Enum | ||
| from typing import TYPE_CHECKING, Optional, Sequence | ||
| from typing import TYPE_CHECKING, Any, Mapping, Optional, Sequence | ||
|
|
||
| from google.cloud.firestore_v1._helpers import encode_value | ||
| from google.cloud.firestore_v1.base_vector_query import DistanceMeasure | ||
| from google.cloud.firestore_v1.pipeline_expressions import ( | ||
| AggregateFunction, | ||
| AliasedExpression, | ||
| BooleanExpression, | ||
| CONSTANT_TYPE, | ||
| Expression, | ||
| Field, | ||
| Ordering, | ||
|
|
@@ -342,6 +343,26 @@ def _pb_args(self): | |
| return [Value(integer_value=self.limit)] | ||
|
|
||
|
|
||
| class Literals(Stage): | ||
| """Returns documents from a fixed set of predefined document objects.""" | ||
|
|
||
| def __init__(self, *documents: Mapping[str, Expression | CONSTANT_TYPE]): | ||
| super().__init__("literals") | ||
| self.documents: tuple[Mapping[str, Any], ...] = documents | ||
|
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. I'd prefer if we didn't use Any here, since this is a publicly exposed attribute We could resolve it using a TypeVar: But honestly, I'd recommend converting the constants at init time instead, which would both resolve this issue, raise conversion errors in a better place, and simplify the code:
Contributor
Author
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. I'm not sure if it's a good idea because encoding the values here could break the What do you think?
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. Sorry, I should have tested the code before posting. Value isn't an expression, so that wouldn't have passed mypy. This should though: This would result in the repr of |
||
|
|
||
| def _pb_args(self): | ||
| args = [] | ||
| for doc in self.documents: | ||
| encoded_doc = {} | ||
| for k, v in doc.items(): | ||
| if hasattr(v, "_to_pb"): | ||
| encoded_doc[k] = v._to_pb() | ||
| else: | ||
| encoded_doc[k] = encode_value(v) | ||
| args.append(Value(map_value={"fields": encoded_doc})) | ||
| return args | ||
|
|
||
|
|
||
| class Offset(Stage): | ||
| """Skips a specified number of documents.""" | ||
|
|
||
|
|
||
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.
nit: it would probably make sense to make this a dict too, to map the pipeline_source
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.
Thanks, I thought I fixed it. It's corrected now.