From 03e2006ad97cabb689e2a84bdbf9300baaf518b5 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 8 Apr 2014 20:30:40 -0700 Subject: refactor mssql nodes to move away from string interpolation --- lib/arel/visitors/mssql.rb | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/arel/visitors/mssql.rb b/lib/arel/visitors/mssql.rb index ef0b78058e..cc077d863b 100644 --- a/lib/arel/visitors/mssql.rb +++ b/lib/arel/visitors/mssql.rb @@ -1,6 +1,14 @@ module Arel module Visitors class MSSQL < Arel::Visitors::ToSql + class RowNumber + attr_reader :expr + + def initialize node + @expr = node + end + end + private # `top` wouldn't really work here. I.e. User.select("distinct first_name").limit(10) would generate @@ -10,6 +18,10 @@ module Arel "" end + def visit_Arel_Visitors_MSSQL_RowNumber o + "ROW_NUMBER() OVER (#{o.expr}) as _row_num" + end + def visit_Arel_Nodes_SelectStatement o if !o.limit && !o.offset return super o @@ -55,7 +67,7 @@ module Arel end def row_num_literal order_by - Nodes::SqlLiteral.new("ROW_NUMBER() OVER (#{order_by}) as _row_num") + RowNumber.new order_by end def select_count? x -- cgit v1.2.3 From 9443c01b80f0942025634c9bd22cb741c891090c Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 8 Apr 2014 20:31:47 -0700 Subject: use if / else so my brain stops hurting --- lib/arel/visitors/mssql.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/arel/visitors/mssql.rb b/lib/arel/visitors/mssql.rb index cc077d863b..49f79c527d 100644 --- a/lib/arel/visitors/mssql.rb +++ b/lib/arel/visitors/mssql.rb @@ -59,7 +59,7 @@ module Arel end def determine_order_by x - unless x.groups.empty? + if x.groups.any? "ORDER BY #{x.groups.map { |g| visit g }.join ', ' }" else "ORDER BY #{find_left_table_pk(x.froms)}" -- cgit v1.2.3 From 3cbafe833b5c388403cd78c3bcb278850733d032 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 8 Apr 2014 20:35:28 -0700 Subject: move all the "ORDER BY" together --- lib/arel/visitors/mssql.rb | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) (limited to 'lib') diff --git a/lib/arel/visitors/mssql.rb b/lib/arel/visitors/mssql.rb index 49f79c527d..479a867e37 100644 --- a/lib/arel/visitors/mssql.rb +++ b/lib/arel/visitors/mssql.rb @@ -27,11 +27,9 @@ module Arel 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) + core_order_by = determine_order_by(o.orders, x) if select_count? x x.projections = [row_num_literal(core_order_by)] is_select_count = true @@ -58,11 +56,15 @@ module Arel end end - def determine_order_by x - if x.groups.any? - "ORDER BY #{x.groups.map { |g| visit g }.join ', ' }" + def determine_order_by orders, x + if orders.any? + "ORDER BY #{orders.map { |x| visit x }.join(', ')}" else - "ORDER BY #{find_left_table_pk(x.froms)}" + if x.groups.any? + "ORDER BY #{x.groups.map { |g| visit g }.join ', ' }" + else + "ORDER BY #{find_left_table_pk(x.froms)}" + end end end -- cgit v1.2.3 From 680bac202da2e9c717d4e47c9402f291a8971fad Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 8 Apr 2014 20:38:00 -0700 Subject: move the ORDER BY to the RowNumber method --- lib/arel/visitors/mssql.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'lib') diff --git a/lib/arel/visitors/mssql.rb b/lib/arel/visitors/mssql.rb index 479a867e37..c7ac56b43e 100644 --- a/lib/arel/visitors/mssql.rb +++ b/lib/arel/visitors/mssql.rb @@ -19,7 +19,7 @@ module Arel end def visit_Arel_Visitors_MSSQL_RowNumber o - "ROW_NUMBER() OVER (#{o.expr}) as _row_num" + "ROW_NUMBER() OVER (ORDER BY #{o.expr}) as _row_num" end def visit_Arel_Nodes_SelectStatement o @@ -58,12 +58,12 @@ module Arel def determine_order_by orders, x if orders.any? - "ORDER BY #{orders.map { |x| visit x }.join(', ')}" + "#{orders.map { |x| visit x }.join(', ')}" else if x.groups.any? - "ORDER BY #{x.groups.map { |g| visit g }.join ', ' }" + "#{x.groups.map { |g| visit g }.join ', ' }" else - "ORDER BY #{find_left_table_pk(x.froms)}" + "#{find_left_table_pk(x.froms)}" end end end -- cgit v1.2.3 From 0d3e9161794e95995ee1f2dcfc063782758e64c0 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 8 Apr 2014 20:44:42 -0700 Subject: build the ast rather than passing around strings --- lib/arel/visitors/mssql.rb | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) (limited to 'lib') diff --git a/lib/arel/visitors/mssql.rb b/lib/arel/visitors/mssql.rb index c7ac56b43e..cc5e74af1f 100644 --- a/lib/arel/visitors/mssql.rb +++ b/lib/arel/visitors/mssql.rb @@ -2,10 +2,10 @@ module Arel module Visitors class MSSQL < Arel::Visitors::ToSql class RowNumber - attr_reader :expr + attr_reader :children def initialize node - @expr = node + @children = node end end @@ -19,7 +19,7 @@ module Arel end def visit_Arel_Visitors_MSSQL_RowNumber o - "ROW_NUMBER() OVER (ORDER BY #{o.expr}) as _row_num" + "ROW_NUMBER() OVER (ORDER BY #{o.children.map { |x| visit x }.join ', '}) as _row_num" end def visit_Arel_Nodes_SelectStatement o @@ -29,12 +29,12 @@ module Arel is_select_count = false sql = o.cores.map { |x| - core_order_by = determine_order_by(o.orders, x) + core_order_by = row_num_literal determine_order_by(o.orders, x) if select_count? x - x.projections = [row_num_literal(core_order_by)] + x.projections = [core_order_by] is_select_count = true else - x.projections << row_num_literal(core_order_by) + x.projections << core_order_by end visit_Arel_Nodes_SelectCore x @@ -58,13 +58,11 @@ module Arel def determine_order_by orders, x if orders.any? - "#{orders.map { |x| visit x }.join(', ')}" + orders + elsif x.groups.any? + x.groups else - if x.groups.any? - "#{x.groups.map { |g| visit g }.join ', ' }" - else - "#{find_left_table_pk(x.froms)}" - end + [Arel.sql(find_left_table_pk(x.froms).to_s)] end end -- cgit v1.2.3