Merge branch 'ahegyi-keyset-experiment' into 'master'

Implement generic keyset pagination

See merge request gitlab-org/gitlab!51481

app/models/merge_request.rb

@@ -310,10 +310,28 @@ class MergeRequest < ApplicationRecord
   end
   scope :by_target_branch, ->(branch_name) { where(target_branch: branch_name) }
   scope :order_merged_at, ->(direction) do
-    query = join_metrics.order(Gitlab::Database.nulls_last_order('merge_request_metrics.merged_at', direction))
-
-    # Add `merge_request_metrics.merged_at` to the `SELECT` in order to make the keyset pagination work.
-    query.select(*query.arel.projections, MergeRequest::Metrics.arel_table[:merged_at].as('"merge_request_metrics.merged_at"'))
+    reverse_direction = { 'ASC' => 'DESC', 'DESC' => 'ASC' }
+    reversed_direction = reverse_direction[direction] || raise("Unknown sort direction was given: #{direction}")
+
+    order = Gitlab::Pagination::Keyset::Order.build([
+      Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
+        attribute_name: 'merge_request_metrics_merged_at',
+        column_expression: MergeRequest::Metrics.arel_table[:merged_at],
+        order_expression: Gitlab::Database.nulls_last_order('merge_request_metrics.merged_at', direction),
+        reversed_order_expression: Gitlab::Database.nulls_first_order('merge_request_metrics.merged_at', reversed_direction),
+        order_direction: direction,
+        nullable: :nulls_last,
+        distinct: false,
+        add_to_projections: true
+      ),
+      Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
+        attribute_name: 'merge_request_metrics_id',
+        order_expression: MergeRequest::Metrics.arel_table[:id].desc,
+        add_to_projections: true
+      )
+    ])
+
+    order.apply_cursor_conditions(join_metrics).order(order)
   end
   scope :order_merged_at_asc, -> { order_merged_at('ASC') }
   scope :order_merged_at_desc, -> { order_merged_at('DESC') }
@@ -411,8 +429,8 @@ class MergeRequest < ApplicationRecord
 
   def self.sort_by_attribute(method, excluded_labels: [])
     case method.to_s
-    when 'merged_at', 'merged_at_asc' then order_merged_at_asc.with_order_id_desc
-    when 'merged_at_desc' then order_merged_at_desc.with_order_id_desc
+    when 'merged_at', 'merged_at_asc' then order_merged_at_asc
+    when 'merged_at_desc' then order_merged_at_desc
     else
       super
     end

app/models/project.rb

@@ -493,10 +493,22 @@ class Project < ApplicationRecord
       { column: arel_table["description"], multiplier: 0.2 }
     ])
 
-    query = reorder(order_expression.desc, arel_table['id'].desc)
+    order = Gitlab::Pagination::Keyset::Order.build([
+      Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
+        attribute_name: 'similarity',
+        column_expression: order_expression,
+        order_expression: order_expression.desc,
+        order_direction: :desc,
+        distinct: false,
+        add_to_projections: true
+      ),
+      Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
+        attribute_name: 'id',
+        order_expression: Project.arel_table[:id].desc
+      )
+    ])
 
-    query = query.select(*query.arel.projections, order_expression.as('similarity')) if include_in_select
-    query
+    order.apply_cursor_conditions(reorder(order))
   end
 
   scope :with_packages, -> { joins(:packages) }

lib/gitlab/database/similarity_score.rb

@@ -74,9 +74,14 @@ module Gitlab
         end
 
         # (SIMILARITY ...) + (SIMILARITY ...)
-        expressions.inject(first_expression) do |expression1, expression2|
+        additions = expressions.inject(first_expression) do |expression1, expression2|
           Arel::Nodes::Addition.new(expression1, expression2)
         end
+
+        score_as_numeric = Arel::Nodes::NamedFunction.new('CAST', [Arel::Nodes::Grouping.new(additions).as('numeric')])
+
+        # Rounding the score to two decimals
+        Arel::Nodes::NamedFunction.new('ROUND', [score_as_numeric, 2])
       end
 
       def self.order_by_similarity?(arel_query)

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

@@ -33,6 +33,7 @@ module Gitlab
           include Gitlab::Utils::StrongMemoize
           include ::Gitlab::Graphql::ConnectionCollectionMethods
           prepend ::Gitlab::Graphql::ConnectionRedaction
+          prepend GenericKeysetPagination
 
           # rubocop: disable Naming/PredicateName
           # https://relay.dev/graphql/connections.htm#sec-undefined.PageInfo.Fields

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

