Merge branch 'master' into Khronos/master

* master:
  visitors can define their own cache strategy for dispatch. fixes #57
  Break Ordering into Ascending/Descending nodes, allow reversal
  remove unnecessary guarding agains literal
  LIMIT and OFFSET support for MS SQL
  Include Arel::Predicates to Arel::Nodes::Function so you can do table[:id].count.eq(2)
  updating spec
  zomg prep release
  make sure thread runs
  do not cache sql literal values
  no longer use this instance variable

10 files changed, 151 insertions, 18 deletions

diff --git a/test/attributes/test_attribute.rb b/test/attributes/test_attribute.rb
index 352774071a..901850ff4b 100644
--- a/test/attributes/test_attribute.rb
+++ b/test/attributes/test_attribute.rb
@@ -619,9 +619,9 @@ module Arel
       end
 
       describe '#asc' do
-        it 'should create an Ordering node' do
+        it 'should create an Ascending node' do
           relation = Table.new(:users)
-          relation[:id].asc.must_be_kind_of Nodes::Ordering
+          relation[:id].asc.must_be_kind_of Nodes::Ascending
         end
 
         it 'should generate ASC in sql' do
@@ -635,9 +635,9 @@ module Arel
       end
 
       describe '#desc' do
-        it 'should create an Ordering node' do
+        it 'should create a Descending node' do
           relation = Table.new(:users)
-          relation[:id].desc.must_be_kind_of Nodes::Ordering
+          relation[:id].desc.must_be_kind_of Nodes::Descending
         end
 
         it 'should generate DESC in sql' do
diff --git a/test/nodes/test_ascending.rb b/test/nodes/test_ascending.rb
new file mode 100644
index 0000000000..0e2c4810c6
--- /dev/null
+++ b/test/nodes/test_ascending.rb
@@ -0,0 +1,34 @@
+require 'helper'
+
+module Arel
+  module Nodes
+    class TestAscending < MiniTest::Unit::TestCase
+      def test_construct
+        ascending = Ascending.new 'zomg'
+        assert_equal 'zomg', ascending.expr
+      end
+
+      def test_reverse
+        ascending = Ascending.new 'zomg'
+        descending = ascending.reverse
+        assert_kind_of Descending, descending
+        assert_equal ascending.expr, descending.expr
+      end
+
+      def test_direction
+        ascending = Ascending.new 'zomg'
+        assert_equal :asc, ascending.direction
+      end
+
+      def test_ascending?
+        ascending = Ascending.new 'zomg'
+        assert ascending.ascending?
+      end
+
+      def test_descending?
+        ascending = Ascending.new 'zomg'
+        assert !ascending.descending?
+      end
+    end
+  end
+end
diff --git a/test/nodes/test_count.rb b/test/nodes/test_count.rb
index afa423e8f5..be53b86855 100644
--- a/test/nodes/test_count.rb
+++ b/test/nodes/test_count.rb
@@ -15,4 +15,13 @@ describe Arel::Nodes::Count do
       }
     end
   end
+
+  describe "eq" do
+    it "should compare the count" do
+      table = Arel::Table.new :users
+      table[:id].count.eq(2).to_sql.must_be_like %{
+        COUNT("users"."id") = 2
+      }
+    end
+  end
 end
diff --git a/test/nodes/test_descending.rb b/test/nodes/test_descending.rb
new file mode 100644
index 0000000000..424f8298cd
--- /dev/null
+++ b/test/nodes/test_descending.rb
@@ -0,0 +1,34 @@
+require 'helper'
+
+module Arel
+  module Nodes
+    class TestDescending < MiniTest::Unit::TestCase
+      def test_construct
+        descending = Descending.new 'zomg'
+        assert_equal 'zomg', descending.expr
+      end
+
+      def test_reverse
+        descending = Descending.new 'zomg'
+        ascending = descending.reverse
+        assert_kind_of Ascending, ascending
+        assert_equal descending.expr, ascending.expr
+      end
+
+      def test_direction
+        descending = Descending.new 'zomg'
+        assert_equal :desc, descending.direction
+      end
+
+      def test_ascending?
+        descending = Descending.new 'zomg'
+        assert !descending.ascending?
+      end
+
+      def test_descending?
+        descending = Descending.new 'zomg'
+        assert descending.descending?
+      end
+    end
+  end
+end
diff --git a/test/nodes/test_infix_operation.rb b/test/nodes/test_infix_operation.rb
index db3216eeee..3d2eb0d9c6 100644
--- a/test/nodes/test_infix_operation.rb
+++ b/test/nodes/test_infix_operation.rb
@@ -21,9 +21,9 @@ module Arel
       def test_opertaion_ordering
         operation = InfixOperation.new :+, 1, 2
         ordering = operation.desc
-        assert_kind_of Ordering, ordering
+        assert_kind_of Descending, ordering
         assert_equal operation, ordering.expr
-        assert_equal :desc, ordering.direction
+        assert ordering.descending?
       end
     end
   end
diff --git a/test/test_select_manager.rb b/test/test_select_manager.rb
index 29d317e16a..e948aec131 100644
--- a/test/test_select_manager.rb
+++ b/test/test_select_manager.rb
@@ -216,7 +216,7 @@ module Arel
         table   = Table.new :users, :engine => Table.engine, :as => 'foo'
         mgr = table.from table
         mgr.skip 10
-        mgr.to_sql.must_be_like %{ SELECT FROM "users" foo OFFSET 10 }
+        mgr.to_sql.must_be_like %{ SELECT FROM "users" "foo" OFFSET 10 }
       end
     end
 
diff --git a/test/visitors/test_depth_first.rb b/test/visitors/test_depth_first.rb
index 5bbdf57697..e62ce5266f 100644
--- a/test/visitors/test_depth_first.rb
+++ b/test/visitors/test_depth_first.rb
@@ -28,6 +28,7 @@ module Arel
         Arel::Nodes::On,
         Arel::Nodes::Grouping,
         Arel::Nodes::Offset,
+        Arel::Nodes::Ordering,
         Arel::Nodes::Having,
         Arel::Nodes::StringJoin,
         Arel::Nodes::UnqualifiedColumn,
@@ -104,7 +105,6 @@ module Arel
         Arel::Nodes::Values,
         Arel::Nodes::As,
         Arel::Nodes::DeleteStatement,
-        Arel::Nodes::Ordering,
         Arel::Nodes::JoinSource,
       ].each do |klass|
         define_method("test_#{klass.name.gsub('::', '_')}") do
diff --git a/test/visitors/test_dot.rb b/test/visitors/test_dot.rb
index b311246436..362e39339c 100644
--- a/test/visitors/test_dot.rb
+++ b/test/visitors/test_dot.rb
@@ -33,6 +33,7 @@ module Arel
         Arel::Nodes::On,
         Arel::Nodes::Grouping,
         Arel::Nodes::Offset,
+        Arel::Nodes::Ordering,
         Arel::Nodes::Having,
         Arel::Nodes::UnqualifiedColumn,
         Arel::Nodes::Top,
@@ -63,7 +64,6 @@ module Arel
         Arel::Nodes::Values,
         Arel::Nodes::As,
         Arel::Nodes::DeleteStatement,
-        Arel::Nodes::Ordering,
         Arel::Nodes::JoinSource,
       ].each do |klass|
         define_method("test_#{klass.name.gsub('::', '_')}") do
diff --git a/test/visitors/test_mssql.rb b/test/visitors/test_mssql.rb
index ccaea395fe..8b2b756900 100644
--- a/test/visitors/test_mssql.rb
+++ b/test/visitors/test_mssql.rb
@@ -5,21 +5,60 @@ module Arel
     describe 'the mssql visitor' do
       before do
         @visitor = MSSQL.new Table.engine
+        @table = Arel::Table.new "users"
       end
 
-      it 'uses TOP to limit results' do
+      it 'should not modify query if no offset or limit' do
         stmt = Nodes::SelectStatement.new
-        stmt.cores.last.top = Nodes::Top.new(1)
         sql = @visitor.accept(stmt)
-        sql.must_be_like "SELECT TOP 1"
+        sql.must_be_like "SELECT"
       end
 
-      it 'uses TOP in updates with a limit' do
-        stmt = Nodes::UpdateStatement.new
-        stmt.limit = Nodes::Limit.new(1)
-        stmt.key = 'id'
+      it 'should go over table PK if no .order() or .group()' do
+        stmt = Nodes::SelectStatement.new
+        stmt.cores.first.from = @table
+        stmt.limit = Nodes::Limit.new(10)
+        sql = @visitor.accept(stmt)
+        sql.must_be_like "SELECT _t.* FROM (SELECT ROW_NUMBER() OVER (ORDER BY \"users\".\"id\") as _row_num FROM \"users\" ) as _t WHERE _row_num BETWEEN 1 AND 10"
+      end
+
+      it 'should go over query ORDER BY if .order()' do
+        stmt = Nodes::SelectStatement.new
+        stmt.limit = Nodes::Limit.new(10)
+        stmt.orders << Nodes::SqlLiteral.new('order_by')
+        sql = @visitor.accept(stmt)
+        sql.must_be_like "SELECT _t.* FROM (SELECT ROW_NUMBER() OVER (ORDER BY order_by) as _row_num) as _t WHERE _row_num BETWEEN 1 AND 10"
+      end
+
+      it 'should go over query GROUP BY if no .order() and there is .group()' do
+        stmt = Nodes::SelectStatement.new
+        stmt.cores.first.groups << Nodes::SqlLiteral.new('group_by')
+        stmt.limit = Nodes::Limit.new(10)
+        sql = @visitor.accept(stmt)
+        sql.must_be_like "SELECT _t.* FROM (SELECT ROW_NUMBER() OVER (ORDER BY group_by) as _row_num GROUP BY group_by) as _t WHERE _row_num BETWEEN 1 AND 10"
+      end
+
+      it 'should use BETWEEN if both .limit() and .offset' do
+        stmt = Nodes::SelectStatement.new
+        stmt.limit = Nodes::Limit.new(10)
+        stmt.offset = Nodes::Offset.new(20)
+        sql = @visitor.accept(stmt)
+        sql.must_be_like "SELECT _t.* FROM (SELECT ROW_NUMBER() OVER (ORDER BY ) as _row_num) as _t WHERE _row_num BETWEEN 21 AND 30"
+      end
+
+      it 'should use >= if only .offset' do
+        stmt = Nodes::SelectStatement.new
+        stmt.offset = Nodes::Offset.new(20)
+        sql = @visitor.accept(stmt)
+        sql.must_be_like "SELECT _t.* FROM (SELECT ROW_NUMBER() OVER (ORDER BY ) as _row_num) as _t WHERE _row_num >= 21"
+      end
+
+      it 'should generate subquery for .count' do
+        stmt = Nodes::SelectStatement.new
+        stmt.limit = Nodes::Limit.new(10)
+        stmt.cores.first.projections << Nodes::Count.new('*')
         sql = @visitor.accept(stmt)
-        sql.must_be_like "UPDATE NULL WHERE 'id' IN (SELECT TOP 1 'id' )"
+        sql.must_be_like "SELECT COUNT(1) as count_id FROM (SELECT _t.* FROM (SELECT ROW_NUMBER() OVER (ORDER BY ) as _row_num) as _t WHERE _row_num BETWEEN 1 AND 10) AS subquery"
       end
 
     end
diff --git a/test/visitors/test_to_sql.rb b/test/visitors/test_to_sql.rb
index 8084e6441c..b52722ddd6 100644
--- a/test/visitors/test_to_sql.rb
+++ b/test/visitors/test_to_sql.rb
@@ -1,7 +1,7 @@
 require 'helper'
 
 class Arel::Visitors::ToSql
-  def last_column; Thread.current[:arel_visitors_to_sql_last_column] || @last_column; end
+  def last_column; Thread.current[:arel_visitors_to_sql_last_column]; end
 end
 
 module Arel
@@ -13,12 +13,29 @@ module Arel
         @attr = @table[:id]
       end
 
+      it 'can define a dispatch method' do
+        visited = false
+        viz = Class.new(Arel::Visitors::Visitor) {
+          define_method(:hello) do |node|
+            visited = true
+          end
+
+          def dispatch
+            { Arel::Table => 'hello' }
+          end
+        }.new
+
+        viz.accept(@table)
+        assert visited, 'hello method was called'
+      end
+
       it "should be thread safe around usage of last_column" do
         visit_integer_column = Thread.new do
           Thread.stop
           @visitor.send(:visit_Arel_Attributes_Attribute, @attr)
         end
 
+        sleep 0.2
         @visitor.accept(@table[:name])
         assert_equal(:string, @visitor.last_column.type)
         visit_integer_column.run