Skip to content

Commit f3fc912

Browse files
Merge pull request #2 from wpolicarpo/fix-calculations-tests
Fix Calculations tests
2 parents fcd0c31 + c91af08 commit f3fc912

File tree

2 files changed

+26
-14
lines changed

2 files changed

+26
-14
lines changed

lib/active_record/connection_adapters/sqlserver/core_ext/calculations.rb

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,20 +9,7 @@ module Calculations
99
private
1010

1111
def build_count_subquery(relation, column_name, distinct)
12-
relation.select_values = [
13-
if column_name == :all
14-
distinct ? table[Arel.star] : Arel.sql(FinderMethods::ONE_AS_ONE)
15-
else
16-
column_alias = Arel.sql("count_column")
17-
aggregate_column(column_name).as(column_alias)
18-
end
19-
]
20-
21-
relation = relation.offset(0)
22-
subquery = relation.arel.as(Arel.sql("subquery_for_count"))
23-
select_value = operation_over_aggregate_column(column_alias || Arel.star, "count", false)
24-
25-
Arel::SelectManager.new(subquery).project(select_value)
12+
super(relation.unscope(:order), column_name, distinct)
2613
end
2714
end
2815
end

test/cases/coerced_tests.rb

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,14 @@ def test_limit_with_offset_is_kept_coerced
176176
queries.first.must_match %r{ORDER BY \[accounts\]\.\[id\] ASC OFFSET @0 ROWS FETCH NEXT @1 ROWS ONLY.*@0 = 1, @1 = 1}
177177
end
178178

179+
# SQL Server needs an alias for the calculated column
180+
coerce_tests! :test_distinct_count_all_with_custom_select_and_order
181+
def test_distinct_count_all_with_custom_select_and_order_coerced
182+
accounts = Account.distinct.select("credit_limit % 10 AS the_limit").order(Arel.sql("credit_limit % 10"))
183+
assert_queries(1) { assert_equal 3, accounts.count(:all) }
184+
assert_queries(1) { assert_equal 3, accounts.load.size }
185+
end
186+
179187
# Leave it up to users to format selects/functions so HAVING works correctly.
180188
coerce_tests! :test_having_with_strong_parameters
181189
end
@@ -189,6 +197,7 @@ class ChangeSchemaTest < ActiveRecord::TestCase
189197
coerce_tests! :test_create_table_with_bigint,
190198
:test_create_table_with_defaults
191199
end
200+
192201
class ChangeSchemaWithDependentObjectsTest < ActiveRecord::TestCase
193202
# In SQL Server you have to delete the tables yourself in the right order.
194203
coerce_tests! :test_create_table_with_force_cascade_drops_dependent_objects
@@ -701,6 +710,22 @@ def test_relations_dont_load_all_records_in_inspect_coerced
701710
# so we are skipping all together.
702711
coerce_tests! :test_empty_complex_chained_relations
703712

713+
# Can't apply offset withour ORDER
714+
coerce_tests! %r{using a custom table affects the wheres}
715+
test 'using a custom table affects the wheres coerced' do
716+
post = posts(:welcome)
717+
718+
assert_equal post, custom_post_relation.where!(title: post.title).order(:id).take
719+
end
720+
721+
# Can't apply offset withour ORDER
722+
coerce_tests! %r{using a custom table with joins affects the joins}
723+
test 'using a custom table with joins affects the joins coerced' do
724+
post = posts(:welcome)
725+
726+
assert_equal post, custom_post_relation.joins(:author).where!(title: post.title).order(:id).take
727+
end
728+
704729
# Use LEN() vs length() function.
705730
coerce_tests! :test_reverse_arel_assoc_order_with_function
706731
def test_reverse_arel_assoc_order_with_function_coerced

0 commit comments

Comments
 (0)