Commit 3828ec75 authored by Bob Van Landuyt's avatar Bob Van Landuyt

Merge branch '199536-graphql-extend-keyset-pagination-to-support-lower' into 'master'

GraphQL: Extend keyset pagination to support `LOWER()`

See merge request gitlab-org/gitlab!24011
parents 56550f91 e2c54a0f
......@@ -6,6 +6,12 @@ module Gitlab
module Keyset
module Conditions
class BaseCondition
# @param [Arel::Table] arel_table for the relation being ordered
# @param [Array<OrderInfo>] order_list of extracted orderings
# @param [Array] values from the decoded cursor
# @param [Array<String>] operators determining sort comparison
# @param [Symbol] before_or_after indicates whether we want
# items :before the cursor or :after the cursor
def initialize(arel_table, order_list, values, operators, before_or_after)
@arel_table, @order_list, @values, @operators, @before_or_after = arel_table, order_list, values, operators, before_or_after
......@@ -20,18 +26,25 @@ module Gitlab
attr_reader :arel_table, :order_list, :values, :operators, :before_or_after
def table_condition(attribute, value, operator)
def table_condition(order_info, value, operator)
if order_info.named_function
target = order_info.named_function
value = value&.downcase if target&.name&.downcase == 'lower'
else
target = arel_table[order_info.attribute_name]
end
case operator
when '>'
arel_table[attribute].gt(value)
target.gt(value)
when '<'
arel_table[attribute].lt(value)
target.lt(value)
when '='
arel_table[attribute].eq(value)
target.eq(value)
when 'is_null'
arel_table[attribute].eq(nil)
target.eq(nil)
when 'is_not_null'
arel_table[attribute].not_eq(nil)
target.not_eq(nil)
end
end
end
......
......@@ -5,10 +5,10 @@ module Gitlab
module Connections
module Keyset
class OrderInfo
attr_reader :attribute_name, :sort_direction
attr_reader :attribute_name, :sort_direction, :named_function
def initialize(order_value)
@attribute_name, @sort_direction =
@attribute_name, @sort_direction, @named_function =
if order_value.is_a?(String)
extract_nulls_last_order(order_value)
else
......@@ -69,11 +69,24 @@ module Gitlab
def extract_nulls_last_order(order_value)
tokens = order_value.downcase.split
[tokens.first, (tokens[1] == 'asc' ? :asc : :desc)]
[tokens.first, (tokens[1] == 'asc' ? :asc : :desc), nil]
end
def extract_attribute_values(order_value)
[order_value.expr.name, order_value.direction]
named = nil
name = if ordering_by_lower?(order_value)
named = order_value.expr
named.expressions[0].name.to_s
else
order_value.expr.name
end
[name, order_value.direction, named]
end
# determine if ordering using LOWER, eg. "ORDER BY LOWER(boards.name)"
def ordering_by_lower?(order_value)
order_value.expr.is_a?(Arel::Nodes::NamedFunction) && order_value.expr&.name&.downcase == 'lower'
end
end
end
......
......@@ -40,17 +40,16 @@ module Gitlab
# "issues"."id" > 500
#
def conditions
attr_names = order_list.map { |field| field.attribute_name }
attr_values = attr_names.map { |name| decoded_cursor[name] }
attr_values = order_list.map { |field| decoded_cursor[field.attribute_name] }
if attr_names.count == 1 && attr_values.first.nil?
if order_list.count == 1 && attr_values.first.nil?
raise Gitlab::Graphql::Errors::ArgumentError.new('Before/after cursor invalid: `nil` was provided as only sortable value')
end
if attr_names.count == 1 || attr_values.first.present?
Keyset::Conditions::NotNullCondition.new(arel_table, attr_names, attr_values, operators, before_or_after).build
if order_list.count == 1 || attr_values.first.present?
Keyset::Conditions::NotNullCondition.new(arel_table, order_list, attr_values, operators, before_or_after).build
else
Keyset::Conditions::NullCondition.new(arel_table, attr_names, attr_values, operators, before_or_after).build
Keyset::Conditions::NullCondition.new(arel_table, order_list, attr_values, operators, before_or_after).build
end
end
......
......@@ -10,7 +10,7 @@ describe Gitlab::Graphql::Connections::Keyset::Conditions::NotNullCondition do
context 'when there is only one ordering field' do
let(:arel_table) { Issue.arel_table }
let(:order_list) { ['id'] }
let(:order_list) { [double(named_function: nil, attribute_name: 'id')] }
let(:values) { [500] }
let(:operators) { ['>'] }
......@@ -25,7 +25,7 @@ describe Gitlab::Graphql::Connections::Keyset::Conditions::NotNullCondition do
context 'when ordering by a column attribute' do
let(:arel_table) { Issue.arel_table }
let(:order_list) { %w(relative_position id) }
let(:order_list) { [double(named_function: nil, attribute_name: 'relative_position'), double(named_function: nil, attribute_name: 'id')] }
let(:values) { [1500, 500] }
shared_examples ':after condition' do
......@@ -71,5 +71,45 @@ describe Gitlab::Graphql::Connections::Keyset::Conditions::NotNullCondition do
it_behaves_like ':after condition'
end
end
context 'when ordering by LOWER' do
let(:arel_table) { Project.arel_table }
let(:relation) { Project.order(arel_table['name'].lower.asc).order(:id) }
let(:order_list) { Gitlab::Graphql::Connections::Keyset::OrderInfo.build_order_list(relation) }
let(:values) { ['Test', 500] }
context 'when :after' do
it 'generates :after sql' do
expected_sql = <<~SQL
(LOWER("projects"."name") > 'test')
OR (
LOWER("projects"."name") = 'test'
AND
"projects"."id" > 500
)
OR (LOWER("projects"."name") IS NULL)
SQL
expect(condition.build.squish).to eq expected_sql.squish
end
end
context 'when :before' do
let(:before_or_after) { :before }
it 'generates :before sql' do
expected_sql = <<~SQL
(LOWER("projects"."name") > 'test')
OR (
LOWER("projects"."name") = 'test'
AND
"projects"."id" > 500
)
SQL
expect(condition.build.squish).to eq expected_sql.squish
end
end
end
end
end
......@@ -11,7 +11,7 @@ describe Gitlab::Graphql::Connections::Keyset::Conditions::NullCondition do
context 'when ordering by a column attribute' do
let(:arel_table) { Issue.arel_table }
let(:order_list) { %w(relative_position id) }
let(:order_list) { [double(named_function: nil, attribute_name: 'relative_position'), double(named_function: nil, attribute_name: 'id')] }
shared_examples ':after condition' do
it 'generates sql' do
......@@ -54,5 +54,42 @@ describe Gitlab::Graphql::Connections::Keyset::Conditions::NullCondition do
it_behaves_like ':after condition'
end
end
context 'when ordering by LOWER' do
let(:arel_table) { Project.arel_table }
let(:relation) { Project.order(arel_table['name'].lower.asc).order(:id) }
let(:order_list) { Gitlab::Graphql::Connections::Keyset::OrderInfo.build_order_list(relation) }
context 'when :after' do
it 'generates sql' do
expected_sql = <<~SQL
(
LOWER("projects"."name") IS NULL
AND
"projects"."id" > 500
)
SQL
expect(condition.build.squish).to eq expected_sql.squish
end
end
context 'when :before' do
let(:before_or_after) { :before }
it 'generates :before sql' do
expected_sql = <<~SQL
(
LOWER("projects"."name") IS NULL
AND
"projects"."id" > 500
)
OR (LOWER("projects"."name") IS NOT NULL)
SQL
expect(condition.build.squish).to eq expected_sql.squish
end
end
end
end
end
......@@ -37,6 +37,20 @@ describe Gitlab::Graphql::Connections::Keyset::OrderInfo do
expect(order_list.count).to eq 1
end
end
context 'when order contains LOWER' do
let(:relation) { Project.order(Arel::Table.new(:projects)['name'].lower.asc).order(:id) }
it 'does not ignore the SQL order' do
expect(order_list.count).to eq 2
expect(order_list.first.attribute_name).to eq 'name'
expect(order_list.first.named_function).to be_kind_of(Arel::Nodes::NamedFunction)
expect(order_list.first.named_function.to_sql).to eq 'LOWER("projects"."name")'
expect(order_list.first.operator_for(:after)).to eq '>'
expect(order_list.last.attribute_name).to eq 'id'
expect(order_list.last.operator_for(:after)).to eq '>'
end
end
end
describe '#validate_ordering' do
......
......@@ -101,5 +101,35 @@ describe Gitlab::Graphql::Connections::Keyset::QueryBuilder do
end
end
end
context 'when sorting using LOWER' do
let(:relation) { Project.order(Arel::Table.new(:projects)['name'].lower.asc).order(:id) }
let(:arel_table) { Project.arel_table }
let(:decoded_cursor) { { 'name' => 'Test', 'id' => 100 } }
context 'when no values are nil' do
context 'when :after' do
it 'generates the correct condition' do
conditions = builder.conditions
expect(conditions).to include '(LOWER("projects"."name") > \'test\')'
expect(conditions).to include '"projects"."id" > 100'
expect(conditions).to include 'OR (LOWER("projects"."name") IS NULL)'
end
end
context 'when :before' do
let(:before_or_after) { :before }
it 'generates the correct condition' do
conditions = builder.conditions
expect(conditions).to include '(LOWER("projects"."name") < \'test\')'
expect(conditions).to include '"projects"."id" < 100'
expect(conditions).to include 'LOWER("projects"."name") = \'test\''
end
end
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment