Merge branch '357245-use-arel-native-nulls-order' into 'master'

Use Arel-native nulls order

See merge request gitlab-org/gitlab!84377

app/models/clusters/agent_token.rb

@@ -18,7 +18,7 @@ module Clusters
     validates :description, length: { maximum: 1024 }
     validates :name, presence: true, length: { maximum: 255 }
 
-    scope :order_last_used_at_desc, -> { order(::Gitlab::Database.nulls_last_order('last_used_at', 'DESC')) }
+    scope :order_last_used_at_desc, -> { order(arel_table[:last_used_at].desc.nulls_last) }
     scope :with_status, -> (status) { where(status: status) }
 
     enum status: {

app/models/concerns/issuable.rb

@@ -318,12 +318,16 @@ module Issuable
       # 2. We can't ORDER BY a column that isn't in the GROUP BY and doesn't
       #    have an aggregate function applied, so we do a useless MIN() instead.
       #
-      milestones_due_date = 'MIN(milestones.due_date)'
+      milestones_due_date = Milestone.arel_table[:due_date].minimum
+      milestones_due_date_with_direction = direction == 'ASC' ? milestones_due_date.asc : milestones_due_date.desc
+
+      highest_priority_arel = Arel.sql('highest_priority')
+      highest_priority_arel_with_direction = direction == 'ASC' ? highest_priority_arel.asc : highest_priority_arel.desc
 
       order_milestone_due_asc
         .order_labels_priority(excluded_labels: excluded_labels, extra_select_columns: [milestones_due_date])
-        .reorder(Gitlab::Database.nulls_last_order(milestones_due_date, direction),
-                Gitlab::Database.nulls_last_order('highest_priority', direction))
+        .reorder(milestones_due_date_with_direction.nulls_last,
+                 highest_priority_arel_with_direction.nulls_last)
     end
 
     def order_labels_priority(direction = 'ASC', excluded_labels: [], extra_select_columns: [], with_cte: false)
@@ -341,12 +345,15 @@ module Issuable
 
       extra_select_columns.unshift("highest_priorities.label_priority as highest_priority")
 
+      highest_priority_arel = Arel.sql('highest_priority')
+      highest_priority_arel_with_direction = direction == 'ASC' ? highest_priority_arel.asc : highest_priority_arel.desc
+
       select(issuable_columns)
         .select(extra_select_columns)
         .from("#{table_name}")
         .joins("JOIN LATERAL(#{highest_priority}) as highest_priorities ON TRUE")
         .group(group_columns)
-        .reorder(Gitlab::Database.nulls_last_order('highest_priority', direction))
+        .reorder(highest_priority_arel_with_direction.nulls_last)
     end
 
     def with_label(title, sort = nil)

app/models/environment.rb

@@ -66,10 +66,10 @@ class Environment < ApplicationRecord
   scope :stopped, -> { with_state(:stopped) }
 
   scope :order_by_last_deployed_at, -> do
-    order(Gitlab::Database.nulls_first_order("(#{max_deployment_id_sql})", 'ASC'))
+    order(Arel::Nodes::Grouping.new(max_deployment_id_query).asc.nulls_first)
   end
   scope :order_by_last_deployed_at_desc, -> do
-    order(Gitlab::Database.nulls_last_order("(#{max_deployment_id_sql})", 'DESC'))
+    order(Arel::Nodes::Grouping.new(max_deployment_id_query).desc.nulls_last)
   end
   scope :order_by_name, -> { order('environments.name ASC') }
 
@@ -151,10 +151,11 @@ class Environment < ApplicationRecord
     find_by(id: id, slug: slug)
   end
 
-  def self.max_deployment_id_sql
-    Deployment.select(Deployment.arel_table[:id].maximum)
-    .where(Deployment.arel_table[:environment_id].eq(arel_table[:id]))
-    .to_sql
+  def self.max_deployment_id_query
+    Arel.sql(
+      Deployment.select(Deployment.arel_table[:id].maximum)
+      .where(Deployment.arel_table[:environment_id].eq(arel_table[:id])).to_sql
+    )
   end
 
   def self.pluck_names

app/models/issue.rb

@@ -118,15 +118,15 @@ class Issue < ApplicationRecord
 
   scope :not_authored_by, ->(user) { where.not(author_id: user) }
 
-  scope :order_due_date_asc, -> { reorder(::Gitlab::Database.nulls_last_order('due_date', 'ASC')) }
-  scope :order_due_date_desc, -> { reorder(::Gitlab::Database.nulls_last_order('due_date', 'DESC')) }
+  scope :order_due_date_asc, -> { reorder(arel_table[:due_date].asc.nulls_last) }
+  scope :order_due_date_desc, -> { reorder(arel_table[:due_date].desc.nulls_last) }
   scope :order_closest_future_date, -> { reorder(Arel.sql('CASE WHEN issues.due_date >= CURRENT_DATE THEN 0 ELSE 1 END ASC, ABS(CURRENT_DATE - issues.due_date) ASC')) }
   scope :order_closed_date_desc, -> { reorder(closed_at: :desc) }
   scope :order_created_at_desc, -> { reorder(created_at: :desc) }
   scope :order_severity_asc, -> { includes(:issuable_severity).order('issuable_severities.severity ASC NULLS FIRST') }
   scope :order_severity_desc, -> { includes(:issuable_severity).order('issuable_severities.severity DESC NULLS LAST') }
-  scope :order_escalation_status_asc, -> { includes(:incident_management_issuable_escalation_status).order(::Gitlab::Database.nulls_last_order('incident_management_issuable_escalation_status.status')) }
-  scope :order_escalation_status_desc, -> { includes(:incident_management_issuable_escalation_status).order(::Gitlab::Database.nulls_last_order('incident_management_issuable_escalation_status.status', 'DESC')) }
+  scope :order_escalation_status_asc, -> { includes(:incident_management_issuable_escalation_status).order(IncidentManagement::IssuableEscalationStatus.arel_table[:status].asc.nulls_last).references(:incident_management_issuable_escalation_status) }
+  scope :order_escalation_status_desc, -> { includes(:incident_management_issuable_escalation_status).order(IncidentManagement::IssuableEscalationStatus.arel_table[:status].desc.nulls_last).references(:incident_management_issuable_escalation_status) }
 
   scope :preload_associated_models, -> { preload(:assignees, :labels, project: :namespace) }
   scope :with_web_entity_associations, -> { preload(:author, project: [:project_feature, :route, namespace: :route]) }
@@ -344,8 +344,8 @@ class Issue < ApplicationRecord
     Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
       attribute_name: 'relative_position',
       column_expression: arel_table[:relative_position],
-      order_expression: Gitlab::Database.nulls_last_order('issues.relative_position', 'ASC'),
-      reversed_order_expression: Gitlab::Database.nulls_last_order('issues.relative_position', 'DESC'),
+      order_expression: Issue.arel_table[:relative_position].asc.nulls_last,
+      reversed_order_expression: Issue.arel_table[:relative_position].desc.nulls_last,
       order_direction: :asc,
       nullable: :nulls_last,
       distinct: false

app/models/key.rb

@@ -49,7 +49,7 @@ class Key < ApplicationRecord
 
   scope :preload_users, -> { preload(:user) }
   scope :for_user, -> (user) { where(user: user) }
-  scope :order_last_used_at_desc, -> { reorder(::Gitlab::Database.nulls_last_order('last_used_at', 'DESC')) }
+  scope :order_last_used_at_desc, -> { reorder(arel_table[:last_used_at].desc.nulls_last) }
 
   # Date is set specifically in this scope to improve query time.
   scope :expired_today_and_not_notified, -> { where(["date(expires_at AT TIME ZONE 'UTC') = CURRENT_DATE AND expiry_notification_delivered_at IS NULL"]) }

app/models/member.rb

@@ -178,14 +178,14 @@ class Member < ApplicationRecord
     unscoped.from(distinct_members, :members)
   end
 
-  scope :order_name_asc, -> { left_join_users.reorder(Gitlab::Database.nulls_last_order('users.name', 'ASC')) }
-  scope :order_name_desc, -> { left_join_users.reorder(Gitlab::Database.nulls_last_order('users.name', 'DESC')) }
-  scope :order_recent_sign_in, -> { left_join_users.reorder(Gitlab::Database.nulls_last_order('users.last_sign_in_at', 'DESC')) }
-  scope :order_oldest_sign_in, -> { left_join_users.reorder(Gitlab::Database.nulls_last_order('users.last_sign_in_at', 'ASC')) }
-  scope :order_recent_last_activity, -> { left_join_users.reorder(Gitlab::Database.nulls_last_order('users.last_activity_on', 'DESC')) }
-  scope :order_oldest_last_activity, -> { left_join_users.reorder(Gitlab::Database.nulls_first_order('users.last_activity_on', 'ASC')) }
-  scope :order_recent_created_user, -> { left_join_users.reorder(Gitlab::Database.nulls_last_order('users.created_at', 'DESC')) }
-  scope :order_oldest_created_user, -> { left_join_users.reorder(Gitlab::Database.nulls_first_order('users.created_at', 'ASC')) }
+  scope :order_name_asc, -> { left_join_users.reorder(User.arel_table[:name].asc.nulls_last) }
+  scope :order_name_desc, -> { left_join_users.reorder(User.arel_table[:name].desc.nulls_last) }
+  scope :order_recent_sign_in, -> { left_join_users.reorder(User.arel_table[:last_sign_in_at].desc.nulls_last) }
+  scope :order_oldest_sign_in, -> { left_join_users.reorder(User.arel_table[:last_sign_in_at].asc.nulls_last) }
+  scope :order_recent_last_activity, -> { left_join_users.reorder(User.arel_table[:last_activity_on].desc.nulls_last) }
+  scope :order_oldest_last_activity, -> { left_join_users.reorder(User.arel_table[:last_activity_on].asc.nulls_first) }
+  scope :order_recent_created_user, -> { left_join_users.reorder(User.arel_table[:created_at].desc.nulls_last) }
+  scope :order_oldest_created_user, -> { left_join_users.reorder(User.arel_table[:created_at].asc.nulls_first) }
 
   scope :on_project_and_ancestors, ->(project) { where(source: [project] + project.ancestors) }
 

app/models/merge_request.rb

@@ -329,15 +329,15 @@ class MergeRequest < ApplicationRecord
   end
   scope :by_target_branch, ->(branch_name) { where(target_branch: branch_name) }
   scope :order_by_metric, ->(metric, direction) do
-    reverse_direction = { 'ASC' => 'DESC', 'DESC' => 'ASC' }
-    reversed_direction = reverse_direction[direction] || raise("Unknown sort direction was given: #{direction}")
+    column_expression = MergeRequest::Metrics.arel_table[metric]
+    column_expression_with_direction = direction == 'ASC' ? column_expression.asc : column_expression.desc
 
     order = Gitlab::Pagination::Keyset::Order.build([
       Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
         attribute_name: "merge_request_metrics_#{metric}",
-        column_expression: MergeRequest::Metrics.arel_table[metric],
-        order_expression: Gitlab::Database.nulls_last_order("merge_request_metrics.#{metric}", direction),
-        reversed_order_expression: Gitlab::Database.nulls_first_order("merge_request_metrics.#{metric}", reversed_direction),
+        column_expression: column_expression,
+        order_expression: column_expression_with_direction.nulls_last,
+        reversed_order_expression: column_expression_with_direction.reverse.nulls_first,
         order_direction: direction,
         nullable: :nulls_last,
         distinct: false,

app/models/milestone.rb

@@ -31,7 +31,7 @@ class Milestone < ApplicationRecord
   end
 
   scope :order_by_name_asc, -> { order(Arel::Nodes::Ascending.new(arel_table[:title].lower)) }
-  scope :reorder_by_due_date_asc, -> { reorder(Gitlab::Database.nulls_last_order('due_date', 'ASC')) }
+  scope :reorder_by_due_date_asc, -> { reorder(arel_table[:due_date].asc.nulls_last) }
   scope :with_api_entity_associations, -> { preload(project: [:project_feature, :route, namespace: :route]) }
   scope :order_by_dates_and_title, -> { order(due_date: :asc, start_date: :asc, title: :asc) }
 
@@ -116,15 +116,15 @@ class Milestone < ApplicationRecord
       when 'due_date_asc'
         reorder_by_due_date_asc
       when 'due_date_desc'
-        reorder(Gitlab::Database.nulls_last_order('due_date', 'DESC'))
+        reorder(arel_table[:due_date].desc.nulls_last)
       when 'name_asc'
         reorder(Arel::Nodes::Ascending.new(arel_table[:title].lower))
       when 'name_desc'
         reorder(Arel::Nodes::Descending.new(arel_table[:title].lower))
       when 'start_date_asc'
-        reorder(Gitlab::Database.nulls_last_order('start_date', 'ASC'))
+        reorder(arel_table[:start_date].asc.nulls_last)
       when 'start_date_desc'
-        reorder(Gitlab::Database.nulls_last_order('start_date', 'DESC'))
+        reorder(arel_table[:start_date].desc.nulls_last)
       else
         order_by(method)
       end

app/models/namespace.rb

@@ -132,7 +132,7 @@ class Namespace < ApplicationRecord
 
   scope :user_namespaces, -> { where(type: Namespaces::UserNamespace.sti_name) }
   scope :without_project_namespaces, -> { where(Namespace.arel_table[:type].not_eq(Namespaces::ProjectNamespace.sti_name)) }
-  scope :sort_by_type, -> { order(Gitlab::Database.nulls_first_order(:type)) }
+  scope :sort_by_type, -> { order(arel_table[:type].asc.nulls_first) }
   scope :include_route, -> { includes(:route) }
   scope :by_parent, -> (parent) { where(parent_id: parent) }
   scope :filter_by_path, -> (query) { where('lower(path) = :query', query: query.downcase) }

app/models/packages/package.rb

@@ -228,8 +228,8 @@ class Packages::Package < ApplicationRecord
 
   def self.keyset_pagination_order(join_class:, column_name:, direction: :asc)
     join_table = join_class.table_name
-    asc_order_expression = Gitlab::Database.nulls_last_order("#{join_table}.#{column_name}", :asc)
-    desc_order_expression = Gitlab::Database.nulls_first_order("#{join_table}.#{column_name}", :desc)
+    asc_order_expression = join_class.arel_table[column_name].asc.nulls_last
+    desc_order_expression = join_class.arel_table[column_name].desc.nulls_first
     order_direction = direction == :asc ? asc_order_expression : desc_order_expression
     reverse_order_direction = direction == :asc ? desc_order_expression : asc_order_expression
     arel_order_classes = ::Gitlab::Pagination::Keyset::ColumnOrderDefinition::AREL_ORDER_CLASSES.invert

app/models/todo.rb

@@ -148,10 +148,10 @@ class Todo < ApplicationRecord
         target_type_column: "todos.target_type",
         target_column: "todos.target_id",
         project_column: "todos.project_id"
-      ).to_sql
+      ).arel.as('highest_priority')
 
-      select("#{table_name}.*, (#{highest_priority}) AS highest_priority")
-        .order(Gitlab::Database.nulls_last_order('highest_priority', 'ASC'))
+      select(arel_table[Arel.star], highest_priority)
+        .order(Arel.sql('highest_priority').asc.nulls_last)
         .order('todos.created_at')
     end
 

app/models/user.rb

@@ -470,10 +470,10 @@ class User < ApplicationRecord
          .where('keys.user_id = users.id')
          .expiring_soon_and_not_notified)
   end
