Convert Package cross table scope to new custom Arel nodes
We should convert Packages cross table
scopes to the new custom Arel nodes:
https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/pagination/keyset/order.rb#L55
So that we can leverage GraphQL keyset pagination.
The following discussion from !58657 (merged) should be addressed:
-
@10io started a discussion: (+7 comments) So here is the summary of our small investigation with @nmezzopera :
- Keyset pagination is used
- Keyset pagination uses a
OrderInfo
to analyze the current query, extract the order Arel node and build the proper conditions for the given page accessed. - The problem is that this
OrderInfo
only supports very specific arel nodes.
Here are the cases supported:
- If the query order is a "full" arel node. Example:
Packages::Package.order(created_at: :asc).order_values => [#<Arel::Nodes::Ascending:0x00007fbf46094018 @expr= #<struct Arel::Attributes::Attribute relation= #<Arel::Table:0x00007fbf7177afc8 @name="packages_packages", @table_alias=nil, @type_caster=#<ActiveRecord::TypeCaster::Map:0x00007fbf7177b040 @types=Packages::Package(id: integer, project_id: integer, created_at: datetime_with_timezone, updated_at: datetime_with_timezone, name: string, version: string, package_type: integer, creator_id: integer, status: integer)>>, name="created_at">>]
Here are the cases not supported:
- If the query order is built with
.order('X ASC')
- The arel node for the order is simply a string:
Packages::Package.order('created_at ASC').order_values => ["created_at ASC"]
- The arel node for the order is simply a string:
- If the query order is built with a joined table
joins(T).order('Ts.X ASC')
- Same thing as 1.
Packages::Package.joins(:project).order('projects.created_at ASC').order_values => ["projects.created_at ASC"]
- Same thing as 1.
- If the query order is built with a joined table
joins(T).order('Ts.X' => :asc)
- The arel node for the order doesn't have an
expr
object but a string:Packages::Package.joins(:project).order('projects.created_at' => :asc).order_values => [#<Arel::Nodes::Ascending:0x00007fbf366cdbb0 @expr="\"projects\".\"created_at\"">]
- The arel node for the order doesn't have an
In this MR, we are on cases (1.) and (2.), see https://gitlab.com/gitlab-org/gitlab/-/blob/5e1e833f2574206dac810359539b24dd959beadc/app/models/packages/package.rb#L124-136.
In my opinion, there are two things here:
- This seems to be a ~bug in the keyset paginator. We have models that have the same order statements (joined in with a
T
table) and currently doesn't work. Those sort functions are exposed on GraphQL and currently not working.- Example:
{ projects(sort: "storage_size_desc") { nodes { name }, pageInfo { endCursor, startCursor } } }
- This uses this sort and this will make the GraphQL query fail. See this sentry error.
- Example:
- For this MR, the keyset paginator will not be possible to use with the current sorting we have on
Packages::Package
. I would thus suggest to use the offset paginator until, the ~bug on the keyset paginator (1.) is resolved.
Should we open an issue for (1.)?
Edited by David Fernandez