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

Improve SQL Performance by Eliminating Unnecessary Order #3352

Merged
merged 1 commit into from
Aug 2, 2023

Conversation

FloThinksPi
Copy link
Member

@FloThinksPi FloThinksPi commented Jul 21, 2023

This commit corrects and improves SQL queries by avoiding redundant orders. It was previously observed that SQL queries were being ordered both by ID and GUID when the default order was applied. The paginator was also adding a secondary sort, introduced to offset issues encountered when using 'created_at' or 'updated_at' as sorting parameters, as the order was not deterministic. However, for default sorting by ID, this was superfluous.

With this change, the paginator now adds the secondary sort to the SQL query only if the order by parameter is not the ID column, in order to ensure fixed order for all column values. This alteration aims to enhance query performance across all listing endpoints.

The change involves a schema update that aligns all related indices in the database. A combined index with the GUID column is created with the understanding that databases can use such combined indices as a replacement for the most left column in the index. For instance, an index over (updated_at, GUID) can be utilized by the database, providing enhanced performance either when querying this table with both fields in the where clause, or when ordering by both columns.

The primary and secondary ordering now use the same direction, an improvement from the previous method where order was only applied to primary sorting. This prevents the need for separate indices for ascending and descending orders, resulting in a system where descending order is the exact opposite of ascending order.

  • An explanation of the use cases your change solves

Performance for end users and platform stability by being able to handle more load.

  • Links to any other associated PRs

  • I have reviewed the contributing guide

  • I have viewed, signed, and submitted the Contributor License Agreement

  • I have made this pull request to the main branch

  • I have run all the unit tests using bundle exec rake

  • I have run CF Acceptance Tests

@FloThinksPi FloThinksPi added performance PoC Issue/PR that is part of a PoC to try out new things labels Jul 21, 2023
@FloThinksPi FloThinksPi force-pushed the improve_pagination_order_performance branch 5 times, most recently from 62fbba5 to 6d4d8f6 Compare July 26, 2023 13:06
@FloThinksPi FloThinksPi removed the PoC Issue/PR that is part of a PoC to try out new things label Jul 26, 2023
@FloThinksPi FloThinksPi force-pushed the improve_pagination_order_performance branch 3 times, most recently from b065fdd to 9efd226 Compare July 27, 2023 08:52
@FloThinksPi FloThinksPi self-assigned this Jul 27, 2023
@FloThinksPi FloThinksPi requested a review from philippthun July 27, 2023 09:11
@FloThinksPi FloThinksPi force-pushed the improve_pagination_order_performance branch 4 times, most recently from 62d3d91 to db52e38 Compare July 27, 2023 12:29
@philippthun
Copy link
Member

Additionally when sorting by created_at we can also just sort by id sice this would sort by created_at automatically and maintain a fixed order since ids are uniqe.

That's not part of the PR (anymore). Can you adapt the commit message as well as the PR description?

This commit corrects and improves SQL queries by avoiding redundant orders. It was previously observed that SQL queries were being ordered both by ID and GUID when the default order was applied. The paginator was also adding a secondary sort, introduced to offset issues encountered when using 'created_at' or 'updated_at' as sorting parameters, as the order was not deterministic. However, for default sorting by ID, this was superfluous.

With this change, the paginator now adds the secondary sort to the SQL query only if the order by parameter is not the ID column, in order to ensure fixed order for all column values. This alteration aims to enhance query performance across all listing endpoints.

The change involves a schema update that aligns all related indices in the database. A combined index with the GUID column is created with the understanding that databases can use such combined indices as a replacement for the most left column in the index. For instance, an index over (updated_at, GUID) can be utilized by the database, providing enhanced performance either when querying this table with both fields in the where clause, or when ordering by both columns.

The primary and secondary ordering now use the same direction, an improvement from the previous method where order was only applied to primary sorting. This prevents the need for separate indices for ascending and descending orders, resulting in a system where descending order is the exact opposite of ascending order.

Co-authored-by: Johannes Haass <[email protected]>
@FloThinksPi FloThinksPi force-pushed the improve_pagination_order_performance branch from 404fd2c to 81a17f9 Compare August 1, 2023 09:22
@FloThinksPi FloThinksPi changed the title Improve sql performance by removing superfluous order Improve SQL Performance by Eliminating Unnecessary Order Aug 1, 2023
@FloThinksPi FloThinksPi merged commit 8bb3f13 into main Aug 2, 2023
@FloThinksPi FloThinksPi deleted the improve_pagination_order_performance branch August 2, 2023 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants