Skip to content

Commit

Permalink
Support reversing of Arel expressions in reverse_order
Browse files Browse the repository at this point in the history
The current behaviour of reverse_order, when presented with an
existing order that is an Arel expression, is to ignore it.
This isn't quite in keeping with how it handles Arel attributes
(which it reverse) or arbitrary strings (which throw an exception).

The easy path would be to throw an exception, but we don't have to.
Any SQL expression that is a valid default sort expression can be
reversed with DESC.

It follows that if there's an existing Arel::Nodes::NodeExpression
that is not already wrapped with an Arel::Nodes::Ordering, then
it can be reversed with DESC.
  • Loading branch information
inopinatus committed Sep 4, 2020
1 parent 7da82b0 commit e32fd53
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 0 deletions.
2 changes: 2 additions & 0 deletions activerecord/lib/active_record/relation/query_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1360,6 +1360,8 @@ def reverse_sql_order(order_query)
o.desc
when Arel::Nodes::Ordering
o.reverse
when Arel::Nodes::NodeExpression
o.desc
when String
if does_not_support_reverse?(o)
raise IrreversibleOrderError, "Order #{o.inspect} cannot be reversed automatically"
Expand Down
5 changes: 5 additions & 0 deletions activerecord/test/cases/relations_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,11 @@ def test_reverse_order_with_function
assert_equal topics(:third).title, topics.first.title
end

def test_reverse_arel_order_with_function
topics = Topic.order(Topic.arel_table[:title].lower).reverse_order
assert_equal topics(:third).title, topics.first.title
end

def test_reverse_arel_assoc_order_with_function
topics = Topic.order(Arel.sql("lower(title)") => :asc).reverse_order
assert_equal topics(:third).title, topics.first.title
Expand Down

0 comments on commit e32fd53

Please sign in to comment.