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(Firestore): SUM / AVG aggregation feature #6557

Merged
merged 8 commits into from
Nov 15, 2023

Conversation

yash30201
Copy link
Contributor

@yash30201 yash30201 commented Aug 11, 2023

Enables sum and average aggregation query.

Design doc: go/php-firestore-sum-avg

@product-auto-label product-auto-label bot added the api: firestore Issues related to the Firestore API. label Aug 11, 2023
@yash30201 yash30201 added do not merge Indicates a pull request not ready for merge, due to either quality or timing. status: In Progress The work on the issue has started. labels Aug 11, 2023
@yash30201 yash30201 marked this pull request as ready for review November 2, 2023 13:43
@yash30201 yash30201 requested review from a team as code owners November 2, 2023 13:43
Update aggregation type tests

Update aggregation query test

Updated AggregationQueryTests

Update transaction unit tests

Temp commit

Many Changes

* Added multi value type return support for aggregation queries
* Ammended Unit tests to conver this ammendment

Add check for multi type result

* Ammend to `getSnapshot` test so as to make sure the tests pass only if the logic allows any of the type to be returned.

Modify existing aggregation tests

to accomodate sum and avergate feature testing.

update trasaction aggregation tests
@yash30201 yash30201 removed do not merge Indicates a pull request not ready for merge, due to either quality or timing. status: In Progress The work on the issue has started. labels Nov 3, 2023
Copy link
Contributor

@vishwarajanand vishwarajanand left a comment

Choose a reason for hiding this comment

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

Overall changes and tests looks good to me.. Left a few nits.
Once those are closed, we can merge this.

Copy link
Contributor

@vishwarajanand vishwarajanand left a comment

Choose a reason for hiding this comment

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

Left some nits

Copy link
Contributor

@vishwarajanand vishwarajanand left a comment

Choose a reason for hiding this comment

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

Confirmed that the changes work locally. Lgtm

@yash30201 yash30201 merged commit 53a2795 into googleapis:main Nov 15, 2023
21 checks passed
@yash30201 yash30201 added the next release PRs to be included in the next release label Nov 29, 2023
@yash30201 yash30201 deleted the firestore-sum-avg branch May 8, 2024 06:32
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 Firestore API. next release PRs to be included in the next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants