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

4 files changed, 67 insertions, 11 deletions

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