-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
fix(api-gateway): fixes an issue where queries t… #8060
Conversation
…o get the total count of results were incorrectly applying sorting from the original query and also were getting default ordering applied when the query ordering was stripped out
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 8 Ignored Deployments
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #8060 +/- ##
==========================================
+ Coverage 47.90% 47.92% +0.02%
==========================================
Files 154 154
Lines 21034 21035 +1
Branches 5422 5422
==========================================
+ Hits 10077 10082 +5
+ Misses 10203 10199 -4
Partials 754 754
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Thanks for your contribution, @rdwoodring! I've assigned this to @paveltiunov so he can do a review. |
@paveltiunov any idea when you'll get a chance to review this or get it pulled in? I've been running on a fork in production and would ideally like to see this pulled so I can get back onto the official images |
@rdwoodring Thanks for contributing! We'd need E2E tests for that. You can put it in the Postgres smoke test. |
Any specific tests you'd like to see, @paveltiunov ? Just that queries with Thanks |
@rdwoodring The later. We need a test to see it doesn't have an order by and executes in db. |
Could we please get this PR reviewed ? I am facing issues in one of the project due to this and I had to execute multiple same queries just to get total number of records. |
fix(api-gateway) fix(schema-compiler): fixes an issue where queries to get the total count of results were incorrectly applying sorting from the original query and also were getting default ordering applied when the query ordering was stripped out
Check List
Issue Reference this PR resolves
#7446
Description of Changes Made (if issue reference is not provided)
order
field from queries that are getting a count of total results matchingorder
so that it does an early return and does not defaultorder
if the query is getting the total (follows the same path as the logic doing an early return for preaggreation queries)