Limit and Offset can take SqlLiteral objects now. Always compose correct SQL. Use Arel.sql vs long winded constructor.

Author: metaskills

lib/arel/visitors/sqlserver.rb

@@ -34,19 +34,18 @@ def order(*exprs)
       @ast.orders.concat(exprs.map{ |x|
         case x
         when Arel::Attributes::Attribute
-          c = engine.connection
           table = Arel::Table.new(x.relation.table_alias || x.relation.name)
           expr = table[x.name]
           Arel::Nodes::Ordering.new expr
         when String
           x.split(',').map do |s|
             expr, direction = s.split
-            expr = Arel::Nodes::SqlLiteral.new(expr)
+            expr = Arel.sql(expr)
             direction = direction =~ /desc/i ? :desc : :asc
             Arel::Nodes::Ordering.new expr, direction
           end
         else
-          expr = Arel::Nodes::SqlLiteral.new x.to_s
+          expr = Arel.sql(x.to_s)
           Arel::Nodes::Ordering.new expr
         end
       }.flatten)
@@ -84,7 +83,7 @@ def visit_Arel_Nodes_SelectStatement(o)
       end
 
       def visit_Arel_Nodes_Offset(o)
-        "WHERE [__rnt].[__rn] > #{visit o.expr}"
+        "WHERE [__rnt].[__rn] > (#{visit o.expr})"
       end
 
       def visit_Arel_Nodes_Limit(o)
@@ -121,7 +120,7 @@ def visit_Arel_Nodes_SelectStatementWithOutOffset(o, windowed=false)
           groups = projections.map { |x| projection_without_expression(x) }
           projections = projections.map { |x| projection_without_expression(x) }
           orders = orders.map do |x|
-            expr = Arel::Nodes::SqlLiteral.new projection_without_expression(x.expr)
+            expr = Arel.sql projection_without_expression(x.expr)
             x.descending? ? Arel::Nodes::Max.new([expr]) : Arel::Nodes::Min.new([expr])
           end
         end
@@ -151,7 +150,7 @@ def visit_Arel_Nodes_SelectStatementWithOffset(o)
 
       def visit_Arel_Nodes_SelectStatementForComplexCount(o)
         core = o.cores.first
-        o.limit.expr = o.limit.expr.to_i + (o.offset ? o.offset.expr.to_i : 0) if o.limit
+        o.limit.expr = Arel.sql("#{o.limit.expr} + #{o.offset ? o.offset.expr : 0}") if o.limit
         orders = rowtable_orders(o)
         [ "SELECT COUNT([count]) AS [count_id]",
           "FROM (",
@@ -286,11 +285,11 @@ def rowtable_projections(o)
           end
         elsif join_in_select_statement?(o) && all_projections_aliased_in_select_statement?(o)
           core.projections.map do |x|
-            Arel::Nodes::SqlLiteral.new x.split(',').map{ |y| y.split(' AS ').last.strip }.join(', ')
+            Arel.sql x.split(',').map{ |y| y.split(' AS ').last.strip }.join(', ')
           end
         elsif function_select_statement?(o)
           # TODO: [ARel 2.2] Use Arel.star
-          [Arel::Nodes::SqlLiteral.new('*')]
+          [Arel.sql('*')]
         else
           tn = table_from_select_statement(o).name
           core.projections.map { |x| x.gsub /\[#{tn}\]\./, '[__rnt].' }
@@ -310,7 +309,7 @@ def rowtable_orders(o)
 
       # TODO: We use this for grouping too, maybe make Grouping objects vs SqlLiteral.
       def projection_without_expression(projection)
-        Arel::Nodes::SqlLiteral.new(projection.split(',').map do |x|
+        Arel.sql(projection.split(',').map do |x|
           x.strip!
           x.sub!(/^(COUNT|SUM|MAX|MIN|AVG)\s*(\((.*)\))?/,'\3')
           x.sub!(/^DISTINCT\s*/,'')

test/cases/offset_and_limit_test_sqlserver.rb

@@ -16,32 +16,40 @@ class OffsetAndLimitTestSqlserver < ActiveRecord::TestCase
       assert_sql(/SELECT TOP \(10\)/) { Book.limit(10).all }
     end
 
-    should 'only allow integers for limit' do
-      assert_sql(/SELECT TOP \(20\)/) { Book.limit('20-twenty').all }
+    should 'allow sql literal for limit' do
+      assert_sql(/SELECT TOP \(3-2\)/) { Book.limit(Arel.sql('3-2')).all }
+      assert_sql(/SELECT TOP \(SELECT 2 AS \[count\]\)/) do 
+        books = Book.all :limit => Arel.sql('SELECT 2 AS [count]')
+        assert_equal 2, books.size
+      end
     end
 
   end
 
   context 'When selecting with offset' do
 
     should 'have no limit (top) if only offset is passed' do
-      assert_sql(/SELECT \[__rnt\]\.\* FROM.*WHERE \[__rnt\]\.\[__rn\] > 1/) { Book.all(:offset=>1) }
+      assert_sql(/SELECT \[__rnt\]\.\* FROM.*WHERE \[__rnt\]\.\[__rn\] > \(1\)/) { Book.all(:offset=>1) }
     end
 
   end
 
   context 'When selecting with limit and offset' do
 
-    should 'only allow integers for offset' do
-      assert_sql(/WHERE \[__rnt\]\.\[__rn\] > 0/) { Book.limit(10).offset('five').all }
+    should 'allow sql literal for offset' do
+      assert_sql(/WHERE \[__rnt\]\.\[__rn\] > \(3-2\)/) { Book.limit(10).offset(Arel.sql('3-2')).all }
+      assert_sql(/WHERE \[__rnt\]\.\[__rn\] > \(SELECT 8 AS \[count\]\)/) do 
+        books = Book.all :limit => 3, :offset => Arel.sql('SELECT 8 AS [count]')
+        assert_equal 2, books.size, 'remember there are only 10 books and offset is 8'
+      end
     end
 
-    should 'convert strings which look like integers to integers' do
-      assert_sql(/WHERE \[__rnt\]\.\[__rn\] > 5/) { Book.limit(10).offset('5').all }
+    should 'not convert strings which look like integers to integers' do
+      assert_sql(/WHERE \[__rnt\]\.\[__rn\] > \(N'5'\)/) { Book.limit(10).offset('5').all }
     end
 
     should 'alter SQL to limit number of records returned offset by specified amount' do
-      sql = %|SELECT TOP (3) [__rnt].* FROM ( SELECT ROW_NUMBER() OVER (ORDER BY [books].[id] ASC) AS [__rn], [books].* FROM [books] ) AS [__rnt] WHERE [__rnt].[__rn] > 5|
+      sql = %|SELECT TOP (3) [__rnt].* FROM ( SELECT ROW_NUMBER() OVER (ORDER BY [books].[id] ASC) AS [__rn], [books].* FROM [books] ) AS [__rnt] WHERE [__rnt].[__rn] > (5)|
       assert_sql(sql) { Book.limit(3).offset(5).all }
     end