Repository navigation
Comments: Add wp_update_comment_counts() to reconcile stored counts - #58
Open
adamsilverstein wants to merge 15 commits into
Open
adamsilverstein wants to merge 15 commits into
adamsilverstein wants to merge 15 commits into
Conversation
wp_update_comment_count_now() only refreshes a post's stored comment_count when that post's comments change, so an existing count can become stale after the set of excluded comment types changes (for example when a plugin registers a type that opts out of default listings via default_excluded_comment_types). Registration runs on every request and is not a state transition, so there is no safe automatic trigger; the established core pattern for this is an explicit recount, like flush_rewrite_rules() for rewrite rules. Add a bulk recount helper that recomputes one or more posts' counts through wp_update_comment_count_now(), so it honors the same exclusion filter. A plugin that changes the excluded set calls it once, typically on activation. See #35214, #65537.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
 |
…re/65537-update-comment-counts
- Bump the comment 'last_changed' cache key: comment query results are salted only by that key, so on a persistent object cache the activation-time flow (register the exclusion filter, then recount) would otherwise keep serving results cached under the previous excluded set. - Include posts with a nonzero stored count but no remaining comment rows in the null path: SELECT DISTINCT on the comments table cannot see a post whose rows were all deleted while its stored count is stale. - Iterate the null path in keyset batches of 1000 instead of materializing every post ID in memory. - Skip negative IDs instead of silently recounting their absolute value, and document the per-post side effects (edit_post hooks fire for cache purgers and indexers) alongside the cache-invalidation behavior. - Tests: null-path return value, invalid/negative/nonexistent IDs, stale-count reset for commentless posts on both paths, and the last_changed bump.
…re/65537-update-comment-counts
The keyset loop was correct but its query was not batched in any useful sense. MySQL cannot push an outer ORDER BY ... LIMIT into a parenthesized UNION arm, so every iteration materialized all remaining rows of both arms into a temp table before taking its thousand - and since comment_count is unindexed, the posts arm scanned the whole remaining table each time. A recount of N commented posts cost roughly N squared over the batch size, which defeats the batching that is the whole point of the null path. Give each arm its own ORDER BY and LIMIT. Keyset semantics are unchanged: the first N rows of the union of two ascending sets are always among the first N of each, so the batch is the same batch, read with a bounded walk of each index. The batch size becomes filterable along the way, which also makes it testable across a boundary - including a post that has both comments and a stale count, so it turns up in both arms. See #65537.
The comment last_changed key was bumped first thing, before the post IDs were even validated, so wp_update_comment_counts( array() ) or a list of nothing but invalid IDs invalidated every cached comment query on the site while recounting nothing at all. Bump it once there is at least one post to visit. The existing cache test relied on that no-op bump, so it now recounts a real post, and the no-op cases get their own test asserting the key is left alone. See #65537.
The docblock recommended calling this from a plugin activation routine while also warning it can be expensive, which is contradictory advice: activation runs in a normal web request, and a full recount on a large site will hit max_execution_time partway through. Point at wp_schedule_single_event() or WP-CLI for that case, and say plainly that stopping partway is safe - posts visited before the stop are correct and the operation is idempotent. Three more things the function does that a caller could only learn by reading it: counts are written immediately rather than joining the wp_defer_comment_counting() queue, post IDs that do not exist are skipped and left out of the return value, and there is no capability check, so anything exposing this to a request has to add its own. See #65537.
…re/65537-update-comment-counts
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. Props Bot commandsYou can trigger Props Bot actions by commenting on this PR:
|
…re/65537-update-comment-counts
…re/65537-update-comment-counts
The null path of wp_update_comment_counts() bumped the comment last_changed key before running its first batch query, so on a site with no comments and no stale counts it still invalidated every cached comment query. The explicit-ID path already avoids that. Bump only once the first batch returns a post, still ahead of any recount so an interrupted run has already invalidated the cache. See #65537.
The docblock and the batch query comment ran to essay length, mixing caller guidance with design history. Keep what a caller needs (filter, hooks fired, where to run a full recount, cache bump, no capability check) and the one line that stops someone dropping the per-arm LIMITs. Also move @SInCE to 7.2.0 to match the parent default_excluded_comment_types filter. See #65537.
The batch test only used posts that both union arms return, so it could not catch a query that returns more than one batch of rows and ends the keyset loop early. Add posts that only one arm sees, alternating by ID across a batch size of 2. Also add native types to the test methods and helpers for PHPStan level 10. See #65537.
…re/65537-update-comment-counts
Trunk is in the 7.2 beta, so these additions can no longer ship in 7.2.0.
This was referenced Nov 8, 2026
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.
Stack: Custom Comment Types (#35214)
register_comment_type()registration API/wp/v2/comment-typesendpointmap_meta_cap()render_callbackdisplay callbackis_pingping groupingdefault_excluded_comment_typesfilterwp_update_comment_counts()recount helper (this PR)WordPress#12311 and WordPress#12310 are on WordPress/wordpress-develop; the rest are fork PRs targeting their parent branch and will retarget to trunk as their base lands.
Claude reviewed the recount helper and rewrote the description:
Trac ticket: https://core.trac.wordpress.org/ticket/35214, https://core.trac.wordpress.org/ticket/65537
AI Use
Code and description by 🤖 Claude Code, with me steering. I will review and test.