_prime_post_caches() doesn't account for primed posts without primed meta/terms

[ocean90]

_prime_post_caches() tries to be smart about whether posts cache needs to be updated by first checking if IDs aren't already cached. Unfortunately this becomes an issue if the post itself is already cached but not the meta/terms. In this case _get_non_cached_ids() returns an empty array and thus update_post_caches() isn't called any longer.

I notices this first by looking into update_post_thumbnail_cache() since it wasn't working as expected.

• It calls get_post_thumbnail_id() which calls get_post() and thus primes the cache in the posts group
• Because of that _prime_post_caches() basically does nothing anymore since the post is already cached
• Result: No meta cache is primed

But then I also found another case which was due to the changes in [54352] for 6.1. update_post_caches() was replaced with _prime_post_caches() causing the same issue as mentioned above. If a post is already cached but meta/terms not, the cache won't be updated. So this part is actually a regression in 6.1.

I'm not sure if we can update _prime_post_caches() to also check if meta/terms caches are already set or if should just continue using update_post_caches().

[peterwilsoncc]

I think this is a bug with _prime_post_caches().

For post meta, a separate check can be done for _get_non_cached_ids( $ids, 'post_meta' );.

Post terms would be more complicated as the group name differs for each taxonomy: "{$taxonomy}_relationships". update_object_term_cache() already handles this so if the function is called to warm caches, then it might be possible to call without wrapping it in a _get_non_cached_ids() check.

https://core.trac.wordpress.org/ticket/57163

Todo:

• [ ] Unit tests

[ocean90]

Replying to peterwilsoncc:

I think this is a bug with _prime_post_caches().

I tend to agree here, especially since the function is no longer considered to be private.

[spacedmonkey]

The issue with all the _prime functions, is that is just guesses that the if the main object is primed not to both to prime the meta / terms. This data should treated as different types and we should ensure it primed always.

Edit: I suspect Test_Query_CacheResults::test_author_cache_warmed_by_the_loop fails due to this very issue. The post cache is warmed by wp_insert_post() when created but its term and meta data cache is not.

Yes, that makes sense. We should probably start documenting what type of queries are expected instead of only having a "random" number in assets call.

Trac ticket: https://core.trac.wordpress.org/ticket/57163

I have created an alternative version of this PR ​https://github.com/WordPress/wordpress-develop/pull/3662. This just removes the usage of update_post_caches.

[ocean90]

_prime_comment_caches(), _prime_term_caches(), and _prime_site_caches() are having the same issue. So whatever solution we'll use here should also get applied to them. Will create a follow-up ticket.

[spacedmonkey]

Replying to ocean90:

_prime_comment_caches(), _prime_term_caches(), and _prime_site_caches() are having the same issue. So whatever solution we'll use here should also get applied to them. Will create a follow-up ticket.

+1 to this. We just have to ensure that, we do not break lazy loading of term / comment meta at the same time.

In addition to my suggestions earlier, I've pushed:

• added Tests_Post_PrimePostCaches to the cache group too
• new test to ensure lazy loading term meta works as expected (this probably falls under test improvements for the purpose of commits.
• self merged my suggestions earlier.

I think this is good to go in but as it's a mix of my, @ocean90's and @spacedmonkey's code we should probably approve each others contributions prior to commit.

[peterwilsoncc]

The ​linked pull request is a group effort and has committer approvals of each other's code.

The commit should probably include a "see #56793" tag as it adds an additional test for query caching.

I'm marking this as commit ready.

@peterwilsoncc Should we change _prime_comment_caches(), _prime_term_caches(), and _prime_site_caches() in this PR as well. They should all work the same way, IMO.

@spacedmonkey We'll change them but in a new ticket/PR for 6.2 since this isn't regression unlike this one.

@spacedmonkey We'll change them but in a new ticket/PR for 6.2 since this isn't regression unlike this one.

Let's do this in another ticket in 6.2.

[ocean90]

In 54892:

Query: Account for primed post caches without primed post meta/term caches.

In [54352] update_post_caches() was replaced by _prime_post_caches() to reduce excessive object cache calls. That's because _prime_post_caches() checks first if post IDs aren't already cached. Unfortunately this becomes an issue if a post itself is cached but not the meta/terms.
To fix this regression, _prime_post_caches() now always calls update_postmeta_cache() and update_object_term_cache() depending on the arguments passed to it. Both functions internally check whether IDs are already cached so the fix from [54352] remains in place.

Props peterwilsoncc, spacedmonkey, ocean90.
Fixes #57163.

[ocean90]

Re-opening for backport to 6.1

[ocean90]

In 54893:

Revert [54892] from the 6.1 branch.

This will be added again once committed to trunk first.

See #57163.

[ocean90]

In 54894:

Query: Account for primed post caches without primed post meta/term caches.

In [54352] update_post_caches() was replaced by _prime_post_caches() to reduce excessive object cache calls. That's because _prime_post_caches() checks first if post IDs aren't already cached. Unfortunately this becomes an issue if a post itself is cached but not the meta/terms.
To fix this regression, _prime_post_caches() now always calls update_postmeta_cache() and update_object_term_cache() depending on the arguments passed to it. Both functions internally check whether IDs are already cached so the fix from [54352] remains in place.

Props peterwilsoncc, spacedmonkey, ocean90.
Fixes #57163.

[ocean90]

Replying to ocean90:

_prime_comment_caches(), _prime_term_caches(), and _prime_site_caches() are having the same issue. So whatever solution we'll use here should also get applied to them. Will create a follow-up ticket.

Done in #57227.

Committed in https://core.trac.wordpress.org/changeset/54894.

[SergeyBiryukov]

Moving to 6.2.1, as there are no plans for 6.1.2 at this time.

[SergeyBiryukov]

This is already included in the 6.2 branch as of [55504], no backport needed.