Skip to content
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

Merged
merged 40 commits into from
Oct 19, 2023
Merged

feat: Sum/Avg aggregation queries #715

merged 40 commits into from
Oct 19, 2023

Conversation

Mariatta
Copy link
Contributor

Adds the ability to perform sum/avg aggregation query through:

  • query.sum(),
  • query.avg(),
  • async_query.sum(),
  • async_query.avg()

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> 🦕

Adds the ability to perform sum/avg aggregation query through:
- query.sum(),
- query.avg(),
- async_query.sum(),
- async_query.avg()
@Mariatta Mariatta requested review from a team as code owners May 10, 2023 23:10
@product-auto-label product-auto-label bot added size: l Pull request size is large. api: firestore Issues related to the googleapis/python-firestore API. labels May 10, 2023
@daniel-sanche daniel-sanche added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 18, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 18, 2023
@product-auto-label product-auto-label bot added size: xl Pull request size is extra large. and removed size: l Pull request size is large. labels Aug 24, 2023
@daniel-sanche daniel-sanche added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 24, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 24, 2023
@daniel-sanche daniel-sanche added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 25, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 25, 2023
for aggregation_result in result:
assert aggregation_result.alias in ["total", "all"]


@firestore.transactional
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add some additional tests for transactions using sum/avg?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a couple, let me know what you think

@kolea2 kolea2 changed the title Feat: Sum/Avg Feature feat: Sum/Avg Feature Aug 29, 2023
@@ -507,6 +507,30 @@ def count(self, alias=None):
"""
return self._aggregation_query().count(alias=alias)

def sum(self, field_ref: str, alias=None):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does FieldPath need to be appended here as well?

Copy link
Contributor

Choose a reason for hiding this comment

The 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)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sgtm, please file an issue for tracking!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a bug for it here: #773

@daniel-sanche daniel-sanche added the kokoro:run Add this label to force Kokoro to re-run the tests. label Oct 10, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Oct 10, 2023
@daniel-sanche daniel-sanche added the kokoro:run Add this label to force Kokoro to re-run the tests. label Oct 11, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Oct 11, 2023
@daniel-sanche daniel-sanche added the kokoro:run Add this label to force Kokoro to re-run the tests. label Oct 11, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Oct 11, 2023
@daniel-sanche
Copy link
Contributor

daniel-sanche commented Oct 11, 2023

Note: the new *_with_start_at tests are expected to fail for now. PR should be blocked until they are ready

EDIT: we are going to release with tests skipped, and re-enable them when the backend fix is ready

@daniel-sanche daniel-sanche added the kokoro:run Add this label to force Kokoro to re-run the tests. label Oct 19, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Oct 19, 2023
@daniel-sanche daniel-sanche added the kokoro:run Add this label to force Kokoro to re-run the tests. label Oct 19, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Oct 19, 2023
@daniel-sanche daniel-sanche merged commit 443475b into main Oct 19, 2023
16 checks passed
@daniel-sanche daniel-sanche deleted the feat-sum-avg-pr branch October 19, 2023 23:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: firestore Issues related to the googleapis/python-firestore API. size: xl Pull request size is extra large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants