Permalink
Browse files

Revert "Merge pull request #10710 from senny/5554_let_the_database_ra…

…ise_on_counts"

This reverts commit b8e2978.

Conflicts:
	activerecord/CHANGELOG.md
	activerecord/lib/active_record/relation/calculations.rb

Reason: This change is not backward compatible and it was reverted
before 4.0.0 at 2ad168e so we can't include in 4-0-stable.

Closes #12417
  • Loading branch information...
1 parent fbf5ece commit 682d7c7035fed76c42ba6fefa38973387e80409e @rafaelfranca rafaelfranca committed Oct 1, 2013
View
@@ -196,22 +196,6 @@
*Yves Senn*
-* 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*
* Fix the `:primary_key` option for `has_many` associations.
@@ -234,7 +234,7 @@ def size
def empty?
return @records.empty? if loaded?
- c = count(:all)
+ c = count
c.respond_to?(:zero?) ? c.zero? : c.empty?
end
@@ -207,14 +207,15 @@ def perform_calculation(operation, column_name, options = {})
end
if operation == "count"
- column_name ||= select_for_count
+ column_name ||= (select_for_count || :all)
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?
@@ -378,9 +379,8 @@ def type_cast_using_column(value, column)
# TODO: refactor to allow non-string `select_values` (eg. Arel nodes).
def select_for_count
if select_values.present?
- select_values.join(", ")
- else
- :all
+ select = select_values.join(", ")
+ select if select !~ /[,*]/
end
end
@@ -162,15 +162,6 @@ def test_no_limit_no_offset
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]

0 comments on commit 682d7c7

Please sign in to comment.