aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
authorKlas Eskilson <klas.eskilson@gmail.com>2017-01-03 16:24:42 -0800
committerKlas Eskilson <klas.eskilson@gmail.com>2017-02-07 11:09:34 -0800
commit4a7b4f88cd3fc10ab56edc3f88d5db0c4f871dc9 (patch)
tree55393f0cecba8fb49b28585502d7bea653129963 /activerecord
parent6599e07674d2389de45b91a11d3be5eb0fbd92f3 (diff)
downloadrails-4a7b4f88cd3fc10ab56edc3f88d5db0c4f871dc9.tar.gz
rails-4a7b4f88cd3fc10ab56edc3f88d5db0c4f871dc9.tar.bz2
rails-4a7b4f88cd3fc10ab56edc3f88d5db0c4f871dc9.zip
Use `count(:all)` in HasManyAssociation#count_records
Problem: Calling `count` on an association can cause invalid SQL queries to be created where the `SELECT COUNT(a, b, c)` function receives multiple columns. This will cause a `StatementInvalid` exception later on. Solution: Use `count(:all)`, which generates a `SELECT COUNT(*)...` query independently of the association. This also includes a test case that, before the fix, broke.
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/CHANGELOG.md5
-rw-r--r--activerecord/lib/active_record/associations/has_many_association.rb2
-rw-r--r--activerecord/test/cases/associations/has_many_associations_test.rb8
-rw-r--r--activerecord/test/models/company.rb2
4 files changed, 16 insertions, 1 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index bae2cab457..41cf2bf159 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,8 @@
+* Use `count(:all)` in `HasManyAssociation#count_records` to prevent invalid
+ SQL queries for association counting.
+
+ *Klas Eskilson*
+
* Deprecate locking records with unpersisted changes.
*Marc Schütz*
diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb
index b413eb3f9c..827e38ab4a 100644
--- a/activerecord/lib/active_record/associations/has_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_association.rb
@@ -66,7 +66,7 @@ module ActiveRecord
count = if reflection.has_cached_counter?
owner._read_attribute(reflection.counter_cache_column).to_i
else
- scope.count
+ scope.count(:all)
end
# If there's nothing in the database and @target has no new records
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index cbecfa84ff..df75776254 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -2446,6 +2446,14 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
assert_equal [first_bulb, second_bulb], car.bulbs
end
+ test "association size calculation works with default scoped selects when not previously fetched" do
+ firm = Firm.create!(name: "Firm")
+ 5.times { firm.developers_with_select << Developer.create!(name: "Developer") }
+
+ same_firm = Firm.find(firm.id)
+ assert_equal 5, same_firm.developers_with_select.size
+ end
+
test "double insertion of new object to association when same association used in the after create callback of a new object" do
reset_callbacks(:save, Bulb) do
Bulb.after_save { |record| record.car.bulbs.load }
diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb
index 20e37710e7..7ab969cdf1 100644
--- a/activerecord/test/models/company.rb
+++ b/activerecord/test/models/company.rb
@@ -85,6 +85,8 @@ class Firm < Company
has_many :association_with_references, -> { references(:foo) }, class_name: "Client"
+ has_many :developers_with_select, -> { select("id, name, first_name") }, class_name: "Developer"
+
has_one :lead_developer, class_name: "Developer"
has_many :projects