Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

When Active Record has already loaded a unique association `.size` returns the wrong number. #5453

Merged
merged 1 commit into from

3 participants

@JonRowe

When ActiveRecord has already eager loaded an association a call to that association returns the wrong number, count still returns correctly because it executes a fresh query. This may be undesirable. This patch respects the :uniq flag on the association and filters the already loaded array accordingly before calling .size.

Tested against SQLite, Mysql and Postgres (even though this modified no sql).

Fixes #4982 (symptoms shown below)

User.where(id:5).first.followers.size
  User Load (0.8ms)  SELECT `users`.* FROM `users` WHERE `users`.`id` = 5 LIMIT 1
  User Load (0.3ms)  SELECT DISTINCT `users`.* FROM `users` INNER JOIN `followers_users` ON `users`.`id` = `followers_users`.`user_id` WHERE `followers_users`.`follower_id` = 5
User.where(id:5).includes(:followers).first.followers.size
  User Load (0.6ms)  SELECT `users`.* FROM `users` WHERE `users`.`id` = 5 LIMIT 1
  SQL (0.8ms)  SELECT `users`.*, `t0`.`follower_id` AS ar_association_key_name FROM `users` INNER JOIN `followers_users` `t0` ON `users`.`id` = `t0`.`user_id` WHERE `t0`.`follower_id` IN (5)
 => 18

See https://gist.github.com/2044768 for files needed to replicate

@jeremyf

The tests all pass for this bug fix. @tenderlove @jonleighton

@tenderlove tenderlove merged commit 0821295 into rails:master
@tenderlove
Owner

@jeremyf thanks for the confirmation! :heart:

@JonRowe

Thanks @tenderlove :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 15, 2012
  1. @JonRowe
This page is out of date. Refresh to see the latest.
View
8 activerecord/lib/active_record/associations/collection_association.rb
@@ -248,8 +248,12 @@ def destroy(*records)
# This method is abstract in the sense that it relies on
# +count_records+, which is a method descendants have to provide.
def size
- if !find_target? || (loaded? && !options[:uniq])
- target.size
+ if !find_target? || loaded?
+ if options[:uniq]
+ target.uniq.size
+ else
+ target.size
+ end
elsif !loaded? && options[:group]
load_target.size
elsif !loaded? && !options[:uniq] && target.is_a?(Array)
View
6 activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
@@ -338,6 +338,12 @@ def test_uniq_option_prevents_duplicate_push
assert_equal 3, project.developers.size
end
+ def test_uniq_when_association_already_loaded
+ project = projects(:active_record)
+ project.developers << [ developers(:jamis), developers(:david), developers(:jamis), developers(:david) ]
+ assert_equal 3, Project.includes(:developers).find(project.id).developers.size
+ end
+
def test_deleting
david = Developer.find(1)
active_record = Project.find(1)
Something went wrong with that request. Please try again.