Query: prevent invalidated query caches from accumulating. - #256
Merged
Conversation
Port the maintenance result-cache fix to the development branch. Store the generation in the cached value, replace stale results under a stable key, and include the generation in primed relationship query results. Remove the unused private cache-key parameter and add regression tests. Props hellofromahmed for reporting the persistent-cache growth. Props robincornett for the existing unset-value cache-key correction. See #253, #254, #185.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
One new regression test is order-dependent/flaky due to last_changed initialization, and the stable-key overwrite path has a concurrency edge that can clobber a newer generation’s cached value.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Ports the query-result cache invalidation fix to the development branch by keeping a stable query-result cache key and embedding the cache generation (last_changed) in the cached value, so repeated invalidations replace rather than orphan entries.
Changes:
- Store
last_changedinside the cached query-result value and validate it before reusing cached results. - Switch query-result writes to overwrite the stable key (
cache_set) and align relationship-result priming to the same cached-value shape. - Add regression tests covering replacement after mutations, empty results, stale/missing generations, and invalidation during a DB read (plus update the enforced test-method count).
File summaries
| File | Description |
|---|---|
| tests/Database/Kern/Query/QueryCacheTest.php | Adds regression tests for stable-key cache replacement and generation-guard behavior. |
| src/Database/Traits/Query/Execution.php | Implements generation capture/check and stable-key cache write behavior for query results. |
| src/Database/Traits/Query/Cache.php | Removes generation from the cache key and updates priming to store generation in the cached value. |
| CLAUDE.md | Updates the documented enforced PHPUnit test-method count. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
185
to
188
| // Only store when caching is enabled for this query. | ||
| if ( true === $cache_results ) { | ||
| $this->cache_add( $cache_key, $cache_value, $this->cache_group ); | ||
| $this->cache_set( $cache_key, $cache_value, $this->cache_group ); | ||
| } |
Comment on lines
+359
to
+361
| wp_cache_delete( $key, $group ); | ||
| $before = wp_cache_get( 'last_changed', $group ); | ||
| $rotate = static function ( $sql ) use ( $group ) { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Port the maintenance query-result cache fix to the development branch. Repeated invalidations replace results at a stable key, with the generation stored in the value and checked before reuse. Capture the generation before normal query reads, update relationship result-cache priming to write the same format, and remove the unused private cache-key parameter.
The existing sentinel normalization remains intact. Add regression tests for replacement after mutations, empty results, missing/stale generations, and invalidation during a database read; update the enforced test-method count.
Validation: PHP 8.1 and 8.2 with WordPress 6.7.7 and MariaDB 10.2 each passed 1,761 tests / 3,861 assertions, including existing relationship priming tests. Composer validation, PHPStan, PHPCS, and diff checks passed.
This ports the focused query-result fix. Existing orphaned entries are not removed, distinct queries do not gain expiration, and the development branch's separate generation-suffixed secondary lookup caches are unchanged.
Props @hellofromahmed for reporting #253 and @robincornett for the existing sentinel correction (#185).
See #253, #254, #185.