+# frozen_string_literal: true
+
+module Gitlab
+  module Graphql
+    module Pagination
+      module Keyset
+        # Use the generic keyset implementation if the given ActiveRecord scope supports it.
+        # Note: this module is temporary, at some point it will be merged with Keyset::Connection
+        module GenericKeysetPagination
+          extend ActiveSupport::Concern
+
+          def ordered_items
+            return super unless Gitlab::Pagination::Keyset::Order.keyset_aware?(items)
+
+            items
+          end
+
+          def cursor_for(node)
+            return super unless Gitlab::Pagination::Keyset::Order.keyset_aware?(items)
+
+            order = Gitlab::Pagination::Keyset::Order.extract_keyset_order_object(items)
+            encode(order.cursor_attributes_for_node(node).to_json)
+          end
+
+          def slice_nodes(sliced, encoded_cursor, before_or_after)
+            return super unless Gitlab::Pagination::Keyset::Order.keyset_aware?(sliced)
+
+            order = Gitlab::Pagination::Keyset::Order.extract_keyset_order_object(sliced)
+            order = order.reversed_order if before_or_after == :before
+
+            decoded_cursor = ordering_from_encoded_json(encoded_cursor)
+            order.apply_cursor_conditions(sliced, decoded_cursor)
+          end
+
+          def sliced_nodes
+            return super unless Gitlab::Pagination::Keyset::Order.keyset_aware?(items)
+
+            sliced = ordered_items
+            sliced = slice_nodes(sliced, before, :before) if before.present?
+            sliced = slice_nodes(sliced, after, :after) if after.present?
+            sliced
+          end
+        end
+      end
+    end
+  end
+end

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

@@ -10,46 +10,14 @@ module Gitlab
         class LastItems
           # rubocop: disable CodeReuse/ActiveRecord
           def self.take_items(scope, count)
-            if custom_order = lookup_custom_reverse_order(scope.order_values)
-              items = scope.reorder(*custom_order).first(count) # returns a single record when count is nil
+            if Gitlab::Pagination::Keyset::Order.keyset_aware?(scope)
+              order = Gitlab::Pagination::Keyset::Order.extract_keyset_order_object(scope)
+              items = scope.reorder(order.reversed_order).first(count)
               items.is_a?(Array) ? items.reverse : items
             else
               scope.last(count)
             end
           end
-          # rubocop: enable CodeReuse/ActiveRecord
-
-          # Detect special ordering and provide the reversed order
-          def self.lookup_custom_reverse_order(order_values)
-            if ordering_by_merged_at_and_mr_id_desc?(order_values)
-              [
-                Gitlab::Database.nulls_first_order('merge_request_metrics.merged_at', 'ASC'), # reversing the order
-                MergeRequest.arel_table[:id].asc
-              ]
-            elsif ordering_by_merged_at_and_mr_id_asc?(order_values)
-              [
-                Gitlab::Database.nulls_first_order('merge_request_metrics.merged_at', 'DESC'),
-                MergeRequest.arel_table[:id].asc
-              ]
-            end
-          end
-
-          def self.ordering_by_merged_at_and_mr_id_desc?(order_values)
-            order_values.size == 2 &&
-              order_values.first.to_s == Gitlab::Database.nulls_last_order('merge_request_metrics.merged_at', 'DESC') &&
-              order_values.last.is_a?(Arel::Nodes::Descending) &&
-              order_values.last.to_sql == MergeRequest.arel_table[:id].desc.to_sql
-          end
-
-          def self.ordering_by_merged_at_and_mr_id_asc?(order_values)
-            order_values.size == 2 &&
-              order_values.first.to_s == Gitlab::Database.nulls_last_order('merge_request_metrics.merged_at', 'ASC') &&
-              order_values.last.is_a?(Arel::Nodes::Descending) &&
-              order_values.last.to_sql == MergeRequest.arel_table[:id].desc.to_sql
-          end
-
-          private_class_method :ordering_by_merged_at_and_mr_id_desc?
-          private_class_method :ordering_by_merged_at_and_mr_id_asc?
         end
       end
     end

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

@@ -92,8 +92,6 @@ module Gitlab
           def extract_attribute_values(order_value)
             if ordering_by_lower?(order_value)
               [order_value.expr.expressions[0].name.to_s, order_value.direction, order_value.expr]
-            elsif ordering_by_similarity?(order_value)
-              ['similarity', order_value.direction, order_value.expr]
             elsif ordering_by_case?(order_value)
               ['case_order_value', order_value.direction, order_value.expr]
             elsif ordering_by_array_position?(order_value)
@@ -113,11 +111,6 @@ module Gitlab
             order_value.expr.is_a?(Arel::Nodes::NamedFunction) && order_value.expr&.name&.downcase == 'array_position'
           end
 
-          # determine if ordering using SIMILARITY scoring based on Gitlab::Database::SimilarityScore
-          def ordering_by_similarity?(order_value)
-            Gitlab::Database::SimilarityScore.order_by_similarity?(order_value)
-          end
-
           # determine if ordering using CASE
           def ordering_by_case?(order_value)
             order_value.expr.is_a?(Arel::Nodes::Case)

