diff --git a/lib/extensions/ar_order.rb b/lib/extensions/ar_order.rb index 6a6e7701eac..8b330682795 100644 --- a/lib/extensions/ar_order.rb +++ b/lib/extensions/ar_order.rb @@ -8,6 +8,10 @@ def columns_for_distinct(columns, orders) #:nodoc: order_columns = orders.reject(&:blank?).map { |s| # Convert Arel node to string unless s.is_a?(String) + if s.kind_of?(Arel::Nodes::Ordering) + s = s.expr + keep_order = true + end if s.respond_to?(:to_sql) s = s.to_sql else # for Arel::Nodes::Attribute @@ -17,9 +21,14 @@ def columns_for_distinct(columns, orders) #:nodoc: s = collector.value end end + # If we haven't already removed the order clause, # Remove any ASC/DESC modifiers - s.gsub(/\s+(?:ASC|DESC)\b/i, "") - .gsub(/\s+NULLS\s+(?:FIRST|LAST)\b/i, "") + if keep_order + s + else + s.gsub(/\s+(?:ASC|DESC)\b/i, "") + .gsub(/\s+NULLS\s+(?:FIRST|LAST)\b/i, "") + end }.reject(&:blank?).map.with_index { |column, i| "#{column} AS alias_#{i}" } (order_columns << super).join(", ") diff --git a/spec/lib/extensions/ar_order_spec.rb b/spec/lib/extensions/ar_order_spec.rb index a0941feec71..38aafe713a6 100644 --- a/spec/lib/extensions/ar_order_spec.rb +++ b/spec/lib/extensions/ar_order_spec.rb @@ -1,4 +1,16 @@ describe "ar_order extension" do + # includes a has_many AND references the has many + # this introduces a DISTINCT to the query. + # rails uses limited_ids_for (which calls columns_for_distinct) to run a quick query + # + # when we use a virtual attribute in the sort (and the attribute has a descending order) + # the string munging gives us issues. + it "supports order when distinct is present for has_many virtual column" do + expect do + VmOrTemplate.includes(:disks).references(:disks).order(:last_compliance_status).first + end.not_to raise_error + end + it "supports order when distinct is present for basic column" do expect do VmOrTemplate.includes(:disks).references(:disks).order(:id).first