Skip to content

Commit 151bf03

Browse files
metaskillsmetaskills
committed
[Rails3] Working on limit offset tests.
1 parent 1cc2ed5 commit 151bf03

File tree

3 files changed

+70
-78
lines changed

3 files changed

+70
-78
lines changed

‎RAILS3_NOTES‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ on a local branch of our remote tracking branch.
3535
= SQL Server Todo
3636

3737
2370 tests, 8467 assertions, 31 failures, 34 errors
38+
2370 tests, 8446 assertions, 16 failures, 53 errors
3839

3940
* SQL Server Test Cases
4041
x eager_association_test_sqlserver.rb (2) Limit offset stuff.
@@ -67,11 +68,11 @@ on a local branch of our remote tracking branch.
6768

6869
= Arel Todo
6970

70-
338 examples, 2 failures, 3 pending
71+
338 examples, 0 failures, 3 pending
7172

7273
* unit/relations
73-
• skip_spec.rb
74-
• take_spec.rb
74+
• skip_spec.rb (try adding more, maybe with order)
75+
• take_spec.rb (try adding more, maybe with order)
7576

7677

7778
* Review/remove our long winded add_limit_offset!

‎lib/active_record/connection_adapters/sqlserver/database_statements.rb‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,10 @@ def add_limit_offset!(sql, options)
5454
limit_string = "TOP (#{limit}) " if limit
5555
select_string = select_clauses.join(', ').gsub(%r|\[#{table_name}\]\.|,'[_rnt].')
5656
order_string = order_clauses.present? ? order_clauses.join(', ') : [quote_table_name(table_name),quote_column_name(primary_key)].join('.')
57-
window_template = %q|
58-
SELECT #{limit_string}#{select_string} FROM (
59-
SELECT ROW_NUMBER() OVER (ORDER BY #{order_string}) AS [rn],
60-
#{sql}
61-
) AS [_rnt]
62-
WHERE [_rnt].[rn] > #{offset}|.squish!
57+
window_template =
58+
'SELECT #{limit_string}#{select_string} ' +
59+
'FROM (SELECT ROW_NUMBER() OVER (ORDER BY #{order_string}) AS [rn], #{sql.strip}) AS [_rnt] ' +
60+
'WHERE [_rnt].[rn] > #{offset}'
6361
# Assembly
6462
sql.sub! /SELECT/i, ''
6563
sql.replace instance_eval("%|#{window_template}|")

‎test/cases/offset_and_limit_test_sqlserver.rb‎

Lines changed: 62 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -9,33 +9,33 @@ def setup
99
@connection = ActiveRecord::Base.connection
1010
end
1111

12-
context 'When selecting with limit' do
13-
14-
setup do
15-
@select_sql = 'SELECT * FROM schema'
16-
end
17-
18-
should 'alter SQL to limit number of records returned' do
19-
options = { :limit => 10 }
20-
assert_equal('SELECT TOP 10 * FROM schema', @connection.add_limit_offset!(@select_sql, options))
21-
end
22-
23-
should 'only allow integers for limit' do
24-
options = { :limit => 'ten' }
25-
assert_raise(ArgumentError) {@connection.add_limit_offset!(@select_sql, options) }
26-
end
27-
28-
should 'convert strings which look like integers to integers' do
29-
options = { :limit => '42' }
30-
assert_nothing_raised(ArgumentError) {@connection.add_limit_offset!(@select_sql, options)}
31-
end
32-
33-
should 'not allow sql injection via limit' do
34-
options = { :limit => '1 * FROM schema; DELETE * FROM table; SELECT TOP 10 *'}
35-
assert_raise(ArgumentError) { @connection.add_limit_offset!(@select_sql, options) }
36-
end
37-
38-
end
12+
# context 'When selecting with limit' do
13+
#
14+
# setup do
15+
# @select_sql = 'SELECT * FROM schema'
16+
# end
17+
#
18+
# should 'alter SQL to limit number of records returned' do
19+
# options = { :limit => 10 }
20+
# assert_equal('SELECT TOP 10 * FROM schema', @connection.add_limit_offset!(@select_sql, options))
21+
# end
22+
#
23+
# should 'only allow integers for limit' do
24+
# options = { :limit => 'ten' }
25+
# assert_raise(ArgumentError) {@connection.add_limit_offset!(@select_sql, options) }
26+
# end
27+
#
28+
# should 'convert strings which look like integers to integers' do
29+
# options = { :limit => '42' }
30+
# assert_nothing_raised(ArgumentError) {@connection.add_limit_offset!(@select_sql, options)}
31+
# end
32+
#
33+
# should 'not allow sql injection via limit' do
34+
# options = { :limit => '1 * FROM schema; DELETE * FROM table; SELECT TOP 10 *'}
35+
# assert_raise(ArgumentError) { @connection.add_limit_offset!(@select_sql, options) }
36+
# end
37+
#
38+
# end
3939

4040
context 'When selecting with limit and offset' do
4141

@@ -49,57 +49,50 @@ def setup
4949
@books.each {|b| b.destroy}
5050
end
5151

52-
should 'have limit if offset is passed' do
53-
options = { :offset => 1 }
54-
assert_raise(ArgumentError) { @connection.add_limit_offset!(@select_sql, options) }
52+
should 'have no limit if only offset is passed' do
53+
assert_sql(/SELECT \[_rnt\]\.* FROM/) { Book.all(:offset=>1) }
54+
# assert_sql(/SELECT \[_rnt\]\.* FROM.*WHERE \[_rnt\]\.\[rn\] > 1/) { Book.all(:offset=>1) }
5555
end
56-
56+
5757
should 'only allow integers for offset' do
58-
options = { :limit => 10, :offset => 'five' }
59-
assert_raise(ArgumentError) { @connection.add_limit_offset!(@select_sql, options)}
58+
assert_sql(/WHERE \[_rnt\]\.\[rn\] > 0/) { Book.limit(10).offset('five').all }
6059
end
61-
60+
6261
should 'convert strings which look like integers to integers' do
63-
options = { :limit => 10, :offset => '5' }
64-
assert_nothing_raised(ArgumentError) {@connection.add_limit_offset!(@select_sql, options)}
62+
assert_sql(/WHERE \[_rnt\]\.\[rn\] > 5/) { Book.limit(10).offset('5').all }
6563
end
6664

6765
should 'alter SQL to limit number of records returned offset by specified amount' do
68-
options = { :limit => 3, :offset => 5 }
69-
expected_sql = "SELECT * FROM (SELECT TOP 3 * FROM (SELECT TOP 8 * FROM books) AS tmp1) AS tmp2"
70-
assert_equal(expected_sql, @connection.add_limit_offset!(@select_sql, options))
66+
sql = %|SELECT TOP (3) [_rnt].*
67+
FROM (SELECT ROW_NUMBER() OVER (ORDER BY [books].[id]) AS [rn], [books].* FROM [books]) AS [_rnt]
68+
WHERE [_rnt].[rn] > 5|.squish
69+
assert_sql(sql) { Book.limit(3).offset(5).all }
7170
end
7271

73-
should_eventually 'add locks to deepest sub select in limit offset sql that has a limited tally' do
74-
options = { :limit => 3, :offset => 5, :lock => 'WITH (NOLOCK)' }
75-
expected_sql = "SELECT * FROM (SELECT TOP 3 * FROM (SELECT TOP 8 * FROM books WITH (NOLOCK)) AS tmp1) AS tmp2"
76-
@connection.add_limit_offset! @select_sql, options
77-
assert_equal expected_sql, @connection.add_lock!(@select_sql,options)
78-
end
79-
80-
# Not really sure what an offset sql injection might look like
81-
should 'not allow sql injection via offset' do
82-
options = { :limit => 10, :offset => '1 * FROM schema; DELETE * FROM table; SELECT TOP 10 *'}
83-
assert_raise(ArgumentError) { @connection.add_limit_offset!(@select_sql, options) }
84-
end
85-
86-
should 'not create invalid SQL with subquery SELECTs with TOP' do
87-
options = { :limit => 5, :offset => 1 }
88-
expected_sql = "SELECT * FROM (SELECT TOP 5 * FROM (SELECT TOP 6 *, (SELECT TOP 1 id FROM books) AS book_id FROM books) AS tmp1) AS tmp2"
89-
assert_equal expected_sql, @connection.add_limit_offset!(@subquery_select_sql,options)
90-
end
91-
92-
should 'add lock hints to tally sql if :lock option is present' do
93-
assert_sql %r|SELECT TOP 1000000000 \* FROM \[people\] WITH \(NOLOCK\)| do
94-
Person.all :limit => 5, :offset => 1, :lock => 'WITH (NOLOCK)'
95-
end
96-
end
97-
98-
should 'not add lock hints to tally sql if there is no :lock option' do
99-
assert_sql %r|\(SELECT TOP 1000000000 \* FROM \[people\] \)| do
100-
Person.all :limit => 5, :offset => 1
101-
end
102-
end
72+
# should_eventually 'add locks to deepest sub select in limit offset sql that has a limited tally' do
73+
# options = { :limit => 3, :offset => 5, :lock => 'WITH (NOLOCK)' }
74+
# expected_sql = "SELECT * FROM (SELECT TOP 3 * FROM (SELECT TOP 8 * FROM books WITH (NOLOCK)) AS tmp1) AS tmp2"
75+
# @connection.add_limit_offset! @select_sql, options
76+
# assert_equal expected_sql, @connection.add_lock!(@select_sql,options)
77+
# end
78+
#
79+
# # Not really sure what an offset sql injection might look like
80+
# should 'not allow sql injection via offset' do
81+
# options = { :limit => 10, :offset => '1 * FROM schema; DELETE * FROM table; SELECT TOP 10 *'}
82+
# assert_raise(ArgumentError) { @connection.add_limit_offset!(@select_sql, options) }
83+
# end
84+
#
85+
# should 'not create invalid SQL with subquery SELECTs with TOP' do
86+
# options = { :limit => 5, :offset => 1 }
87+
# expected_sql = "SELECT * FROM (SELECT TOP 5 * FROM (SELECT TOP 6 *, (SELECT TOP 1 id FROM books) AS book_id FROM books) AS tmp1) AS tmp2"
88+
# assert_equal expected_sql, @connection.add_limit_offset!(@subquery_select_sql,options)
89+
# end
90+
#
91+
# should 'not add lock hints to tally sql if there is no :lock option' do
92+
# assert_sql %r|\(SELECT TOP 1000000000 \* FROM \[people\] \)| do
93+
# Person.all :limit => 5, :offset => 1
94+
# end
95+
# end
10396

10497
end
10598

0 commit comments

Comments
 (0)