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(Datastore): Enable SUM / AVG Aggregations #6817

Merged
merged 8 commits into from
Dec 1, 2023

Conversation

yash30201
Copy link
Contributor

@yash30201 yash30201 commented Nov 26, 2023

Implements sum and average aggregations.

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

Depends on #6811

Fixes #6808

@product-auto-label product-auto-label bot added the api: datastore Issues related to the Datastore API. label Nov 26, 2023
1) Implement logic
2) Refactor and implement unit testing for existing unit tests
3) Refactor and implement system testing for existing system tests

Implement `NAN`, `INF`, `-INF` and `null` handling

1) Update entity mapper to parse requests and responses properly for
rest as well as grpc
2) Implement unit testing to enusre this feature works properly
   a) At client level by mocking
   b) At entity mapper by invoking
3) Implement system testing to ensure proper insertions and lookup
behaviour with NAN, null and +- INF's.

Test corner cases for aggregations.
fix(Datastore): Fix `INF` and `NAN` handling in rest client

nit fix

Final fix for NAN, null and INF handling
@yash30201 yash30201 added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. release blocking Required feature/issue must be fixed prior to next release. and removed release blocking Required feature/issue must be fixed prior to next release. labels Nov 26, 2023
@yash30201 yash30201 marked this pull request as ready for review November 28, 2023 12:26
@yash30201 yash30201 requested review from a team as code owners November 28, 2023 12:26
* For @vishwarajanand's reviews
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.

Lgtm, left some nits

@yash30201 yash30201 added the next release PRs to be included in the next release label Nov 30, 2023
@yash30201 yash30201 merged commit b43313e into googleapis:main Dec 1, 2023
21 checks passed
@yash30201 yash30201 deleted the datastore-sum-avg branch May 8, 2024 06:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: datastore Issues related to the Datastore API. next release PRs to be included in the next release type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Datastore: NAN and INF value throw error while parsing
3 participants