Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix increase_parent_counter_cache callback #618

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kbrock
Copy link
Collaborator

@kbrock kbrock commented Mar 4, 2023

Counter caches get corrupted.

It is possible that strategy: sql is the culprit here.
There is also probably a problem around cache depth too.

The increase_parent_counter_cache callback is not consistent with the decrease_parent_counter_cache or the udpate


some ideas on making them consistent and fixing the problem

  • update parent_counter_cache to respects @_trigger_update_callback
  • drop @_trigger_delete_callback (are these unnecessary internals?)

Not sure if update follows the same rules as destroy introduced in 63b42f5 But would like both trees to follow the same logic

There are reports of incorrect cache counts. So when insert and destroy have different guard clauses, it is suspect.

I admit that I am not totally familiar to the core use case so I will need to more into why this was introduced in #307 - trying to resolve #584 (and may be able to provide advice for #586 )

TODO:

@kbrock kbrock mentioned this pull request Mar 4, 2023
@kbrock kbrock force-pushed the counter_cache_fix branch from 86d7c4a to 493f079 Compare March 4, 2023 23:31
@kbrock kbrock force-pushed the counter_cache_fix branch 2 times, most recently from 76c34f2 to d5f587b Compare March 13, 2023 02:37
@kbrock
Copy link
Collaborator Author

kbrock commented Mar 13, 2023

Feature (not fixing the bug):

  • see if we can use REPLACE instead of REGEX_REPLACE. regex buys us anchoring at the beginning of the line (which is needed if we want to support materialized_path), but REPLACE buys us binary support.

@kbrock kbrock force-pushed the counter_cache_fix branch from d5f587b to 6f4e8a5 Compare March 28, 2023 21:56
@kbrock kbrock force-pushed the counter_cache_fix branch from 6f4e8a5 to 1b7f0b3 Compare July 13, 2023 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

find and fix corrupted children counts
1 participant