From 14efb42a906aa8079adff2b1e56d6444d147a386 Mon Sep 17 00:00:00 2001 From: Bogdan Gusiev Date: Wed, 27 Jan 2016 12:53:51 +0200 Subject: Introduce ActiveRecord::IrreversibleOrderError Raises when #reverse_order can not process SQL order instead of making invalid SQL before this patch --- activerecord/lib/active_record/errors.rb | 5 +++++ .../lib/active_record/relation/query_methods.rb | 18 ++++++++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/errors.rb b/activerecord/lib/active_record/errors.rb index e5906b6756..87f32c042c 100644 --- a/activerecord/lib/active_record/errors.rb +++ b/activerecord/lib/active_record/errors.rb @@ -275,4 +275,9 @@ module ActiveRecord # The mysql2 and postgresql adapters support setting the transaction isolation level. class TransactionIsolationError < ActiveRecordError end + + # IrreversibleOrderError is raised when a relation's order is too complex for + # +reverse_order+ to automatically reverse. + class IrreversibleOrderError < ActiveRecordError + end end diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 716b1e8505..8ef9f9f627 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1104,14 +1104,21 @@ module ActiveRecord end def reverse_sql_order(order_query) - order_query = ["#{quoted_table_name}.#{quoted_primary_key} ASC"] if order_query.empty? + if order_query.empty? + return [table[primary_key].desc] if primary_key + raise IrreversibleOrderError, + "Relation has no current order and table has no primary key to be used as default order" + end order_query.flat_map do |o| case o when Arel::Nodes::Ordering o.reverse when String - o.to_s.split(',').map! do |s| + if does_not_support_reverse?(o) + raise IrreversibleOrderError, "Order #{o.inspect} can not be reversed automatically" + end + o.split(',').map! do |s| s.strip! s.gsub!(/\sasc\Z/i, ' DESC') || s.gsub!(/\sdesc\Z/i, ' ASC') || s.concat(' DESC') end @@ -1121,6 +1128,13 @@ module ActiveRecord end end + def does_not_support_reverse?(order) + #uses sql function with multiple arguments + order =~ /\([^()]*,[^()]*\)/ || + # uses "nulls first" like construction + order =~ /nulls (first|last)\Z/i + end + def build_order(arel) orders = order_values.uniq orders.reject!(&:blank?) -- cgit v1.2.3