jruby/activerecord-jdbc-adapter — Issue #154: mssql adapter produces duplicate records for SELECT DISTINCT with :limit and INNER JOINs
Author: cmoran92
Created: 2012-01-17 04:25:22+00:00
State: open

This query produces duplicate records with the mssql adapter:

```ruby
@payments = Payment.paginate(:page => params[:page],
:select => 'DISTINCT payments.*',
:joins => [:apple_app_store_payment, :payment_status_logs, :customer],
:conditions => conditions,
:order => params[:order_by_fragment], :per_page => 100)
```
It produces this SQL query:

```sql
SELECT DISTINCT t. *
FROM (SELECT ROW_NUMBER() OVER (ORDER BY payments.created_at DESC) AS _row_num,
payments. *
FROM [payments]
INNER JOIN [apple_app_store_payments]ON apple_app_store_payments.payment_id = payments.id
INNER JOIN [payment_status_logs]ON payment_status_logs.payment_id = payments.id
INNER JOIN [customers]ON [customers].id = [payments].customer_id
WHERE (1 = 1)) AS t
WHERE t._row_num BETWEEN 1 AND 100
```
The problem with this query is that the DISTINCT clause has to consider the values for payments.* (which might contain duplicates) combined with _row_num, which makes all the rows unique. This problem appears only when one is using DISTINCT and one or more JOINs with the :limit option (which will_paginate appends in this case).

The cause of this problem seems to be in lib/arjdbc/mssql/limit_helpers.rb, in the code that implements the :limit option for the mssql adapter. I added a few lines that introduce an additional subquery where the DISTINCT clause is properly placed.

```ruby
def replace_limit_offset!(sql, limit, offset, order)
if limit
offset ||= 0
start_row = offset + 1
end_row = offset + limit.to_i
find_select = /\b(SELECT(?:\s+DISTINCT)?)\b(.*)/im
whole, select, rest_of_query = find_select.match(sql).to_a
puts "sql=#{sql}"
rest_of_query.strip!
if rest_of_query[0...1] == "1" && rest_of_query !~ /1 AS/i
rest_of_query[0] = "*"
end
if rest_of_query[0] == "*"
from_table = LimitHelpers.get_table_name(rest_of_query)
rest_of_query = from_table + '.' + rest_of_query
end
if select =~ /DISTINCT/
order.gsub!(/([a-z0-9_])+\./, 't.')
new_sql = "SELECT t.* FROM (SELECT ROW_NUMBER() OVER(#{order}) AS _row_num, t.* FROM (#{select} #{rest_of_query}"
new_sql << ") AS t) AS t WHERE t._row_num BETWEEN #{start_row.to_s} AND #{end_row.to_s}"
else
new_sql = "#{select} t.* FROM (SELECT ROW_NUMBER() OVER(#{order}) AS _row_num, #{rest_of_query}"
new_sql << ") AS t WHERE t._row_num BETWEEN #{start_row.to_s} AND #{end_row.to_s}"
end
sql.replace(new_sql)
end
sql
end
end
```

This is the diff:

```diff
diff --git a/lib/arjdbc/mssql/limit_helpers.rb b/lib/arjdbc/mssql/limit_helpers.rb
index 2529c67..424fd43 100644
--- a/lib/arjdbc/mssql/limit_helpers.rb
+++ b/lib/arjdbc/mssql/limit_helpers.rb
@@ -79,8 +79,15 @@ module ::ArJdbc
from_table = LimitHelpers.get_table_name(rest_of_query)
rest_of_query = from_table + '.' + rest_of_query
end
- new_sql = "#{select} t.* FROM (SELECT ROW_NUMBER() OVER(#{order}) AS _row_num, #{rest_of_query}"
- new_sql << ") AS t WHERE t._row_num BETWEEN #{start_row.to_s} AND #{end_row.to_s}"
+ # We might want to add the condition that rest_of_query has at least one JOIN outside of signle quotes
+ if select =~ /DISTINCT/
+ order.gsub!(/([a-z0-9_])+\./, 't.')
+ new_sql = "SELECT t.* FROM (SELECT ROW_NUMBER() OVER(#{order}) AS _row_num, t.* FROM (#{select} #{rest_of_query}"
+ new_sql << ") AS t) AS t WHERE t._row_num BETWEEN #{start_row.to_s} AND #{end_row.to_s}"
+ else
+ new_sql = "#{select} t.* FROM (SELECT ROW_NUMBER() OVER(#{order}) AS _row_num, #{rest_of_query}"
+ new_sql << ") AS t WHERE t._row_num BETWEEN #{start_row.to_s} AND #{end_row.to_s}"
+ end
sql.replace(new_sql)
end
sql
```