lib/gitlab/pagination/keyset/column_order_definition.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Pagination
+    module Keyset
+      # This class stores information for one column (or SQL expression) which can be used in an
+      # ORDER BY SQL clasue.
+      # The goal of this class is to encapsulate all the metadata in one place which are needed to
+      # make keyset pagination work in a generalized way.
+      #
+      # == Arguments
+      #
+      # **order expression** (Arel::Nodes::Node | String)
+      #
+      # The actual SQL expression for the ORDER BY clause.
+      #
+      # Examples:
+      #   # Arel column order definition
+      #   Project.arel_table[:id].asc # ORDER BY projects.id ASC
+      #
+      #   # Arel expression, calculated order definition
+      #   Arel::Nodes::NamedFunction.new("COALESCE", [Project.arel_table[:issue_count].asc, 0]).asc # ORDER BY COALESCE(projects.issue_count, 0)
+      #
+      #   # Another Arel expression
+      #   Arel::Nodes::Multiplication(Issue.arel_table[:weight], Issue.arel_table[:time_spent]).desc
+      #
+      #   # Raw string order definition
+      #   'issues.type DESC NULLS LAST'
+      #
+      # **column_expression** (Arel::Nodes::Node | String)
+      #
+      # Expression for the database column or an expression. This value will be used with logical operations (>, <, =, !=)
+      # when building the database query for the next page.
+      #
+      # Examples:
+      #   # Arel column reference
+      #   Issue.arel_table[:title]
+      #
+      #   # Calculated value
+      #   Arel::Nodes::Multiplication(Issue.arel_table[:weight], Issue.arel_table[:time_spent])
+      #
+      # **attribute_name** (String | Symbol)
+      #
+      # An attribute on the loaded ActiveRecord model where the value can be obtained.
+      #
+      # Examples:
+      #   # Simple attribute definition
+      #   attribute_name = :title
+      #
+      #   # Later on this attribute will be used like this:
+      #   my_record = Issue.find(x)
+      #   value = my_record[attribute_name] # reads data from the title column
+      #
+      #   # Calculated value based on an Arel or raw SQL expression
+      #
+      #   attribute_name = :lowercase_title
+      #
+      #   # `lowercase_title` is not is not a table column therefore we need to make sure it's available in the `SELECT` clause
+      #
+      #   my_record = Issue.select(:id, 'LOWER(title) as lowercase_title').last
+      #   value = my_record[:lowercase_title]
+      #
+      # **distinct**
+      #
+      # Boolean value.
+      #
+      # Tells us whether the database column contains only distinct values. If the column is covered by
+      # a unique index then set to true.
+      #
+      # **nullable** (:not_nullable | :nulls_last | :nulls_first)
+      #
+      # Tells us whether the database column is nullable or not. This information can be
+      # obtained from the DB schema.
+      #
+      # If the column is not nullable, set this attribute to :not_nullable.
+      #
+      # If the column is nullable, then additional information is needed. Based on the ordering, the null values
+      # will show up at the top or at the bottom of the resultset.
+      #
+      # Examples:
+      #     # Nulls are showing up at the top (for example: ORDER BY column ASC):
+      #     nullable = :nulls_first
+      #
+      #     # Nulls are showing up at the bottom (for example: ORDER BY column DESC):
+      #     nullable = :nulls_last
+      #
+      # **order_direction**
+      #
+      # :asc or :desc
+      #
+      # Note: this is an optional attribute, the value will be inferred from the order_expression.
+      # Sometimes it's not possible to infer the order automatically. In this case an exception will be
+      # raised (when the query is executed). If the reverse order cannot be computed, it must be provided explicitly.
+      #
+      # **reversed_order_expression**
+      #
+      # The reversed version of the order_expression.
+      #
+      # A ColumnOrderDefinition object is able to reverse itself which is used when paginating backwards.
+      # When a complex order_expression is provided (raw string), then reversing the order automatically
+      # is not possible. In this case an exception will be raised.
+      #
+      # Example:
+      #
+      #   order_expression = Project.arel_table[:id].asc
+      #   reversed_order_expression = Project.arel_table[:id].desc
+      #
+      # **add_to_projections**
+      #
+      # Set to true if the column is not part of the queried table. (Not part of SELECT *)
+      #
+      #  Example:
+      #
+      #  - When the order is a calculated expression or the column is in another table (JOIN-ed)
+      #
+      #  If the add_to_projections is true, the query builder will automatically add the column to the SELECT values
+      class ColumnOrderDefinition
+        REVERSED_ORDER_DIRECTIONS = { asc: :desc, desc: :asc }.freeze
+        REVERSED_NULL_POSITIONS = { nulls_first: :nulls_last, nulls_last: :nulls_first }.freeze
+        AREL_ORDER_CLASSES = { Arel::Nodes::Ascending => :asc, Arel::Nodes::Descending => :desc }.freeze
+        ALLOWED_NULLABLE_VALUES = [:not_nullable, :nulls_first, :nulls_last].freeze
+
+        attr_reader :attribute_name, :column_expression, :order_expression, :add_to_projections
+
+        def initialize(attribute_name:, order_expression:, column_expression: nil, reversed_order_expression: nil, nullable: :not_nullable, distinct: true, order_direction: nil, add_to_projections: false)
+          @attribute_name = attribute_name
+          @order_expression = order_expression
+          @column_expression = column_expression || calculate_column_expression(order_expression)
+          @distinct = distinct
+          @reversed_order_expression = reversed_order_expression || calculate_reversed_order(order_expression)
+          @nullable = parse_nullable(nullable, distinct)
+          @order_direction = parse_order_direction(order_expression, order_direction)
+          @add_to_projections = add_to_projections
+        end
+
+        def reverse
+          self.class.new(
+            attribute_name: attribute_name,
+            column_expression: column_expression,
+            order_expression: reversed_order_expression,
+            reversed_order_expression: order_expression,
+            nullable: not_nullable? ? :not_nullable : REVERSED_NULL_POSITIONS[nullable],
+            distinct: distinct,
+            order_direction: REVERSED_ORDER_DIRECTIONS[order_direction]
+          )
+        end
+
+        def ascending_order?
+          order_direction == :asc
+        end
+
+        def descending_order?
+          order_direction == :desc
+        end
+
+        def nulls_first?
+          nullable == :nulls_first
+        end
+
+        def nulls_last?
+          nullable == :nulls_last
+        end
+
+        def not_nullable?
+          nullable == :not_nullable
+        end
+
+        def nullable?
+          !not_nullable?
+        end
+
+        def distinct?
+          distinct
+        end
+
+        private
+
+        attr_reader :reversed_order_expression, :nullable, :distinct, :order_direction
+
+        def calculate_reversed_order(order_expression)
+          unless AREL_ORDER_CLASSES.has_key?(order_expression.class) # Arel can reverse simple orders
+            raise "Couldn't determine reversed order for `#{order_expression}`, please provide the `reversed_order_expression` parameter."
+          end
+
+          order_expression.reverse
+        end
+
+        def calculate_column_expression(order_expression)
+          if order_expression.respond_to?(:expr)
+            order_expression.expr
+          else
+            raise("Couldn't calculate the column expression. Please pass an ARel node as the order_expression, not a string.")
+          end
+        end
+
+        def parse_order_direction(order_expression, order_direction)
+          transformed_order_direction = if order_direction.nil? && AREL_ORDER_CLASSES[order_expression.class]
+                                          AREL_ORDER_CLASSES[order_expression.class]
+                                        elsif order_direction.present?
+                                          order_direction.to_s.downcase.to_sym
+                                        end
+
+          unless REVERSED_ORDER_DIRECTIONS.has_key?(transformed_order_direction)
+            raise "Invalid or missing `order_direction` (value: #{order_direction}) was given, the allowed values are: :asc or :desc"
+          end
+
+          transformed_order_direction
+        end
+
+        def parse_nullable(nullable, distinct)
+          if ALLOWED_NULLABLE_VALUES.exclude?(nullable)
+            raise "Invalid `nullable` is given (value: #{nullable}), the allowed values are: #{ALLOWED_NULLABLE_VALUES.join(', ')}"
+          end
+
+          if nullable != :not_nullable && distinct
+            raise 'Invalid column definition, `distinct` and `nullable` columns are not allowed at the same time'
+          end
+
+          nullable
+        end
+      end
+    end
+  end
+end

