identity_cache icon indicating copy to clipboard operation
identity_cache copied to clipboard

prefetch_associations assumes passed in records have loaded the same associations

Open dylanahsmith opened this issue 5 years ago • 0 comments

This method was initially just used internally to support the includes option to model.fetch_by_id, where the records had just been loaded, so it seemed like a safe assumption. Rails made a similar assumption with its association preloader for similar reasons, so it also seemed like this assumption made sense by being consistent with rails' association preloader. However, rails 5.2 changed the association preloader in https://github.com/rails/rails/pull/37747 to check the whole collection when checking if the association has already been loaded.

prefetch_associations has since been made public in identity_cache, which also makes it such that an application can pass it an array of records where the association might only be loaded on the first record. The bug fixed by https://github.com/Shopify/identity_cache/pull/449 also shows an additional reason why we might need to check all records, because an association could be modified on a subset of the records, which can cause it to fallback to using the active record association instead of the cached association.

We should get rid of this assumption for when prefetch_associations is called directly. However, we may want to leverage some assumptions we can make for the fetch_by_id includes option, such as that the association hasn't been modified and that embedded associations have been loaded, to avoid adding significant overhead for that common case.

dylanahsmith avatar Apr 06 '20 16:04 dylanahsmith