Make all things passed to ARel order a real Ordering object so we can uniq them.

Author: metaskills

lib/arel/visitors/sqlserver.rb

@@ -17,12 +17,47 @@ def initialize expr
       end
     end
 
+    class Ordering < Arel::Nodes::Binary
+      def hash
+        expr.hash
+      end
+      def ==(other)
+        self.class == other.class && self.expr == other.expr
+      end
+      def eql?(other)
+        self == other
+      end
+    end  
+    
   end
 
   class SelectManager < Arel::TreeManager
 
     alias :lock_without_sqlserver :lock
 
+    def order(*exprs)
+      @ast.orders.concat(exprs.map{ |x|
+        case x
+        when Arel::Attributes::Attribute
+          c = engine.connection
+          tn = x.relation.table_alias || x.relation.name
+          expr = Nodes::SqlLiteral.new "#{c.quote_table_name(tn)}.#{c.quote_column_name(x.name)}"
+          Nodes::Ordering.new expr
+        when String
+          x.split(',').map do |s|
+            expr, direction = s.split
+            expr = Nodes::SqlLiteral.new(expr)
+            direction = direction =~ /desc/i ? :desc : :asc
+            Nodes::Ordering.new expr, direction
+          end
+        else
+          expr = Nodes::SqlLiteral.new x.to_s
+          Nodes::Ordering.new expr
+        end
+      }.flatten)
+      self
+    end
+    
     def lock(locking=true)
       if Arel::Visitors::SQLServer === @visitor
         @ast.lock = Nodes::LockWithSQLServer.new(locking)
@@ -82,7 +117,7 @@ def visit_Arel_Nodes_SelectStatementWithOutOffset(o, windowed=false)
         core = o.cores.first
         projections = core.projections
         groups = core.groups
-        orders = o.orders
+        orders = o.orders.reverse.uniq.reverse
         if windowed && !function_select_statement?(o)
           projections =  projections.map { |x| projection_without_expression(x) }
         elsif eager_limiting_select_statement?(o)
@@ -116,7 +151,7 @@ def visit_Arel_Nodes_SelectStatementWithOffset(o)
           (visit(o.limit) if o.limit && !single_distinct_select_statement?(o)),
           (rowtable_projections(o).map{ |x| visit(x) }.join(', ')),
           "FROM (",
-            "SELECT ROW_NUMBER() OVER (ORDER BY #{orders.map{ |x| visit(x) }.uniq.join(', ')}) AS [__rn],",
+            "SELECT ROW_NUMBER() OVER (ORDER BY #{orders.map{ |x| visit(x) }.join(', ')}) AS [__rn],",
             visit_Arel_Nodes_SelectStatementWithOutOffset(o,true),
           ") AS [__rnt]",
           (visit(o.offset) if o.offset),
@@ -132,14 +167,14 @@ def visit_Arel_Nodes_SelectStatementForComplexCount(o)
           "FROM (",
             "SELECT",
             (visit(o.limit) if o.limit),
-            "ROW_NUMBER() OVER (ORDER BY #{orders.map{ |x| visit(x) }.uniq.join(', ')}) AS [__rn],",
+            "ROW_NUMBER() OVER (ORDER BY #{orders.map{ |x| visit(x) }.join(', ')}) AS [__rn],",
             "1 AS [count]",
             visit(core.source),
             (visit(o.lock) if o.lock),
             ("WHERE #{core.wheres.map{ |x| visit(x) }.join ' AND ' }" unless core.wheres.empty?),
             ("GROUP BY #{core.groups.map { |x| visit x }.join ', ' }" unless core.groups.empty?),
             (visit(core.having) if core.having),
-            ("ORDER BY #{o.orders.map{ |x| visit(x) }.uniq.join(', ')}" if !o.orders.empty?),
+            ("ORDER BY #{o.orders.map{ |x| visit(x) }.join(', ')}" if !o.orders.empty?),
           ") AS [__rnt]",
           (visit(o.offset) if o.offset)
         ].compact.join ' '
@@ -226,7 +261,7 @@ def rowtable_orders(o)
         else
           tn = table_name_from_select_statement(o)
           [Arel::Table.new(tn, @engine).primary_key.asc]
-        end
+        end.reverse.uniq.reverse
       end
 
       def projection_without_expression(projection)

test/cases/method_scoping_test_sqlserver.rb

@@ -17,7 +17,7 @@ def test_coerced_test_merged_scoped_find
     poor_jamis = developers(:poor_jamis)
     Developer.send(:with_scope, :find => { :conditions => "salary < 100000" }) do
       Developer.send(:with_scope, :find => { :offset => 1, :order => 'id asc' }) do
-        assert_sql /ORDER BY id asc/ do
+        assert_sql /ORDER BY id asc/i do
           assert_equal(poor_jamis, Developer.find(:first, :order => 'id asc'))
         end
       end

test/cases/scratch_test_sqlserver.rb

@@ -1,26 +1,27 @@
 require 'cases/sqlserver_helper'
-require 'models/tag'
 require 'models/tagging'
 require 'models/post'
-require 'models/item'
+require 'models/topic'
 require 'models/comment'
+require 'models/reply'
 require 'models/author'
-require 'models/category'
-require 'models/categorization'
-require 'models/vertex'
-require 'models/edge'
-require 'models/book'
-require 'models/citation'
+require 'models/comment'
+require 'models/entrant'
+require 'models/developer'
+require 'models/company'
+require 'models/bird'
+require 'models/car'
+require 'models/engine'
+require 'models/tyre'
 
 class ScratchTestSqlserver < ActiveRecord::TestCase
-  
-  self.use_transactional_fixtures = false
-  
-  fixtures :posts, :authors, :categories, :categorizations, :comments, :tags, :taggings, 
-           :author_favorites, :vertices, :items, :books, :edges
+
+  fixtures :topics, :companies, :developers, :projects, :computers, :accounts, :minimalistics, 'warehouse-things', 
+           :authors, :categorizations, :categories, :posts
 
   should 'pass' do
-    assert_equal 1, posts(:welcome).tags.count
+    combined = Developer.find(:all, :order => 'developers.name, developers.salary')
+    assert_equal combined, Developer.find(:all, :order => ['developers.name', 'developers.salary'])
   end