Hello,
On https://github.com/rails/rails/issues/10865 there's a discussion about counter caches being incremented/decremented twice if you assign records to belongs_to associations, which started happening in 4.0 and is still an issue in 4.1.
Basically it seems that there are now two implementations of counter caching for belongs to. One is called from the ActiveRecord::Associations::BelongsToAssociation#replace method. The second is a set of callbacks in ActiveRecord::Associations::Builder::BelongsTo which update the counter after create and after update.
Unfortunately these two implementations will both run if you update a belongs_to association to point to a new record, so the counts go up twice on 4.x. Also interesting is that if you instead update the association by changing the _id columns, this doesn't happen - only if you assign to the belongs_to itself.
(The reason it usually worked on 3.2 is that 3.2 was missing the _update callback, so we had a different set of bugs, but #replace-ing a record and updating the parent generally counted correctly.)
There is an older pull request on the above issue trying to work around this by keeping track of whether one of the two methodologies has already run.
Instead of doing that, I'd like to propose that we remove the #replace implementation of counter caching entirely. It runs the moment you assign a record to the association, even though this does not trigger a save.
IMHO, if the owner record (the one that defines the belongs_to) hasn't been saved yet, then the database state shouldn't be updated - otherwise the counter is being changed outside the save transaction. Even aside from the possibility of transaction rollback, the app may simply decide not to save the record at all, for example if a validation error occurs, and then the counter value is permanently left wrong.
So personally I think that the #replace implementation is simply the wrong place to do it and that we should use the more reliable callback mechanism instead.
Any thoughts on this approach? This would be a behavior change, so not appropriate for a point release, but would a pull request be considered for 4.2?
Cheers, Will