Refactor package scopes to use arel nodes [RUN ALL RSPEC] [RUN AS-IF-FOSS]
🛠 What does this MR do?
This MR updates two scopes in the Packages::Package model to build the order by clause using full arel nodes via the Gitlab::Pagination::Keyset modules.
This will allow us to sort packages by project path using GraphQL while also using keyset pagination.
The change is behind a feature flag to give some safety in the case that the update has an unforeseen effect somewhere the scope is being used.
🐘 Database
The intent is that the query itself does not change in any way, just the way that rails is able to interpret it using arel nodes instead of a string expression. This is an example query using the scope, the only difference is the new query is explicit about null value placement in ordering, but the results and plans remain the same:
Query generated by Packages::GroupPackagesFinder, which is used by the graphql Resolvers::GroupPackagesResolver:
Packages::GroupPackagesFinder.new(user, group, { sort: 'asc', order_by: 'project_path' }).execute- Explain plan and query before (feature flag disabled): https://explain.depesz.com/s/bR3P
- Explain plan and query after (feature flag enabled): https://explain.depesz.com/s/7wBT
☑ Does this MR meet the acceptance criteria?
Conformity
-
I have included a changelog entry, or it's not needed. (Does this MR need a changelog?) -
I have added/updated documentation, or it's not needed. (Is documentation required?) -
I have properly separated EE content from FOSS, or this MR is FOSS only. (Where should EE code go?) -
I have added information for database reviewers in the MR description, or it's not needed. (Does this MR have database related changes?) -
I have self-reviewed this MR per code review guidelines. -
This MR does not harm performance, or I have asked a reviewer to help assess the performance impact. (Merge request performance guidelines) -
I have followed the style guides.
Availability and Testing
-
I have added/updated tests following the Testing Guide, or it's not needed. (Consider all test levels. See the Test Planning Process.) -
I have tested this MR in all supported browsers, or it's not needed. -
I have informed the Infrastructure department of a default or new setting change per definition of done, or it's not needed.
Related to #328212 (closed)