Skip to content

Commit d64fcb4

Browse files
committed
Pass AR tests by moving DISTINCT to GROUP BY in windowed SQL.
* Tests now passing. test_count_eager_with_has_many_and_limit_and_high_offset test_eager_with_has_many_and_limit_and_high_offset * SQL like this: SELECT DISTINCT TOP (2) [__rnt].id FROM ( SELECT ROW_NUMBER() OVER (ORDER BY [posts].[id] ASC) AS [__rn], [posts].id FROM [posts] LEFT OUTER JOIN [authors] ON [authors].[id] = [posts].[author_id] LEFT OUTER JOIN [comments] ON [comments].[post_id] = [posts].[id] WHERE (authors.name = N'David') ) AS [__rnt] WHERE [__rnt].[__rn] > (10) Now like this: SELECT TOP (2) [__rnt].id FROM ( SELECT ROW_NUMBER() OVER (ORDER BY [posts].[id] ASC) AS [__rn], [posts].id FROM [posts] LEFT OUTER JOIN [authors] ON [authors].[id] = [posts].[author_id] LEFT OUTER JOIN [comments] ON [comments].[post_id] = [posts].[id] WHERE (authors.name = N'David') GROUP BY [posts].id ) AS [__rnt] WHERE [__rnt].[__rn] > (10)
1 parent e10adcd commit d64fcb4

File tree

3 files changed

+25
-3
lines changed

3 files changed

+25
-3
lines changed

CHANGELOG

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,11 @@
11

2+
* master *
3+
4+
* Pass AR tests by moving DISTINCT to GROUP BY in windowed SQL.
5+
- test_count_eager_with_has_many_and_limit_and_high_offset
6+
- test_eager_with_has_many_and_limit_and_high_offset
7+
8+
29
* 3.1.3 *
310

411
* Distinguish between identity and primary key key columns during schema reflection. Allows us

lib/arel/visitors/sqlserver.rb

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ def visit_Arel_Nodes_SelectStatementWithOutOffset(o, windowed=false)
138138
orders = o.orders.uniq
139139
if windowed
140140
projections = function_select_statement?(o) ? projections : projections.map { |x| projection_without_expression(x) }
141+
groups = projections.map { |x| projection_without_expression(x) } if windowed_single_distinct_select_statement?(o) && groups.empty?
141142
elsif eager_limiting_select_statement?(o)
142143
groups = projections.map { |x| projection_without_expression(x) }
143144
projections = projections.map { |x| projection_without_expression(x) }
@@ -162,7 +163,7 @@ def visit_Arel_Nodes_SelectStatementWithOutOffset(o, windowed=false)
162163
def visit_Arel_Nodes_SelectStatementWithOffset(o)
163164
orders = rowtable_orders(o)
164165
[ "SELECT",
165-
(visit(o.limit) if o.limit && !single_distinct_select_statement?(o)),
166+
(visit(o.limit) if o.limit && !windowed_single_distinct_select_statement?(o)),
166167
(rowtable_projections(o).map{ |x| visit(x) }.join(', ')),
167168
"FROM (",
168169
"SELECT ROW_NUMBER() OVER (ORDER BY #{orders.map{ |x| visit(x) }.join(', ')}) AS [__rn],",
@@ -238,6 +239,10 @@ def single_distinct_select_statement?(o)
238239
p1.respond_to?(:include?) && p1.include?('DISTINCT'))
239240
end
240241

242+
def windowed_single_distinct_select_statement?(o)
243+
o.limit && o.offset && single_distinct_select_statement?(o)
244+
end
245+
241246
def single_distinct_select_everything_statement?(o)
242247
single_distinct_select_statement?(o) && visit(o.cores.first.projections.first).ends_with?(".*")
243248
end
@@ -316,7 +321,17 @@ def find_and_fix_uncorrelated_joins_in_select_statement(o)
316321

317322
def rowtable_projections(o)
318323
core = o.cores.first
319-
if single_distinct_select_statement?(o)
324+
if windowed_single_distinct_select_statement?(o) && core.groups.blank?
325+
tn = table_from_select_statement(o).name
326+
core.projections.map do |x|
327+
x.dup.tap do |p|
328+
p.sub! 'DISTINCT', ''
329+
p.insert 0, visit(o.limit) if o.limit
330+
p.gsub! /\[?#{tn}\]?\./, '[__rnt].'
331+
p.strip!
332+
end
333+
end
334+
elsif single_distinct_select_statement?(o)
320335
tn = table_from_select_statement(o).name
321336
core.projections.map do |x|
322337
x.dup.tap do |p|

test/cases/pessimistic_locking_test_sqlserver.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ def setup
6565
end
6666

6767
should 'cope with eager loading un-locked paginated' do
68-
eager_ids_sql = /SELECT DISTINCT TOP \(5\).*FROM \[people\] WITH \(NOLOCK\)/
68+
eager_ids_sql = /SELECT TOP \(5\).*FROM \[people\] WITH \(NOLOCK\)/
6969
loader_sql = /FROM \[people\] WITH \(NOLOCK\).*WHERE \[people\]\.\[id\] IN/
7070
assert_sql(eager_ids_sql,loader_sql) do
7171
Person.all(:include => :readers, :lock => 'WITH (NOLOCK)', :limit => 5, :offset => 10)

0 commit comments

Comments
 (0)