-
Notifications
You must be signed in to change notification settings - Fork 75
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
feat: Sum/Avg aggregation queries #715
Changes from 15 commits
608464a
7e4d335
f1322b5
ed247b9
b3d1d7a
eaac103
19c2c4b
20ad46e
c2a804d
59a02c2
91dbc48
3901dd7
dff8459
3e66842
ed06271
47d45da
90b006c
5f00661
2340fea
63f252a
109e8b8
85c8dbd
93ff4bb
e60aa75
6a0950a
e03871e
bb9e8bb
a488a6d
b258fde
6d3b154
4b6de1a
7b40aa7
c06aa19
176ce2d
1b16357
d42bf07
0e969a6
82bde42
9a10dc2
3bb4bf5
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 |
---|---|---|
|
@@ -507,6 +507,30 @@ def count(self, alias=None): | |
""" | ||
return self._aggregation_query().count(alias=alias) | ||
|
||
def sum(self, field_ref: str, alias=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 FieldPath need to be appended here as well? 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. good catch! Fixed (The type checking system should have caught this, but it looks like it's not running properly. I'll have to fix that in a separate PR) 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. sgtm, please file an issue for tracking! 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 have a bug for it here: #773 |
||
""" | ||
Adds a sum over the nested query. | ||
|
||
:type field_ref: str | ||
:param field_ref: The field_ref for the aggregation | ||
|
||
:type alias: str | ||
:param alias: (Optional) The alias for the aggregation | ||
""" | ||
return self._aggregation_query().sum(field_ref, alias=alias) | ||
|
||
def avg(self, field_ref: str, alias=None): | ||
""" | ||
Adds an avg over the nested query. | ||
|
||
:type field_ref: str | ||
:param field_ref: The field_ref for the aggregation | ||
|
||
:type alias: str | ||
:param alias: (Optional) The alias for the aggregation | ||
""" | ||
return self._aggregation_query().avg(field_ref, alias=alias) | ||
|
||
|
||
def _auto_id() -> str: | ||
"""Generate a "random" automatically generated ID. | ||
|
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.
similar to https://github.com/googleapis/java-firestore/pull/1387/files#diff-fdd9700384056ce568a3fd41266eb1dfc5487c620bade78cde24dd677d7016b5R83, should we also have a constructor that accepts a
FieldPath
?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.
Done. Although I used the same constructor with an additional accepted type, which is how Python handles this kind of thing