Skip to content

Commit 8e85d2f

Browse files
pschambacherkares
authored andcommitted
Fix #367 pagination not working with group on SQLSERVER 2000.
The pagination condition was added before the ORDER condition. If there is a GROUP BY clause, the pagination was in the GROUP BY instead of the WHERE clause. New approach is extraction each clause and build a new proper query.
1 parent ec20a9d commit 8e85d2f

File tree

3 files changed

+44
-9
lines changed

3 files changed

+44
-9
lines changed

lib/arjdbc/mssql/adapter.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -679,4 +679,4 @@ module ActiveRecord::ConnectionAdapters
679679
class MSSQLColumn < JdbcColumn
680680
include ArJdbc::MSSQL::Column
681681
end
682-
end
682+
end

lib/arjdbc/mssql/limit_helpers.rb

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -68,22 +68,49 @@ def replace_limit_offset!(sql, limit, offset, order)
6868
new_sql = "#{select} TOP 1 #{rest_of_query} #{new_order}"
6969
sql.replace(new_sql)
7070
else
71-
#UGLY
72-
#KLUDGY?
73-
#removing out stuff before the FROM...
74-
rest = rest_of_query[/FROM/i=~ rest_of_query.. -1]
71+
# We are in deep trouble here. SQL Server does not have any kind of OFFSET build in.
72+
# Only remaining solution is adding a where condition to be sure that the ID is not in SELECT TOP OFFSET FROM SAME_QUERY.
73+
# To do so we need to extract each part of the query to insert our additional condition in the right place.
74+
query_without_select = rest_of_query[/FROM/i=~ rest_of_query.. -1]
75+
additional_condition = "#{table_name}.#{primary_key} NOT IN (#{select} TOP #{offset} #{table_name}.#{primary_key} #{query_without_select} #{new_order})"
7576

76-
if (rest_of_query.match(/WHERE/).nil?)
77-
new_sql = "#{select} TOP #{limit} #{rest_of_query} WHERE #{table_name}.#{primary_key} NOT IN (#{select} TOP #{offset} #{table_name}.#{primary_key} #{rest} #{new_order}) #{order} "
77+
# Extract the different parts of the query
78+
having, group_by, where, from, selection = split_sql(rest_of_query, /having/i, /group by/i, /where/i, /from/i)
79+
80+
# Update the where part to add our additional condition
81+
if where.blank?
82+
where = "WHERE #{additional_condition}"
7883
else
79-
new_sql = "#{select} TOP #{limit} #{rest_of_query} AND #{table_name}.#{primary_key} NOT IN (#{select} TOP #{offset} #{table_name}.#{primary_key} #{rest} #{new_order}) #{order} "
84+
where = "#{where} AND #{additional_condition}"
8085
end
8186

82-
sql.replace(new_sql)
87+
# Replace the query to be our new customized query
88+
sql.replace("#{select} TOP #{limit} #{selection} #{from} #{where} #{group_by} #{having} #{new_order}")
8389
end
8490
end
8591
sql
8692
end
93+
94+
# Split the rest_of_query into chunks based on regexs (applied from end of string to the beginning)
95+
# The result is an array of regexs.size+1 elements (the last one being the remaining once everything was chopped away)
96+
def split_sql rest_of_query, *regexs
97+
results = Array.new
98+
99+
regexs.each do |regex|
100+
if position = (regex =~ rest_of_query)
101+
# Extract the matched string and chop the rest_of_query
102+
matched = rest_of_query[position..-1]
103+
rest_of_query = rest_of_query[0...position]
104+
else
105+
matched = nil
106+
end
107+
108+
results << matched
109+
end
110+
results << rest_of_query
111+
112+
results
113+
end
87114

88115
def get_primary_key(order, table_name) # table_name might be quoted
89116
if order =~ /(\w*id\w*)/i

test/db/mssql/limit_offset_test.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,4 +169,12 @@ def test_offset_without_limit
169169
assert_equal("You must specify :limit with :offset.", error.message)
170170
end
171171

172+
def test_limit_with_group_by
173+
%w(one two three four five six seven eight).each do |name|
174+
LongShip.create!(:name => name)
175+
end
176+
177+
ships = LongShip.group(:name).find(:all, :limit => 2)
178+
asset_equal(['one', 'two'], ships.map(&:name))
179+
end
172180
end

0 commit comments

Comments
 (0)