From 030a7794401615c292d02625cca783227f7dc060 Mon Sep 17 00:00:00 2001 From: Joe Rafaniello Date: Wed, 7 Dec 2011 12:00:22 -0500 Subject: [PATCH 1/2] Fix custom order method which created invalid syntax for order by clauses containing case expressions, inline functions or other non-column expressions. Moved ordering tests into a new test file. --- lib/arel/visitors/sqlserver.rb | 6 +- test/cases/adapter_test_sqlserver.rb | 6 -- test/cases/order_test_sqlserver.rb | 136 +++++++++++++++++++++++++++ 3 files changed, 140 insertions(+), 8 deletions(-) create mode 100644 test/cases/order_test_sqlserver.rb diff --git a/lib/arel/visitors/sqlserver.rb b/lib/arel/visitors/sqlserver.rb index 187cb2b66..ece8a8ac5 100644 --- a/lib/arel/visitors/sqlserver.rb +++ b/lib/arel/visitors/sqlserver.rb @@ -41,9 +41,11 @@ def order(*expr) x when String x.split(',').map do |s| - e, d = s.split + s.strip! + d = s =~ /(asc|desc)$/i ? $1.upcase : nil + e = d.nil? ? s : s[0...-d.length].strip e = Arel.sql(e) - d =~ /desc/i ? Arel::Nodes::Descending.new(e) : Arel::Nodes::Ascending.new(e) + d && d == "DESC" ? Arel::Nodes::Descending.new(e) : Arel::Nodes::Ascending.new(e) end else e = Arel.sql(x.to_s) diff --git a/test/cases/adapter_test_sqlserver.rb b/test/cases/adapter_test_sqlserver.rb index 78ad54351..9831414e8 100644 --- a/test/cases/adapter_test_sqlserver.rb +++ b/test/cases/adapter_test_sqlserver.rb @@ -19,12 +19,6 @@ def setup context 'For abstract behavior' do - should 'not mangel complex order clauses' do - xyz_order = "CASE WHEN [title] LIKE N'XYZ%' THEN 0 ELSE 1 END" - xyz_post = Post.create :title => 'XYZ Post', :body => 'Test cased orders.' - assert_equal xyz_post, Post.order(Arel::Nodes::Ordering.new(Arel.sql(xyz_order))).first - end - should 'have a 128 max #table_alias_length' do assert @connection.table_alias_length <= 128 end diff --git a/test/cases/order_test_sqlserver.rb b/test/cases/order_test_sqlserver.rb new file mode 100644 index 000000000..53ef39e9d --- /dev/null +++ b/test/cases/order_test_sqlserver.rb @@ -0,0 +1,136 @@ +require 'cases/sqlserver_helper' +require 'models/post' + +class OrderTestSqlserver < ActiveRecord::TestCase + + fixtures :posts + + context 'Order by' do + + should 'not mangel complex order clauses' do + xyz_order = "CASE WHEN [title] LIKE N'XYZ%' THEN 0 ELSE 1 END" + xyz_post = Post.create :title => 'XYZ Post', :body => 'Test cased orders.' + assert_equal xyz_post, Post.order(Arel::Nodes::Ordering.new(Arel.sql(xyz_order))).first + end + + should 'support column' do + order = "title" + post1 = Post.create :title => 'AAA Post', :body => 'Test cased orders.' + assert_equal post1, Post.order(order).first + end + + should 'support column ASC' do + order = "title ASC" + post1 = Post.create :title => 'AAA Post', :body => 'Test cased orders.' + assert_equal post1, Post.order(order).first + end + + should 'support column DESC' do + order = "title DESC" + post1 = Post.create :title => 'ZZZ Post', :body => 'Test cased orders.' + assert_equal post1, Post.order(order).first + end + + should 'support column as symbol' do + order = :title + post1 = Post.create :title => 'AAA Post', :body => 'Test cased orders.' + assert_equal post1, Post.order(order).first + end + + should 'support table and column' do + order = "posts.title" + post1 = Post.create :title => 'AAA Post', :body => 'Test cased orders.' + assert_equal post1, Post.order(order).first + end + + should 'support quoted column' do + order = "[title]" + post1 = Post.create :title => 'AAA Post', :body => 'Test cased orders.' + assert_equal post1, Post.order(order).first + end + + should 'support quoted table and column' do + order = "[posts].[title]" + post1 = Post.create :title => 'AAA Post', :body => 'Test cased orders.' + assert_equal post1, Post.order(order).first + end + + should 'support primary: column, secondary: column' do + order = "title DESC, body" + post1 = Post.create :title => 'ZZZ Post', :body => 'Test cased orders.' + post2 = Post.create :title => 'ZZZ Post', :body => 'ZZZ Test cased orders.' + assert_equal post1, Post.order(order).first + assert_equal post2, Post.order(order).second + end + + should 'support primary: table and column, secondary: column' do + order = "posts.title DESC, body" + post1 = Post.create :title => 'ZZZ Post', :body => 'Test cased orders.' + post2 = Post.create :title => 'ZZZ Post', :body => 'ZZZ Test cased orders.' + assert_equal post1, Post.order(order).first + assert_equal post2, Post.order(order).second + end + + should 'support primary: case expression, secondary: column' do + order = "(CASE WHEN [title] LIKE N'ZZZ%' THEN title ELSE '' END) DESC, body" + post1 = Post.create :title => 'ZZZ Post', :body => 'Test cased orders.' + post2 = Post.create :title => 'ZZZ Post', :body => 'ZZZ Test cased orders.' + assert_equal post1, Post.order(order).first + assert_equal post2, Post.order(order).second + end + + should 'support primary: quoted table and column, secondary: case expresion' do + order = "[posts].[body] DESC, (CASE WHEN [title] LIKE N'ZZZ%' THEN title ELSE '' END) DESC" + post1 = Post.create :title => 'ZZZ Post', :body => 'ZZZ Test cased orders.' + post2 = Post.create :title => 'ZZY Post', :body => 'ZZZ Test cased orders.' + assert_equal post1, Post.order(order).first + assert_equal post2, Post.order(order).second + end + + should 'support inline function' do + order = "LEN(title)" + post1 = Post.create :title => 'A', :body => 'AAA Test cased orders.' + assert_equal post1, Post.order(order).first + end + + should 'support primary: inline function, secondary: column' do + order = "LEN(title), body" + post1 = Post.create :title => 'A', :body => 'AAA Test cased orders.' + post2 = Post.create :title => 'A', :body => 'Test cased orders.' + assert_equal post1, Post.order(order).first + assert_equal post2, Post.order(order).second + end + + should 'support primary: inline function, secondary: column with direction' do + order = "LEN(title) ASC, body DESC" + post1 = Post.create :title => 'A', :body => 'ZZZ Test cased orders.' + post2 = Post.create :title => 'A', :body => 'Test cased orders.' + assert_equal post1, Post.order(order).first + assert_equal post2, Post.order(order).second + end + + should 'support primary: column, secondary: inline function' do + order = "body DESC, LEN(title)" + post1 = Post.create :title => 'Post', :body => 'ZZZ Test cased orders.' + post2 = Post.create :title => 'Longer Post', :body => 'ZZZ Test cased orders.' + assert_equal post1, Post.order(order).first + assert_equal post2, Post.order(order).second + end + + should 'support primary: case expression, secondary: inline function' do + order = "CASE WHEN [title] LIKE N'ZZZ%' THEN title ELSE '' END DESC, LEN(body) ASC" + post1 = Post.create :title => 'ZZZ Post', :body => 'Z' + post2 = Post.create :title => 'ZZZ Post', :body => 'Test cased orders.' + assert_equal post1, Post.order(order).first + assert_equal post2, Post.order(order).second + end + + should 'support primary: inline function, secondary: case expression' do + order = "LEN(body), CASE WHEN [title] LIKE N'ZZZ%' THEN title ELSE '' END DESC" + post1 = Post.create :title => 'ZZZ Post', :body => 'Z' + post2 = Post.create :title => 'Post', :body => 'Z' + assert_equal post1, Post.order(order).first + assert_equal post2, Post.order(order).second + end + end +end From 1dfb7755ac9d7702ef2a34aa0c5d0fff9c14d38d Mon Sep 17 00:00:00 2001 From: Joe Rafaniello Date: Wed, 7 Dec 2011 12:01:15 -0500 Subject: [PATCH 2/2] Added failing test for inline functions with parameters. This fails because custom ordering code splits on comma. --- test/cases/order_test_sqlserver.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/cases/order_test_sqlserver.rb b/test/cases/order_test_sqlserver.rb index 53ef39e9d..c6ea2f83f 100644 --- a/test/cases/order_test_sqlserver.rb +++ b/test/cases/order_test_sqlserver.rb @@ -93,6 +93,12 @@ class OrderTestSqlserver < ActiveRecord::TestCase assert_equal post1, Post.order(order).first end + should 'support inline function with parameters' do + order = "SUBSTRING(title, 1, 3)" + post1 = Post.create :title => 'AAA Post', :body => 'Test cased orders.' + assert_equal post1, Post.order(order).first + end + should 'support primary: inline function, secondary: column' do order = "LEN(title), body" post1 = Post.create :title => 'A', :body => 'AAA Test cased orders.'