Skip to content

update behavior of counter_cache#9236

Closed
jasl wants to merge 2 commits intorails:masterfrom
jasl:fix_counter_update
Closed

update behavior of counter_cache#9236
jasl wants to merge 2 commits intorails:masterfrom
jasl:fix_counter_update

Conversation

@jasl
Copy link
Copy Markdown
Contributor

@jasl jasl commented Feb 10, 2013

I update behavior of counter_cache, the reason has described at #8997.

@jasl
Copy link
Copy Markdown
Contributor Author

jasl commented Feb 10, 2013

@rafaelfranca sorry to trouble you again, I fix this but changed it's behavior, please check.
add_counter_cache_callbacks seems ugly but I didn't have another better way, also I tried to make update counter after destroy but failed cause ActiveModel::Dirty doesn't support.

@jasl
Copy link
Copy Markdown
Contributor Author

jasl commented Feb 18, 2013

@vad4msiu seems no one attention this

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is supposed to be specified_primary_key.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@parndt fixed

@jeremy
Copy link
Copy Markdown
Member

jeremy commented May 5, 2014

#14849

@jeremy jeremy closed this May 5, 2014
@kevin-jj
Copy link
Copy Markdown

counter_cache still increases twice when association would be updated.

@tomav
Copy link
Copy Markdown

tomav commented Jul 25, 2015

Same problem here, tried with 4.2.1 and 4.2.3.

kamipo added a commit to kamipo/rails that referenced this pull request Sep 18, 2018
This is a 4th attempt to make counter cache transactional completely.

Past attempts: rails#9236, rails#14849, rails#23357.

All existing counter cache issues (increment/decrement twice, lost
increment) are caused due to updating counter cache on the outside of
the record saving transaction by assigning belongs_to record, even
though assigning that doesn't cause the record saving.

We have the `@_after_replace_counter_called` guard condition to mitigate
double increment/decrement issues, but we can't completely prevent that
inconsistency as long as updating counter cache on the outside of the
transaction, since saving the record is not always happened after that.

We already have handling counter cache after create/update/destroy,

https://github.com/rails/rails/blob/1b90f614b1b3d06b7f02a8b9ea6cd84f15d58643/activerecord/lib/active_record/counter_cache.rb#L162-L189
https://github.com/rails/rails/blob/1b90f614b1b3d06b7f02a8b9ea6cd84f15d58643/activerecord/lib/active_record/associations/builder/belongs_to.rb#L33-L59

so just removing assigning logic on the belongs_to association makes
counter cache transactional completely.

Closes rails#14849.
Closes rails#23357.
Closes rails#31493.
Closes rails#31494.
Closes rails#32372.
Closes rails#33113.
Closes rails#33117
Closes rails#33129.
Closes rails#33458.
@jasl jasl deleted the fix_counter_update branch December 31, 2019 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants