diff options
| -rw-r--r-- | activerecord/CHANGELOG.md | 16 | ||||
| -rw-r--r-- | activerecord/lib/active_record/relation/calculations.rb | 16 | ||||
| -rw-r--r-- | activerecord/test/cases/calculations_test.rb | 9 |
3 files changed, 31 insertions, 10 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 98033e904e..170084cf67 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,19 @@ +* Remove column restrictions for `count`, let the database raise if the SQL is + invalid. The previos behavior was untested and surprising for the user. + Fixes #5554. + + Example: + + User.select("name, username").count + # Before => SELECT count(*) FROM users + # After => ActiveRecord::StatementInvalid + + # you can still use `count(:all)` to perform a query unrelated to the + # selected columns + User.select("name, username").count(:all) # => SELECT count(*) FROM users + + *Yves Senn* + * Rails now automatically detects inverse associations. If you do not set the `:inverse_of` option on the association, then Active Record will guess the inverse association based on heuristics. diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index ccb48247b7..4becf3980d 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -207,15 +207,18 @@ module ActiveRecord end if operation == "count" - column_name ||= (select_for_count || :all) + if select_values.present? + column_name ||= select_values.join(", ") + else + column_name ||= :all + end unless arel.ast.grep(Arel::Nodes::OuterJoin).empty? distinct = true end column_name = primary_key if column_name == :all && distinct - - distinct = nil if column_name =~ /\s*DISTINCT\s+/i + distinct = nil if column_name =~ /\s*DISTINCT[\s(]+/i end if group_values.any? @@ -376,13 +379,6 @@ module ActiveRecord column ? column.type_cast(value) : value end - def select_for_count - if select_values.present? - select = select_values.join(", ") - select if select !~ /[,*]/ - end - end - def build_count_subquery(relation, column_name, distinct) column_alias = Arel.sql('count_column') subquery_alias = Arel.sql('subquery_for_count') diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index c8ac77984f..0f3f9aecfc 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -167,6 +167,15 @@ class CalculationsTest < ActiveRecord::TestCase assert_no_match(/OFFSET/, queries.first) end + def test_count_on_invalid_columns_raises + e = assert_raises(ActiveRecord::StatementInvalid) { + Account.select("credit_limit, firm_name").count + } + + assert_match "accounts", e.message + assert_match "credit_limit, firm_name", e.message + end + def test_should_group_by_summed_field_having_condition c = Account.group(:firm_id).having('sum(credit_limit) > 50').sum(:credit_limit) assert_nil c[1] |