lib/gitlab/pagination/keyset/order.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Pagination
+    module Keyset
+      # This class is a special ORDER BY clause which is compatible with ActiveRecord. It helps
+      # building keyset paginated queries.
+      #
+      # In ActiveRecord we use the `order()` method which will generate the `ORDER BY X` SQL clause
+      #
+      # Project.where(active: true).order(id: :asc)
+      #
+      # # Or
+      #
+      # Project.where(active: true).order(created_at: :asc, id: desc)
+      #
+      # Gitlab::Pagination::Keyset::Order class encapsulates more information about the order columns
+      # in order to implement keyset pagination in a generic way
+      #
+      # - Extract values from a record (usually the last item of the previous query)
+      # - Build query conditions based on the column configuration
+      #
+      # Example 1: Order by primary key
+      #
+      #   # Simple order definition for the primary key as an ActiveRecord scope
+      #   scope :id_asc_ordered, -> {
+      #     keyset_order = Gitlab::Pagination::Keyset::Order.build([
+      #       Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
+      #         attribute: :id,
+      #         order_expression: Project.arel_table[:id].asc
+      #       )
+      #     ])
+      #
+      #     reorder(keyset_order)
+      #   }
+      #
+      #   # ... Later in the application code:
+      #
+      #   # Compatible with ActiveRecord's `order()` method
+      #   page1 = Project.where(active: true).id_asc_ordered.limit(5)
+      #   keyset_order = Gitlab::Pagination::Keyset::Order.extract_keyset_order_object(page1)
+      #
+      #   last_record = page1.last
+      #   cursor_values = keyset_order.cursor_attributes_for_node(last_record) # { id: x }
+      #
+      #   page2 = keyset_order.apply_cursor_conditions(Project.where(active: true).id_asc_ordered, cursor_values).limit(5)
+      #
+      #   last_record = page2.last
+      #   cursor_values = keyset_order.cursor_attributes_for_node(last_record)
+      #
+      #   page3 = keyset_order.apply_cursor_conditions(Project.where(active: true).id_asc_ordered, cursor_values).limit(5)
+      #
+      # Example 2: Order by creation time and primary key (primary key is the tie breaker)
+      #
+      #   scope :created_at_ordered, -> {
+      #     keyset_order = Gitlab::Pagination::Keyset::Order.build([
+      #       Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
+      #         attribute: :created_at,
+      #         column_expression: Project.arel_table[:created_at],
+      #         order_expression: Project.arel_table[:created_at].asc,
+      #         distinct: false, # values in the column are not unique
+      #         nullable: :nulls_last # we might see NULL values (bottom)
+      #       ),
+      #       Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
+      #         attribute: :id,
+      #         order_expression: Project.arel_table[:id].asc
+      #       )
+      #     ])
+      #
+      #     reorder(keyset_order)
+      #   }
+      #
+      class Order < Arel::Nodes::SqlLiteral
+        attr_reader :column_definitions
+
+        def initialize(column_definitions:)
+          @column_definitions = column_definitions
+
+          super(to_sql_literal(@column_definitions))
+        end
+
+        # Tells whether the given ActiveRecord::Relation has keyset ordering
+        def self.keyset_aware?(scope)
+          scope.order_values.first.is_a?(self) && scope.order_values.one?
+        end
+
+        def self.extract_keyset_order_object(scope)
+          scope.order_values.first
+        end
+
+        def self.build(column_definitions)
+          new(column_definitions: column_definitions)
+        end
+
+        def cursor_attributes_for_node(node)
+          column_definitions.each_with_object({}) do |column_definition, hash|
+            field_value = node[column_definition.attribute_name]
+            hash[column_definition.attribute_name] = if field_value.is_a?(Time)
+                                                       field_value.strftime('%Y-%m-%d %H:%M:%S.%N %Z')
+                                                     elsif field_value.nil?
+                                                       nil
+                                                     else
+                                                       field_value.to_s
+                                                     end
+          end
+        end
+
+        # This methods builds the conditions for the keyset pagination
+        #
+        # Example:
+        #
+        # |created_at|id|
+        # |----------|--|
+        # |2020-01-01| 1|
+        # |      null| 2|
+        # |      null| 3|
+        # |2020-02-01| 4|
+        #
+        # Note: created_at is not distinct and nullable
+        # Order `ORDER BY created_at DESC, id DESC`
+        #
+        # We get the following cursor values from the previous page:
+        # { id: 4, created_at: '2020-02-01' }
+        #
+        # To get the next rows, we need to build the following conditions:
+        #
+        # (created_at = '2020-02-01' AND id < 4) OR (created_at < '2020-01-01')
+        #
+        # DESC ordering ensures that NULL values are on top so we don't need conditions for NULL values
+        #
+        # Another cursor example:
+        # { id: 3, created_at: nil }
+        #
+        # To get the next rows, we need to build the following conditions:
+        #
+        # (id < 3 AND created_at IS NULL) OR (created_at IS NOT NULL)
+        def build_where_values(values)
+          return if values.blank?
+
+          verify_incoming_values!(values)
+
+          where_values = []
+
+          reversed_column_definitions = column_definitions.reverse
+          reversed_column_definitions.each_with_index do |column_definition, i|
+            value = values[column_definition.attribute_name]
+
+            conditions_for_column(column_definition, value).each do |condition|
+              column_definitions_after_index = reversed_column_definitions.last(column_definitions.reverse.size - i - 1)
+
+              equal_conditon_for_rest = column_definitions_after_index.map do |definition|
+                definition.column_expression.eq(values[definition.attribute_name])
+              end
+
+              where_values << Arel::Nodes::Grouping.new(Arel::Nodes::And.new([condition, *equal_conditon_for_rest].compact))
+            end
+          end
+
+          build_or_query(where_values)
+        end
+
+        # rubocop: disable CodeReuse/ActiveRecord
+        def apply_cursor_conditions(scope, values = {})
+          scope = apply_custom_projections(scope)
+          scope.where(build_where_values(values))
+        end
+        # rubocop: enable CodeReuse/ActiveRecord
+
+        def reversed_order
+          self.class.build(column_definitions.map(&:reverse))
+        end
+
+        private
+
+        # Adds extra columns to the SELECT clause
+        def apply_custom_projections(scope)
+          additional_projections = column_definitions.select(&:add_to_projections).map do |column_definition|
+            # avoid mutating the original column_expression
+            column_definition.column_expression.dup.as(column_definition.attribute_name).to_sql
+          end
+
+          scope = scope.select(*scope.arel.projections, *additional_projections) if additional_projections
+          scope
+        end
+
+        def conditions_for_column(column_definition, value)
+          conditions = []
+          # Depending on the order, build a query condition fragment for taking the next rows
+          if column_definition.distinct? || (!column_definition.distinct? && value.present?)
+            conditions << compare_column_with_value(column_definition, value)
+          end
+
+          # When the column is nullable, additional conditions for NULL a NOT NULL values are necessary.
+          # This depends on the position of the nulls (top or bottom of the resultset).
+          if column_definition.nulls_first? && value.blank?
+            conditions << column_definition.column_expression.not_eq(nil)
+          elsif column_definition.nulls_last? && value.present?
+            conditions << column_definition.column_expression.eq(nil)
+          end
+
+          conditions
+        end
+
+        def compare_column_with_value(column_definition, value)
+          if column_definition.descending_order?
+            column_definition.column_expression.lt(value)
+          else
+            column_definition.column_expression.gt(value)
+          end
+        end
+
+        def build_or_query(expressions)
+          or_expression = expressions.reduce { |or_expression, expression| Arel::Nodes::Or.new(or_expression, expression) }
+
+          Arel::Nodes::Grouping.new(or_expression)
+        end
+
+        def to_sql_literal(column_definitions)
+          column_definitions.map do |column_definition|
+            if column_definition.order_expression.respond_to?(:to_sql)
+              column_definition.order_expression.to_sql
+            else
+              column_definition.order_expression.to_s
+            end
+          end.join(', ')
+        end
+
+        def verify_incoming_values!(values)
+          value_keys = values.keys.map(&:to_s)
+          order_attrbute_names = column_definitions.map(&:attribute_name).map(&:to_s)
+          missing_items = order_attrbute_names - value_keys
+          extra_items = value_keys - order_attrbute_names
+
+          if missing_items.any? || extra_items.any?
+            error_text = ['Incorrect cursor values were given']
+
+            error_text << "Extra items: #{extra_items.join(', ')}" if extra_items.any?
+            error_text << "Missing items: #{missing_items.join(', ')}" if missing_items.any?
+
+            error_text.compact
+
+            raise error_text.join('. ')
+          end
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/database/similarity_score_spec.rb

