From be48ed3071fd6524d0145c4ad3faeb4aafe3eda3 Mon Sep 17 00:00:00 2001 From: arkadiy kraportov Date: Thu, 9 Jun 2011 18:24:54 +0900 Subject: [PATCH 1/2] LIMIT and OFFSET support for MS SQL --- lib/arel/visitors/mssql.rb | 78 ++++++++++++++++++++++++++++++++----- test/visitors/test_mssql.rb | 63 ++++++++++++++++++++++++++---- 2 files changed, 124 insertions(+), 17 deletions(-) diff --git a/lib/arel/visitors/mssql.rb b/lib/arel/visitors/mssql.rb index ea7ab639..713ad0f0 100644 --- a/lib/arel/visitors/mssql.rb +++ b/lib/arel/visitors/mssql.rb @@ -3,19 +3,79 @@ module Visitors class MSSQL < Arel::Visitors::ToSql private - def build_subselect key, o - stmt = super - core = stmt.cores.first - core.top = Nodes::Top.new(o.limit.expr) if o.limit - stmt + # `top` wouldn't really work here. I.e. User.select("distinct first_name").limit(10) would generate + # "select top 10 distinct first_name from users", which is invalid query! it should be + # "select distinct top 10 first_name from users" + def visit_Arel_Nodes_Top o + "" end - def visit_Arel_Nodes_Limit o - "" + def visit_Arel_Nodes_SelectStatement o + if !o.limit && !o.offset + return super o + end + + select_order_by = "ORDER BY #{o.orders.map { |x| visit x }.join(', ')}" unless o.orders.empty? + + is_select_count = false + sql = o.cores.map { |x| + core_order_by = select_order_by || determine_order_by(x) + if select_count? x + x.projections = [row_num_literal(core_order_by)] + is_select_count = true + else + guard_against_select_constant! x + x.projections << row_num_literal(core_order_by) + end + + visit_Arel_Nodes_SelectCore x + }.join + + sql = "SELECT _t.* FROM (#{sql}) as _t WHERE #{get_offset_limit_clause(o)}" + # fixme count distinct wouldn't work with limit or offset + sql = "SELECT COUNT(1) as count_id FROM (#{sql}) AS subquery" if is_select_count + sql end - def visit_Arel_Nodes_Top o - "TOP #{visit o.expr}" + def get_offset_limit_clause o + first_row = o.offset ? o.offset.expr.to_i + 1 : 1 + last_row = o.limit ? o.limit.expr.to_i - 1 + first_row : nil + if last_row + " _row_num BETWEEN #{first_row} AND #{last_row}" + else + " _row_num >= #{first_row}" + end + end + + def determine_order_by x + unless x.groups.empty? + "ORDER BY #{x.groups.map { |g| visit g }.join ', ' }" + else + "ORDER BY #{find_left_table_pk(x.froms)}" + end + end + + def row_num_literal order_by + Nodes::SqlLiteral.new("ROW_NUMBER() OVER (#{order_by}) as _row_num") + end + + def select_count? x + x.projections.length == 1 && Arel::Nodes::Count === x.projections.first + end + + def guard_against_select_constant! x + # guard against .select(1) (i.e. validate_uniqueness uses it to minimize qry result set) + # todo it won't work for .select('a'), which is probably ok. 'coz of workaround: .select("'a' as a") + x.projections.map! do |p| + p.kind_of?(Fixnum) ? Nodes::SqlLiteral.new("#{p} as _fld_#{p}") : p + end + end + + # fixme raise exception of there is no pk? + # fixme!! Table.primary_key will be depricated. What is the replacement?? + def find_left_table_pk o + return visit o.primary_key if o.instance_of? Arel::Table + find_left_table_pk o.left if o.kind_of? Arel::Nodes::Join end end diff --git a/test/visitors/test_mssql.rb b/test/visitors/test_mssql.rb index ccaea395..4e8d5c4b 100644 --- a/test/visitors/test_mssql.rb +++ b/test/visitors/test_mssql.rb @@ -5,21 +5,68 @@ module Visitors 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 guard FixNums' do + stmt = Nodes::SelectStatement.new + stmt.limit = Nodes::Limit.new(10) + stmt.cores.first.projections << 1 + sql = @visitor.accept(stmt) + sql.must_be_like "SELECT _t.* FROM (SELECT 1 as _fld_1, ROW_NUMBER() OVER (ORDER BY ) as _row_num) as _t WHERE _row_num BETWEEN 1 AND 10" + 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 From 3da816a77318d28617d0e12739d29d76e372a4f9 Mon Sep 17 00:00:00 2001 From: arkadiy kraportov Date: Fri, 10 Jun 2011 12:47:55 +0900 Subject: [PATCH 2/2] remove unnecessary guarding agains literal --- lib/arel/visitors/mssql.rb | 10 ---------- test/visitors/test_mssql.rb | 8 -------- 2 files changed, 18 deletions(-) diff --git a/lib/arel/visitors/mssql.rb b/lib/arel/visitors/mssql.rb index 713ad0f0..23dc06a9 100644 --- a/lib/arel/visitors/mssql.rb +++ b/lib/arel/visitors/mssql.rb @@ -24,7 +24,6 @@ def visit_Arel_Nodes_SelectStatement o x.projections = [row_num_literal(core_order_by)] is_select_count = true else - guard_against_select_constant! x x.projections << row_num_literal(core_order_by) end @@ -63,21 +62,12 @@ def select_count? x x.projections.length == 1 && Arel::Nodes::Count === x.projections.first end - def guard_against_select_constant! x - # guard against .select(1) (i.e. validate_uniqueness uses it to minimize qry result set) - # todo it won't work for .select('a'), which is probably ok. 'coz of workaround: .select("'a' as a") - x.projections.map! do |p| - p.kind_of?(Fixnum) ? Nodes::SqlLiteral.new("#{p} as _fld_#{p}") : p - end - end - # fixme raise exception of there is no pk? # fixme!! Table.primary_key will be depricated. What is the replacement?? def find_left_table_pk o return visit o.primary_key if o.instance_of? Arel::Table find_left_table_pk o.left if o.kind_of? Arel::Nodes::Join end - end end end diff --git a/test/visitors/test_mssql.rb b/test/visitors/test_mssql.rb index 4e8d5c4b..8b2b7569 100644 --- a/test/visitors/test_mssql.rb +++ b/test/visitors/test_mssql.rb @@ -53,14 +53,6 @@ module Visitors 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 guard FixNums' do - stmt = Nodes::SelectStatement.new - stmt.limit = Nodes::Limit.new(10) - stmt.cores.first.projections << 1 - sql = @visitor.accept(stmt) - sql.must_be_like "SELECT _t.* FROM (SELECT 1 as _fld_1, ROW_NUMBER() OVER (ORDER BY ) as _row_num) as _t WHERE _row_num BETWEEN 1 AND 10" - end - it 'should generate subquery for .count' do stmt = Nodes::SelectStatement.new stmt.limit = Nodes::Limit.new(10)