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.

Sign up for GitHub

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

Jump to bottom

fix: Implementing count with aggregation query #1782

Closed
wants to merge 4 commits into from
Closed

fix: Implementing count with aggregation query #1782

wants to merge 4 commits into from

Conversation

its-snorlax
Copy link
Contributor

Modifying the implementation of DatastoreTemplate#count to use recently introduced COUNT aggregation and Aggregation queries in datastore.

fix #1781

@its-snorlax its-snorlax force-pushed the count-with-aggregation-query branch from c69268f to eaab754 Compare April 28, 2023 12:05
@its-snorlax its-snorlax changed the title Implementing count with aggregation query fix: Implementing count with aggregation query Apr 28, 2023
@its-snorlax its-snorlax marked this pull request as ready for review May 4, 2023 06:16
@its-snorlax
Copy link
Contributor Author

Runtime comparison between old and new implementation
Reference - https://gist.github.com/its-snorlax/fd4fdcb931c991edc071e02822bcd3b3

# Entities Key Only Query Count duration (ms) Aggregation Query Count duration (ms)
~5k 829 221
~10k 1318 262
~20k 2140 297
~50k 4845 372
~100k 8532 417
~200k 18414 549
~500k 45527 663
~1m 81831 1055

meltsufin
meltsufin approved these changes May 5, 2023
View reviewed changes
Copy link
Member

@meltsufin meltsufin left a comment

Choose a reason for hiding this comment

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

LGTM. @jainsahab Please take a look as well.

@meltsufin meltsufin requested a review from jainsahab May 5, 2023 13:53
@sonarqubecloud
Copy link

sonarqubecloud bot commented May 5, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

jainsahab
jainsahab approved these changes May 7, 2023
View reviewed changes
Copy link
Contributor

@jainsahab jainsahab left a comment

Choose a reason for hiding this comment

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

LGTM

@its-snorlax
Copy link
Contributor Author

@meltsufin @jainsahab are we good to merge now?

renovate-bot pushed a commit to renovate-bot/GoogleCloudPlatform-_-spring-cloud-gcp that referenced this pull request May 8, 2023
@its-snorlax
fix: Implementing count with aggregation query (GoogleCloudPlatform#1782
1ee2244 
)

Modifying the implementation of DatastoreTemplate#count to use recently introduced [COUNT aggregation and Aggregation queries in datastore](https://cloud.google.com/datastore/docs/aggregation-queries).

Fixes. GoogleCloudPlatform#1781
@meltsufin
Copy link
Member

@meltsufin @jainsahab are we good to merge now?

It was merged, but due to some quirk in GitHub the PR didn't close.

@meltsufin meltsufin closed this May 8, 2023
@release-please release-please bot mentioned this pull request May 9, 2023
Merged
@meltsufin meltsufin mentioned this pull request May 20, 2023
Closed
@release-please release-please bot mentioned this pull request Dec 18, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@meltsufin meltsufin meltsufin approved these changes

@jainsahab jainsahab jainsahab approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Datastore: Using new datastore aggregation queries for count
3 participants
@its-snorlax @meltsufin @jainsahab