@@ -71,7 +71,7 @@ RSpec.describe Gitlab::Database::SimilarityScore do
       let(:search) { 'xyz' }
 
       it 'results have 0 similarity score' do
-        expect(query_result.map { |row| row['similarity'] }).to all(eq(0))
+        expect(query_result.map { |row| row['similarity'].to_f }).to all(eq(0))
       end
     end
   end

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

@@ -4,7 +4,7 @@ require 'spec_helper'
 
 RSpec.describe Gitlab::Graphql::Pagination::Keyset::LastItems do
   let_it_be(:merge_request) { create(:merge_request) }
-  let(:scope) { MergeRequest.order_merged_at_asc.with_order_id_desc }
+  let(:scope) { MergeRequest.order_merged_at_asc }
 
   subject { described_class.take_items(*args) }
 

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

@@ -52,18 +52,6 @@ RSpec.describe Gitlab::Graphql::Pagination::Keyset::OrderInfo do
       end
     end
 
-    context 'when ordering by SIMILARITY' do
-      let(:relation) { Project.sorted_by_similarity_desc('test', include_in_select: true) }
-
-      it 'assigns the right attribute name, named function, and direction' do
-        expect(order_list.count).to eq 2
-        expect(order_list.first.attribute_name).to eq 'similarity'
-        expect(order_list.first.named_function).to be_kind_of(Arel::Nodes::Addition)
-        expect(order_list.first.named_function.to_sql).to include 'SIMILARITY('
-        expect(order_list.first.sort_direction).to eq :desc
-      end
-    end
-
     context 'when ordering by CASE', :aggregate_failuers do
       let(:relation) { Project.order(Arel::Nodes::Case.new(Project.arel_table[:pending_delete]).when(true).then(100).else(1000).asc) }
 

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

