Skip to content

Commit

Permalink
Fix ActiveRecord::QueryMethods#in_order_of to work with nils
Browse files Browse the repository at this point in the history
  • Loading branch information
fatkodima committed Aug 4, 2023
1 parent 36597e2 commit d25e34d
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 19 deletions.
8 changes: 8 additions & 0 deletions activerecord/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
* Fix `ActiveRecord::QueryMethods#in_order_of` to include `nil`s, to match the
behavior of `Enumerable#in_order_of`.

For example, `Post.in_order_of(:title, [nil, "foo"])` will now include posts
with `nil` titles, the same as `Post.all.to_a.in_order_of(:title, [nil, "foo"])`.

*fatkodima*

* Revert "Fix autosave associations with validations added on `:base` of the associated objects."

This change intended to remove the :base attribute from the message,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -657,15 +657,6 @@ def schema_version
migration_context.current_version
end

def field_ordered_value(column, values) # :nodoc:
node = Arel::Nodes::Case.new(column)
values.each.with_index(1) do |value, order|
node.when(value).then(order)
end

Arel::Nodes::Ascending.new(node.else(values.length + 1))
end

class << self
private
def initialize_type_map(m)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,11 +138,6 @@ def supports_insert_on_duplicate_update?
true
end

def field_ordered_value(column, values) # :nodoc:
field = Arel::Nodes::NamedFunction.new("FIELD", [column, values.reverse.map { |value| Arel::Nodes.build_quoted(value) }])
Arel::Nodes::Descending.new(field)
end

def get_advisory_lock(lock_name, timeout = 0) # :nodoc:
query_value("SELECT GET_LOCK(#{quote(lock_name.to_s)}, #{timeout})") == 1
end
Expand Down
29 changes: 24 additions & 5 deletions activerecord/lib/active_record/relation/query_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -444,13 +444,16 @@ def order!(*args) # :nodoc:
self
end

# Allows to specify an order by a specific set of values. Depending on your
# adapter this will either use a CASE statement or a built-in function.
# Allows to specify an order by a specific set of values.
#
# User.in_order_of(:id, [1, 5, 3])
# # SELECT "users".* FROM "users"
# # ORDER BY FIELD("users"."id", 1, 5, 3)
# # WHERE "users"."id" IN (1, 5, 3)
# # ORDER BY CASE
# # WHEN "users"."id" = 1 THEN 1
# # WHEN "users"."id" = 5 THEN 2
# # WHEN "users"."id" = 3 THEN 3
# # END ASC
#
def in_order_of(column, values)
klass.disallow_raw_sql!([column], permit: connection.column_name_with_order_matcher)
Expand All @@ -462,9 +465,16 @@ def in_order_of(column, values)
values = values.map { |value| type_caster.type_cast_for_database(column, value) }
arel_column = column.is_a?(Symbol) ? order_column(column.to_s) : column

where_clause =
if values.include?(nil)
arel_column.in(values.compact).or(arel_column.eq(nil))
else
arel_column.in(values)
end

spawn
.order!(connection.field_ordered_value(arel_column, values))
.where!(arel_column.in(values))
.order!(build_case_for_value_position(arel_column, values))
.where!(where_clause)
end

# Replaces any existing order defined on the relation with the specified order.
Expand Down Expand Up @@ -1669,6 +1679,15 @@ def order_column(field)
end
end

def build_case_for_value_position(column, values)
node = Arel::Nodes::Case.new
values.each.with_index(1) do |value, order|
node.when(column.eq(value)).then(order)
end

Arel::Nodes::Ascending.new(node)
end

def resolve_arel_attributes(attrs)
attrs.flat_map do |attr|
case attr
Expand Down
12 changes: 12 additions & 0 deletions activerecord/test/cases/relation/field_ordered_values_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,16 @@ def test_in_order_of_after_regular_order

assert_equal(order, posts.map(&:id))
end

def test_in_order_of_with_nil
Book.destroy_all
Book.create!(format: "paperback")
Book.create!(format: "ebook")
Book.create!(format: nil)

order = ["ebook", nil, "paperback"]
books = Book.in_order_of(:format, order)

assert_equal(order, books.map(&:format))
end
end

0 comments on commit d25e34d

Please sign in to comment.