-  scope :order_recent_sign_in, -> { reorder(Gitlab::Database.nulls_last_order('current_sign_in_at', 'DESC')) }
-  scope :order_oldest_sign_in, -> { reorder(Gitlab::Database.nulls_last_order('current_sign_in_at', 'ASC')) }
-  scope :order_recent_last_activity, -> { reorder(Gitlab::Database.nulls_last_order('last_activity_on', 'DESC')) }
-  scope :order_oldest_last_activity, -> { reorder(Gitlab::Database.nulls_first_order('last_activity_on', 'ASC')) }
+  scope :order_recent_sign_in, -> { reorder(arel_table[:current_sign_in_at].desc.nulls_last) }
+  scope :order_oldest_sign_in, -> { reorder(arel_table[:current_sign_in_at].asc.nulls_last) }
+  scope :order_recent_last_activity, -> { reorder(arel_table[:last_activity_on].desc.nulls_last) }
+  scope :order_oldest_last_activity, -> { reorder(arel_table[:last_activity_on].asc.nulls_first) }
   scope :by_id_and_login, ->(id, login) { where(id: id).where('username = LOWER(:login) OR email = LOWER(:login)', login: login) }
   scope :dormant, -> { with_state(:active).human_or_service_user.where('last_activity_on <= ?', MINIMUM_INACTIVE_DAYS.day.ago.to_date) }
   scope :with_no_activity, -> { with_state(:active).human_or_service_user.where(last_activity_on: nil) }

doc/development/database/keyset_pagination.md

@@ -166,7 +166,7 @@ These order objects can be defined in the model classes as normal ActiveRecord s
 Consider the following scope:
 
 ```ruby
-scope = Issue.where(project_id: 10).order(Gitlab::Database.nulls_last_order('relative_position', 'DESC'))
+scope = Issue.where(project_id: 10).order(Issue.arel_table[:relative_position].desc.nulls_last)
 # SELECT "issues".* FROM "issues" WHERE "issues"."project_id" = 10 ORDER BY relative_position DESC NULLS LAST
 
 scope.keyset_paginate # raises: Gitlab::Pagination::Keyset::UnsupportedScopeOrder: The order on the scope does not support keyset pagination
@@ -190,8 +190,8 @@ order = Gitlab::Pagination::Keyset::Order.build([
   Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
     attribute_name: 'relative_position',
     column_expression: Issue.arel_table[:relative_position],
-    order_expression: Gitlab::Database.nulls_last_order('relative_position', 'DESC'),
-    reversed_order_expression: Gitlab::Database.nulls_first_order('relative_position', 'ASC'),
+    order_expression: Issue.arel_table[:relative_position].desc.nulls_last,
+    reversed_order_expression: Issue.arel_table[:relative_position].asc.nulls_first,
     nullable: :nulls_last,
     order_direction: :desc,
     distinct: false

ee/app/finders/geo/repository_verification_finder.rb

@@ -168,7 +168,7 @@ module Geo
     end
 
     def last_repository_updated_at_asc
-      Gitlab::Database.nulls_last_order('projects.last_repository_updated_at', 'ASC')
+      Project.arel_table[:last_repository_updated_at].asc.nulls_last
     end
 
     # rubocop: disable CodeReuse/ActiveRecord

ee/app/models/concerns/geo/replicable_registry.rb

@@ -59,7 +59,7 @@ module Geo::ReplicableRegistry
     attr_accessor :custom_max_retry_wait_time
 
     scope :failed, -> { with_state(:failed) }
-    scope :needs_sync_again, -> { failed.retry_due.order(Gitlab::Database.nulls_first_order(:retry_at)) }
+    scope :needs_sync_again, -> { failed.retry_due.order(arel_table[:retry_at].asc.nulls_first) }
     scope :never_attempted_sync, -> { pending.where(last_synced_at: nil) }
     scope :ordered, -> { order(:id) }
     scope :pending, -> { with_state(:pending) }

ee/app/models/concerns/geo/verification_state.rb

@@ -65,7 +65,7 @@ module Geo
       # query.
       #
       def verification_pending_batch(batch_size:)
-        relation = verification_pending.order(Gitlab::Database.nulls_first_order(:verified_at)).limit(batch_size)
+        relation = verification_pending.order(verification_arel_table[:verified_at].asc.nulls_first).limit(batch_size)
 
         start_verification_batch(relation)
       end
@@ -76,7 +76,7 @@ module Geo
       # query.
       #
       def verification_failed_batch(batch_size:)
-        relation = verification_failed.verification_retry_due.order(Gitlab::Database.nulls_first_order(:verification_retry_at)).limit(batch_size)
+        relation = verification_failed.verification_retry_due.order(verification_arel_table[:verification_retry_at].asc.nulls_first).limit(batch_size)
 
         start_verification_batch(relation)
       end

ee/app/models/ee/epic.rb

@@ -139,7 +139,7 @@ module EE
       end
 
       scope :order_relative_position_on_board, ->(board_id) do
-        reorder(::Gitlab::Database.nulls_last_order('boards_epic_board_positions.relative_position', 'ASC'), 'epics.id DESC')
+        reorder(::Boards::EpicBoardPosition.arel_table[:relative_position].asc.nulls_last, 'epics.id DESC')
       end
 
       scope :without_board_position, ->(board_id) do
@@ -359,14 +359,15 @@ module EE
       end
 
       def keyset_pagination_for(column_name:, direction: 'ASC')
-        reverse_direction = direction == 'ASC' ? 'DESC' : 'ASC'
+        column_expression = ::Epic.arel_table[column_name]
+        column_expression_with_direction = direction == 'ASC' ? column_expression.asc : column_expression.desc
 
         ::Gitlab::Pagination::Keyset::Order.build([
           ::Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
             attribute_name: column_name.to_s,
-            column_expression: ::Epic.arel_table[column_name],
-            order_expression: ::Gitlab::Database.nulls_last_order(column_name, direction),
-            reversed_order_expression: ::Gitlab::Database.nulls_last_order(column_name, reverse_direction),
+            column_expression: column_expression,
+            order_expression: column_expression_with_direction.nulls_last,
+            reversed_order_expression: column_expression_with_direction.reverse.nulls_last,
             order_direction: direction,
             distinct: false,
             nullable: :nulls_last

ee/app/models/ee/issue.rb

@@ -23,8 +23,8 @@ module EE
 
       scope :order_blocking_issues_asc, -> { reorder(blocking_issues_count: :asc) }
       scope :order_blocking_issues_desc, -> { reorder(blocking_issues_count: :desc) }
-      scope :order_weight_desc, -> { reorder ::Gitlab::Database.nulls_last_order('weight', 'DESC') }
-      scope :order_weight_asc, -> { reorder ::Gitlab::Database.nulls_last_order('weight') }
+      scope :order_weight_desc, -> { reorder(arel_table[:weight].desc.nulls_last) }
+      scope :order_weight_asc, -> { reorder(arel_table[:weight].asc.nulls_last) }
       scope :order_status_page_published_first, -> { includes(:status_page_published_incident).order('status_page_published_incidents.id ASC NULLS LAST') }
       scope :order_status_page_published_last, -> { includes(:status_page_published_incident).order('status_page_published_incidents.id ASC NULLS FIRST') }
       scope :order_sla_due_at_asc, -> { includes(:issuable_sla).order('issuable_slas.due_at ASC NULLS LAST') }

ee/app/models/ee/merge_request.rb

@@ -59,8 +59,7 @@ module EE
       accepts_nested_attributes_for :approval_rules, allow_destroy: true
 
       scope :order_review_time_desc, -> do
-        joins(:metrics)
-          .reorder(::Gitlab::Database.nulls_last_order(::MergeRequest::Metrics.review_time_field))
+        joins(:metrics).reorder(::MergeRequest::Metrics.review_time_field.asc.nulls_last)
       end
 
       scope :with_code_review_api_entity_associations, -> do

ee/app/models/geo/container_repository_registry.rb

@@ -43,7 +43,7 @@ class Geo::ContainerRepositoryRegistry < Geo::BaseRegistry
     include Delay
 
     def find_registries_needs_sync_again(batch_size:, except_ids: [])
-      super.order(Gitlab::Database.nulls_first_order(:last_synced_at))
+      super.order(arel_table[:last_synced_at].asc.nulls_first)
     end
 
     def delete_for_model_ids(container_repository_ids)

ee/app/models/geo/design_registry.rb

@@ -62,7 +62,7 @@ class Geo::DesignRegistry < Geo::BaseRegistry
   end
 
   def self.find_registries_needs_sync_again(batch_size:, except_ids: [])
-    super.order(Gitlab::Database.nulls_first_order(:last_synced_at))
+    super.order(arel_table[:last_synced_at].asc.nulls_first)
   end
 
   # Search for a list of projects associated with registries,

ee/app/models/geo/project_registry.rb

@@ -45,7 +45,7 @@ class Geo::ProjectRegistry < Geo::BaseRegistry
   end
 
   def self.find_registries_needs_sync_again(batch_size:, except_ids: [])
-    super.order(Gitlab::Database.nulls_first_order(:last_repository_synced_at))
+    super.order(arel_table[:last_repository_synced_at].asc.nulls_first)
   end
 
   def self.delete_worker_class

lib/gitlab/database.rb

@@ -161,24 +161,6 @@ module Gitlab
       end
     end
 
-    def self.nulls_order(field, direction = :asc, nulls_order = :nulls_last)
-      raise ArgumentError unless [:nulls_last, :nulls_first].include?(nulls_order)
-      raise ArgumentError unless [:asc, :desc].include?(direction)
-
-      case nulls_order
-      when :nulls_last then nulls_last_order(field, direction)
-      when :nulls_first then nulls_first_order(field, direction)
-      end
-    end
-
-    def self.nulls_last_order(field, direction = 'ASC')
-      Arel.sql("#{field} #{direction} NULLS LAST")
-    end
-
-    def self.nulls_first_order(field, direction = 'ASC')
-      Arel.sql("#{field} #{direction} NULLS FIRST")
-    end
-
     def self.random
       "RANDOM()"
     end

lib/gitlab/graphql/pagination/keyset/connection.rb

@@ -14,10 +14,6 @@
 #   Issue.order(created_at: :asc).order(:id)
 #   Issue.order(due_date: :asc)
 #
-# You can also use `Gitlab::Database.nulls_last_order`:
-#
-#   Issue.reorder(::Gitlab::Database.nulls_last_order('due_date', 'DESC'))
-#
 # It will tolerate non-attribute ordering, but only attributes determine the cursor.
 # For example, this is legitimate:
 #

lib/gitlab/import_export/json/streaming_serializer.rb

@@ -145,8 +145,8 @@ module Gitlab
           arel_order_classes = ::Gitlab::Pagination::Keyset::ColumnOrderDefinition::AREL_ORDER_CLASSES.invert
           reverse_direction = ::Gitlab::Pagination::Keyset::ColumnOrderDefinition::REVERSED_ORDER_DIRECTIONS[direction]
           reverse_nulls_position = ::Gitlab::Pagination::Keyset::ColumnOrderDefinition::REVERSED_NULL_POSITIONS[nulls_position]
-          order_expression = ::Gitlab::Database.nulls_order(column, direction, nulls_position)
-          reverse_order_expression = ::Gitlab::Database.nulls_order(column, reverse_direction, reverse_nulls_position)
+          order_expression = arel_table[column].public_send(direction).public_send(nulls_position) # rubocop:disable GitlabSecurity/PublicSend
+          reverse_order_expression = arel_table[column].public_send(reverse_direction).public_send(reverse_nulls_position) # rubocop:disable GitlabSecurity/PublicSend
 
           ::Gitlab::Pagination::Keyset::Order.build([
             ::Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(

lib/gitlab/relative_positioning/item_context.rb

@@ -84,7 +84,7 @@ module Gitlab
         # MAX(relative_position) without the GROUP BY, due to index usage:
         # https://gitlab.com/gitlab-org/gitlab-foss/issues/54276#note_119340977
         relation = scoped_items
-                     .order(Gitlab::Database.nulls_last_order('position', 'DESC'))
+                     .order(Arel.sql('position').desc.nulls_last)
                      .group(grouping_column)
                      .limit(1)
 
@@ -101,7 +101,7 @@ module Gitlab
 
       def max_sibling
         sib = relative_siblings
-          .order(Gitlab::Database.nulls_last_order('relative_position', 'DESC'))
+          .order(model_class.arel_table[:relative_position].desc.nulls_last)
           .first
 
         neighbour(sib)
@@ -109,7 +109,7 @@ module Gitlab
 
       def min_sibling
         sib = relative_siblings
-          .order(Gitlab::Database.nulls_last_order('relative_position', 'ASC'))
+          .order(model_class.arel_table[:relative_position].asc.nulls_last)
           .first
 
         neighbour(sib)

spec/lib/gitlab/database_spec.rb

@@ -185,16 +185,6 @@ RSpec.describe Gitlab::Database do
     end
   end
 
-  describe '.nulls_last_order' do
-    it { expect(described_class.nulls_last_order('column', 'ASC')).to eq 'column ASC NULLS LAST'}
-    it { expect(described_class.nulls_last_order('column', 'DESC')).to eq 'column DESC NULLS LAST'}
-  end
-
-  describe '.nulls_first_order' do
-    it { expect(described_class.nulls_first_order('column', 'ASC')).to eq 'column ASC NULLS FIRST'}
-    it { expect(described_class.nulls_first_order('column', 'DESC')).to eq 'column DESC NULLS FIRST'}
-  end
-
   describe '.db_config_for_connection' do
     context 'when the regular connection is used' do
       it 'returns db_config' do

spec/lib/gitlab/graphql/pagination/keyset/connection_generic_keyset_spec.rb

@@ -19,8 +19,8 @@ RSpec.describe Gitlab::Graphql::Pagination::Keyset::Connection do
     Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
       attribute_name: 'last_repository_check_at',
       column_expression: Project.arel_table[:last_repository_check_at],
-      order_expression: Gitlab::Database.nulls_last_order('last_repository_check_at', :asc),
-      reversed_order_expression: Gitlab::Database.nulls_last_order('last_repository_check_at', :desc),
+      order_expression: Project.arel_table[:last_repository_check_at].asc.nulls_last,
+      reversed_order_expression: Project.arel_table[:last_repository_check_at].desc.nulls_last,
       order_direction: :asc,
       nullable: :nulls_last,
       distinct: false)
@@ -30,8 +30,8 @@ RSpec.describe Gitlab::Graphql::Pagination::Keyset::Connection do
     Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
       attribute_name: 'last_repository_check_at',
       column_expression: Project.arel_table[:last_repository_check_at],
-      order_expression: Gitlab::Database.nulls_last_order('last_repository_check_at', :desc),
-      reversed_order_expression: Gitlab::Database.nulls_last_order('last_repository_check_at', :asc),
+      order_expression: Project.arel_table[:last_repository_check_at].desc.nulls_last,
+      reversed_order_expression: Project.arel_table[:last_repository_check_at].asc.nulls_last,
       order_direction: :desc,
       nullable: :nulls_last,
       distinct: false)

spec/lib/gitlab/graphql/pagination/keyset/connection_spec.rb

@@ -264,7 +264,6 @@ RSpec.describe Gitlab::Graphql::Pagination::Keyset::Connection do
 
         where(:nodes) do
           [
-            lazy { Issue.order(::Gitlab::Database.nulls_last_order('relative_position', 'ASC')) },
             lazy { Issue.order(Issue.arel_table[:relative_position].asc.nulls_last) }
           ]
         end
@@ -279,7 +278,6 @@ RSpec.describe Gitlab::Graphql::Pagination::Keyset::Connection do
 
         where(:nodes) do
           [
-            lazy { Issue.order(::Gitlab::Database.nulls_last_order('relative_position', 'DESC')) },
             lazy { Issue.order(Issue.arel_table[:relative_position].desc.nulls_last) }
 ]
         end
@@ -294,7 +292,6 @@ RSpec.describe Gitlab::Graphql::Pagination::Keyset::Connection do
 
         where(:nodes) do
           [
-            lazy { Issue.order(::Gitlab::Database.nulls_first_order('relative_position', 'ASC')).order(id: :asc) },
             lazy { Issue.order(Issue.arel_table[:relative_position].asc.nulls_first).order(id: :asc) }
 ]
         end
@@ -309,7 +306,6 @@ RSpec.describe Gitlab::Graphql::Pagination::Keyset::Connection do
 
         where(:nodes) do
           [
-            lazy { Issue.order(::Gitlab::Database.nulls_first_order('relative_position', 'DESC')).order(id: :asc) },
             lazy { Issue.order(Issue.arel_table[:relative_position].desc.nulls_first).order(id: :asc) }
 ]
         end

spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb

@@ -115,7 +115,7 @@ RSpec.describe Gitlab::ImportExport::Json::StreamingSerializer do
         end
 
         it 'orders exported issues by custom column(relative_position)' do
-          expected_issues = exportable.issues.reorder(::Gitlab::Database.nulls_first_order('relative_position', 'DESC')).order(id: :desc).map(&:to_json)
+          expected_issues = exportable.issues.reorder(Issue.arel_table[:relative_position].desc.nulls_first).order(id: :desc).map(&:to_json)
 
           expect(json_writer).to receive(:write_relation_array).with(exportable_path, :issues, expected_issues)
 

spec/lib/gitlab/pagination/keyset/column_order_definition_spec.rb

@@ -140,8 +140,8 @@ RSpec.describe Gitlab::Pagination::Keyset::ColumnOrderDefinition do
         described_class.new(
           attribute_name: :name,
           column_expression: Project.arel_table[:name],
-          order_expression: Gitlab::Database.nulls_last_order('merge_request_metrics.merged_at', :desc),
-          reversed_order_expression: Gitlab::Database.nulls_first_order('merge_request_metrics.merged_at', :asc),
+          order_expression: MergeRequest::Metrics.arel_table[:merged_at].desc.nulls_last,
+          reversed_order_expression: MergeRequest::Metrics.arel_table[:merged_at].asc.nulls_first,
           order_direction: :desc,
           nullable: :nulls_last, # null values are always last
           distinct: false
@@ -161,8 +161,8 @@ RSpec.describe Gitlab::Pagination::Keyset::ColumnOrderDefinition do
           described_class.new(
             attribute_name: :name,
             column_expression: Project.arel_table[:name],
-            order_expression: Gitlab::Database.nulls_last_order('merge_request_metrics.merged_at', :desc),
-            reversed_order_expression: Gitlab::Database.nulls_first_order('merge_request_metrics.merged_at', :asc),
+            order_expression: MergeRequest::Metrics.arel_table[:merged_at].desc.nulls_last,
+            reversed_order_expression: MergeRequest::Metrics.arel_table[:merged_at].asc.nulls_first,
             order_direction: :desc,
             nullable: true,
             distinct: false
@@ -175,8 +175,8 @@ RSpec.describe Gitlab::Pagination::Keyset::ColumnOrderDefinition do
           described_class.new(
             attribute_name: :name,
             column_expression: Project.arel_table[:name],
-            order_expression: Gitlab::Database.nulls_last_order('merge_request_metrics.merged_at', :desc),
-            reversed_order_expression: Gitlab::Database.nulls_first_order('merge_request_metrics.merged_at', :asc),
+            order_expression: MergeRequest::Metrics.arel_table[:merged_at].desc.nulls_last,
+            reversed_order_expression: MergeRequest::Metrics.arel_table[:merged_at].asc.nulls_first,
             order_direction: :desc,
             nullable: :nulls_last,
             distinct: true
@@ -191,8 +191,8 @@ RSpec.describe Gitlab::Pagination::Keyset::ColumnOrderDefinition do
       described_class.new(
         attribute_name: :name,
         column_expression: Project.arel_table[:name],
-        order_expression: Gitlab::Database.nulls_last_order('merge_request_metrics.merged_at', :desc),
-        reversed_order_expression: Gitlab::Database.nulls_first_order('merge_request_metrics.merged_at', :asc),
+        order_expression: MergeRequest::Metrics.arel_table[:merged_at].desc.nulls_last,
+        reversed_order_expression: MergeRequest::Metrics.arel_table[:merged_at].asc.nulls_first,
         order_direction: :desc,
         nullable: :nulls_last, # null values are always last
         distinct: false

spec/lib/gitlab/pagination/keyset/in_operator_optimization/query_builder_spec.rb

@@ -121,8 +121,8 @@ RSpec.describe Gitlab::Pagination::Keyset::InOperatorOptimization::QueryBuilder
         Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
           attribute_name: :relative_position,
           column_expression: Issue.arel_table[:relative_position],
-          order_expression: Gitlab::Database.nulls_last_order('relative_position', :desc),
-          reversed_order_expression: Gitlab::Database.nulls_first_order('relative_position', :asc),
+          order_expression: Issue.arel_table[:relative_position].desc.nulls_last,
+          reversed_order_expression: Issue.arel_table[:relative_position].asc.nulls_first,
           order_direction: :desc,
           nullable: :nulls_last,
           distinct: false

spec/lib/gitlab/pagination/keyset/iterator_spec.rb

@@ -19,8 +19,8 @@ RSpec.describe Gitlab::Pagination::Keyset::Iterator do
       Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
         attribute_name: column,
         column_expression: klass.arel_table[column],
-        order_expression: ::Gitlab::Database.nulls_order(column, direction, nulls_position),
-        reversed_order_expression: ::Gitlab::Database.nulls_order(column, reverse_direction, reverse_nulls_position),
+        order_expression: klass.arel_table[column].public_send(direction).public_send(nulls_position), # rubocop:disable GitlabSecurity/PublicSend
+        reversed_order_expression: klass.arel_table[column].public_send(reverse_direction).public_send(reverse_nulls_position), # rubocop:disable GitlabSecurity/PublicSend
         order_direction: direction,
         nullable: nulls_position,
         distinct: false
@@ -99,7 +99,7 @@ RSpec.describe Gitlab::Pagination::Keyset::Iterator do
 
             iterator.each_batch(of: 2) { |rel| positions.concat(rel.pluck(:relative_position, :id)) }
 
-            expect(positions).to eq(project.issues.reorder(::Gitlab::Database.nulls_last_order('relative_position', 'ASC')).order(id: :asc).pluck(:relative_position, :id))
+            expect(positions).to eq(project.issues.reorder(Issue.arel_table[:relative_position].asc.nulls_last).order(id: :asc).pluck(:relative_position, :id))
           end
         end
 
@@ -111,7 +111,7 @@ RSpec.describe Gitlab::Pagination::Keyset::Iterator do
 
             iterator.each_batch(of: 2) { |rel| positions.concat(rel.pluck(:relative_position, :id)) }
 
-            expect(positions).to eq(project.issues.reorder(::Gitlab::Database.nulls_first_order('relative_position', 'DESC')).order(id: :desc).pluck(:relative_position, :id))
+            expect(positions).to eq(project.issues.reorder(Issue.arel_table[:relative_position].desc.nulls_first).order(id: :desc).pluck(:relative_position, :id))
           end
         end
 
@@ -123,7 +123,7 @@ RSpec.describe Gitlab::Pagination::Keyset::Iterator do
 
             iterator.each_batch(of: 2) { |rel| positions.concat(rel.pluck(:relative_position, :id)) }
 
-            expect(positions).to eq(project.issues.reorder(::Gitlab::Database.nulls_first_order('relative_position', 'ASC')).order(id: :asc).pluck(:relative_position, :id))
+            expect(positions).to eq(project.issues.reorder(Issue.arel_table[:relative_position].asc.nulls_first).order(id: :asc).pluck(:relative_position, :id))
           end
         end
 
@@ -136,7 +136,7 @@ RSpec.describe Gitlab::Pagination::Keyset::Iterator do
 
             iterator.each_batch(of: 2) { |rel| positions.concat(rel.pluck(:relative_position, :id)) }
 
-            expect(positions).to eq(project.issues.reorder(::Gitlab::Database.nulls_last_order('relative_position', 'DESC')).order(id: :desc).pluck(:relative_position, :id))
+            expect(positions).to eq(project.issues.reorder(Issue.arel_table[:relative_position].desc.nulls_last).order(id: :desc).pluck(:relative_position, :id))
           end
         end
 

spec/lib/gitlab/pagination/keyset/order_spec.rb

@@ -262,8 +262,8 @@ RSpec.describe Gitlab::Pagination::Keyset::Order do
           Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
             attribute_name: 'year',
             column_expression: table['year'],
-            order_expression: Gitlab::Database.nulls_last_order('year', :asc),
-            reversed_order_expression: Gitlab::Database.nulls_first_order('year', :desc),
+            order_expression: table[:year].asc.nulls_last,
+            reversed_order_expression: table[:year].desc.nulls_first,
             order_direction: :asc,
             nullable: :nulls_last,
             distinct: false
@@ -271,8 +271,8 @@ RSpec.describe Gitlab::Pagination::Keyset::Order do
           Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
             attribute_name: 'month',
             column_expression: table['month'],
-            order_expression: Gitlab::Database.nulls_last_order('month', :asc),
-            reversed_order_expression: Gitlab::Database.nulls_first_order('month', :desc),
+            order_expression: table[:month].asc.nulls_last,
+            reversed_order_expression: table[:month].desc.nulls_first,
             order_direction: :asc,
             nullable: :nulls_last,
             distinct: false
@@ -328,8 +328,8 @@ RSpec.describe Gitlab::Pagination::Keyset::Order do
           Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
             attribute_name: 'year',
             column_expression: table['year'],
-            order_expression: Gitlab::Database.nulls_first_order('year', :asc),
-            reversed_order_expression: Gitlab::Database.nulls_last_order('year', :desc),
+            order_expression: table[:year].asc.nulls_first,
+            reversed_order_expression: table[:year].desc.nulls_last,
             order_direction: :asc,
             nullable: :nulls_first,
             distinct: false
@@ -337,9 +337,9 @@ RSpec.describe Gitlab::Pagination::Keyset::Order do
           Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
             attribute_name: 'month',
             column_expression: table['month'],
-            order_expression: Gitlab::Database.nulls_first_order('month', :asc),
+            order_expression: table[:month].asc.nulls_first,
             order_direction: :asc,
-            reversed_order_expression: Gitlab::Database.nulls_last_order('month', :desc),
+            reversed_order_expression: table[:month].desc.nulls_last,
             nullable: :nulls_first,
             distinct: false
           ),

spec/lib/gitlab/pagination/keyset/simple_order_builder_spec.rb

@@ -94,26 +94,28 @@ RSpec.describe Gitlab::Pagination::Keyset::SimpleOrderBuilder do
 
   context "NULLS order given as as an Arel literal" do
     context 'when NULLS LAST order is given without a tie-breaker' do
-      let(:scope) { Project.order(::Gitlab::Database.nulls_last_order('created_at', 'ASC')) }
+      let(:scope) { Project.order(Project.arel_table[:created_at].asc.nulls_last) }
 
       it 'sets the column definition for created_at appropriately' do
         expect(column_definition.attribute_name).to eq('created_at')
       end
 
       it 'orders by primary key' do
-        expect(sql_with_order).to end_with('ORDER BY "projects".created_at ASC NULLS LAST, "projects"."id" DESC')
+        expect(sql_with_order)
+          .to end_with('ORDER BY "projects"."created_at" ASC NULLS LAST, "projects"."id" DESC')
       end
     end
 
     context 'when NULLS FIRST order is given with a tie-breaker' do
-      let(:scope) { Issue.order(::Gitlab::Database.nulls_first_order('relative_position', 'DESC')).order(id: :asc) }
+      let(:scope) { Issue.order(Issue.arel_table[:relative_position].desc.nulls_first).order(id: :asc) }
 
       it 'sets the column definition for created_at appropriately' do
         expect(column_definition.attribute_name).to eq('relative_position')
       end
 
       it 'orders by the given primary key' do
-        expect(sql_with_order).to end_with('ORDER BY "issues".relative_position DESC NULLS FIRST, "issues"."id" ASC')
+        expect(sql_with_order)
+          .to end_with('ORDER BY "issues"."relative_position" DESC NULLS FIRST, "issues"."id" ASC')
       end
     end
   end
@@ -159,8 +161,8 @@ RSpec.describe Gitlab::Pagination::Keyset::SimpleOrderBuilder do
       where(:scope) do
         [
           lazy { Project.order(Arel.sql('projects.updated_at created_at Asc Nulls Last')) },
-          lazy { Project.order(::Gitlab::Database.nulls_first_order('created_at', 'ZZZ')) },
-          lazy { Project.order(::Gitlab::Database.nulls_last_order('relative_position', 'ASC')) }
+          lazy { Project.order(Arel.sql('projects.created_at ZZZ NULLS FIRST')) },
+          lazy { Project.order(Arel.sql('projects.relative_position ASC NULLS LAST')) }
         ]
       end
 

spec/models/packages/package_spec.rb

@@ -1021,13 +1021,13 @@ RSpec.describe Packages::Package, type: :model do
       context 'ascending direction' do
         let(:direction) { :asc }
 
-        it { is_expected.to eq('projects.name asc NULLS LAST, "packages_packages"."id" ASC') }
+        it { is_expected.to eq('"projects"."name" ASC NULLS LAST, "packages_packages"."id" ASC') }
       end
 
       context 'descending direction' do
         let(:direction) { :desc }
 
-        it { is_expected.to eq('projects.name desc NULLS FIRST, "packages_packages"."id" DESC') }
+        it { is_expected.to eq('"projects"."name" DESC NULLS FIRST, "packages_packages"."id" DESC') }
       end
     end
   end