@@ -131,43 +131,5 @@ RSpec.describe Gitlab::Graphql::Pagination::Keyset::QueryBuilder do
         end
       end
     end
-
-    context 'when sorting using SIMILARITY' do
-      let(:relation) { Project.sorted_by_similarity_desc('test', include_in_select: true) }
-      let(:arel_table) { Project.arel_table }
-      let(:decoded_cursor) { { 'similarity' => 0.5, 'id' => 100 } }
-      let(:similarity_function_call) { Gitlab::Database::SimilarityScore::SIMILARITY_FUNCTION_CALL_WITH_ANNOTATION }
-      let(:similarity_sql) do
-        [
-          "(#{similarity_function_call}(COALESCE(\"projects\".\"path\", ''), 'test') * CAST('1' AS numeric))",
-          "(#{similarity_function_call}(COALESCE(\"projects\".\"name\", ''), 'test') * CAST('0.7' AS numeric))",
-          "(#{similarity_function_call}(COALESCE(\"projects\".\"description\", ''), 'test') * CAST('0.2' AS numeric))"
-        ].join(' + ')
-      end
-
-      context 'when no values are nil' do
-        context 'when :after' do
-          it 'generates the correct condition' do
-            conditions = builder.conditions.gsub(/\s+/, ' ')
-
-            expect(conditions).to include "(#{similarity_sql} < 0.5)"
-            expect(conditions).to include '"projects"."id" < 100'
-            expect(conditions).to include "OR (#{similarity_sql} IS NULL)"
-          end
-        end
-
-        context 'when :before' do
-          let(:before_or_after) { :before }
-
-          it 'generates the correct condition' do
-            conditions = builder.conditions.gsub(/\s+/, ' ')
-
-            expect(conditions).to include "(#{similarity_sql} > 0.5)"
-            expect(conditions).to include '"projects"."id" > 100'
-            expect(conditions).to include "OR ( #{similarity_sql} = 0.5"
-          end
-        end
-      end
-    end
   end
 end

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Pagination::Keyset::ColumnOrderDefinition do
+  let_it_be(:project_name_column) do
+    described_class.new(
+      attribute_name: :name,
+      order_expression: Project.arel_table[:name].asc,
+      nullable: :not_nullable,
+      distinct: true
+    )
+  end
+
+  let_it_be(:project_name_lower_column) do
+    described_class.new(
+      attribute_name: :name,
+      order_expression: Project.arel_table[:name].lower.desc,
+      nullable: :not_nullable,
+      distinct: true
+    )
+  end
+
+  let_it_be(:project_calculated_column_expression) do
+    # COALESCE("projects"."description", 'No Description')
+    Arel::Nodes::NamedFunction.new('COALESCE', [
+      Project.arel_table[:description],
+      Arel.sql("'No Description'")
+    ])
+  end
+
+  let_it_be(:project_calculated_column) do
+    described_class.new(
+      attribute_name: :name,
+      column_expression: project_calculated_column_expression,
+      order_expression: project_calculated_column_expression.asc,
+      nullable: :not_nullable,
+      distinct: true
+    )
+  end
+
+  describe '#order_direction' do
+    context 'inferring order_direction from order_expression' do
+      it { expect(project_name_column).to be_ascending_order }
+      it { expect(project_name_column).not_to be_descending_order }
+
+      it { expect(project_name_lower_column).to be_descending_order }
+      it { expect(project_name_lower_column).not_to be_ascending_order }
+
+      it { expect(project_calculated_column).to be_ascending_order }
+      it { expect(project_calculated_column).not_to be_descending_order }
+
+      it 'raises error when order direction cannot be infered' do
+        expect do
+          described_class.new(
+            attribute_name: :name,
+            column_expression: Project.arel_table[:name],
+            order_expression: 'name asc',
+            reversed_order_expression: 'name desc',
+            nullable: :not_nullable,
+            distinct: true
+          )
+        end.to raise_error(RuntimeError, /Invalid or missing `order_direction`/)
+      end
+
+      it 'does not raise error when order direction is explicitly given' do
+        column_order_definition = described_class.new(
+          attribute_name: :name,
+          column_expression: Project.arel_table[:name],
+          order_expression: 'name asc',
+          reversed_order_expression: 'name desc',
+          order_direction: :asc,
+          nullable: :not_nullable,
+          distinct: true
+        )
+
+        expect(column_order_definition).to be_ascending_order
+      end
+    end
+  end
+
+  describe '#column_expression' do
+    context 'inferring column_expression from order_expression' do
+      it 'infers the correct column expression' do
+        column_order_definition = described_class.new(attribute_name: :name, order_expression: Project.arel_table[:name].asc)
+
+        expect(column_order_definition.column_expression).to eq(Project.arel_table[:name])
+      end
+
+      it 'raises error when raw string is given as order expression' do
+        expect do
+          described_class.new(attribute_name: :name, order_expression: 'name DESC')
+        end.to raise_error(RuntimeError, /Couldn't calculate the column expression. Please pass an ARel node/)
+      end
+    end
+  end
+
+  describe '#reversed_order_expression' do
+    it 'raises error when order cannot be reversed automatically' do
+      expect do
+        described_class.new(
+          attribute_name: :name,
+          column_expression: Project.arel_table[:name],
+          order_expression: 'name asc',
+          order_direction: :asc,
+          nullable: :not_nullable,
+          distinct: true
+        )
+      end.to raise_error(RuntimeError, /Couldn't determine reversed order/)
+    end
+  end
+
+  describe '#reverse' do
+    it { expect(project_name_column.reverse.order_expression).to eq(Project.arel_table[:name].desc) }
+    it { expect(project_name_column.reverse).to be_descending_order }
+
+    it { expect(project_calculated_column.reverse.order_expression).to eq(project_calculated_column_expression.desc) }
+    it { expect(project_calculated_column.reverse).to be_descending_order }
+
+    context 'when reversed_order_expression is given' do
+      it 'uses the given expression' do
+        column_order_definition = described_class.new(
+          attribute_name: :name,
+          column_expression: Project.arel_table[:name],
+          order_expression: 'name asc',
+          reversed_order_expression: 'name desc',
+          order_direction: :asc,
+          nullable: :not_nullable,
+          distinct: true
+        )
+
+        expect(column_order_definition.reverse.order_expression).to eq('name desc')
+      end
+    end
+  end
+
+  describe '#nullable' do
+    context 'when the column is nullable' do
+      let(:nulls_last_order) 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_direction: :desc,
+          nullable: :nulls_last, # null values are always last
+          distinct: false
+        )
+      end
+
+      it 'requires the position of the null values in the result' do
+        expect(nulls_last_order).to be_nulls_last
+      end
+
+      it 'reverses nullable correctly' do
+        expect(nulls_last_order.reverse).to be_nulls_first
+      end
+
+      it 'raises error when invalid nullable value is given' do
+        expect 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_direction: :desc,
+            nullable: true,
+            distinct: false
+          )
+        end.to raise_error(RuntimeError, /Invalid `nullable` is given/)
+      end
+
+      it 'raises error when the column is nullable and distinct' do
+        expect 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_direction: :desc,
+            nullable: :nulls_last,
+            distinct: true
+          )
+        end.to raise_error(RuntimeError, /Invalid column definition/)
+      end
+    end
+  end
+end

spec/requests/api/graphql/project/merge_requests_spec.rb

@@ -381,29 +381,41 @@ RSpec.describe 'getting merge request listings nested in a project' do
     end
 
     context 'when sorting by merged_at DESC' do
-      it_behaves_like 'sorted paginated query' do
-        let(:sort_param) { :MERGED_AT_DESC }
-        let(:first_param) { 2 }
+      let(:sort_param) { :MERGED_AT_DESC }
+      let(:expected_results) do
+        [
+          merge_request_b,
+          merge_request_d,
+          merge_request_c,
+          merge_request_e,
+          merge_request_a
+        ].map { |mr| global_id_of(mr) }
+      end
 
-        let(:expected_results) do
-          [
-            merge_request_b,
-            merge_request_d,
-            merge_request_c,
-            merge_request_e,
-            merge_request_a
-          ].map { |mr| global_id_of(mr) }
-        end
+      before do
+        five_days_ago = 5.days.ago
+
+        merge_request_d.metrics.update!(merged_at: five_days_ago)
 
-        before do
-          five_days_ago = 5.days.ago
+        # same merged_at, the second order column will decide (merge_request.id)
+        merge_request_c.metrics.update!(merged_at: five_days_ago)
+
+        merge_request_b.metrics.update!(merged_at: 1.day.ago)
+      end
+
+      it_behaves_like 'sorted paginated query' do
+        let(:first_param) { 2 }
+      end
 
-          merge_request_d.metrics.update!(merged_at: five_days_ago)
+      context 'when last parameter is given' do
+        let(:params) { graphql_args(sort: sort_param, last: 2) }
+        let(:page_info) { nil }
 
-          # same merged_at, the second order column will decide (merge_request.id)
-          merge_request_c.metrics.update!(merged_at: five_days_ago)
+        it 'takes the last 2 records' do
+          query = pagination_query(params)
+          post_graphql(query, current_user: current_user)
 
-          merge_request_b.metrics.update!(merged_at: 1.day.ago)
+          expect(results.map { |item| item["id"] }).to eq(expected_results.last(2))
         end
       end
     end