Don't call wp_count_comments() when we don't need to

[johnbillion]

Comments get counted on every admin screen with wp_count_comments(). We can skip a database call by not counting them if the user doesn't have the edit_posts capability.

---

Side note: With a persistent object cache it's possible to have admin screens with zero database queries with this patch applied. As an exercise you could write a database class that lazy loads MySQL and you'll get an admin screen that doesn't load MySQL. Almost completely pointless, but interesting nonetheless.

[johnbillion]

Patch

[scribu]

I think 'moderate_comments' would be a more appropriate capability to check. Related: #12104

[ocean90]

Related: #11409, #14751

[johnbillion]

#32366 was marked as a duplicate.

[justindocanto]

The proposed solution here does not fix what I mentioned in #32366.

Yes, both mention that it shouldn't be called when it's not needed, but the scenarios are completely different.

If I'm an admin and can 'edit_posts' Im still going to have the problem mentioned in #32366 because I'm still querying 5,000,000 comments on every single page load.

I think this ticket needs a better way to disable the function from being called OR we leave my ticket open and create another solution for the problem I brought up since this solution does not address it.

[wonderboymusic]

@justindocanto - this ticket suffices. We are attacking comments hard this release, will look at your issue with this one.

[DrewAPicture]

@wonderboymusic: What's the word on a new patch here?

[wonderboymusic]

this just needs an hour of my time at some point

[bordoni]

Updated to 4.4 + More readable code.

[bordoni]

Don't know if it helps or not, just updated the Patch for 4.4;

[justindocanto]

Would like to point out that the proposed patch still does not address the issue mentioned in #32366 which was merged into this issue.

With this patch, a user who can edit posts still has the issue of WP querying potentially millions of comments on every single page load and there is no way to disable it other than editing core files. There should be a 2nd check of some kind. Some sort of check if the user has flagged for this to be disabled...

e.g.

if ( current_user_can( 'edit_posts' ) && IF_USER_HAS_NOT_DISABLED_THIS ) {

[sboisvert]

The issue #32366 solves (slow query on admin) isn't solved by the latest patch.
What would be nice is to be able to short circuit this with a filter. Either to ignore this completely or to enable others to add a layer of caching in wp_count_comments.
Would people be open to putting a filter in there or another way of bypassing/fixing the potentially slow query?

[bordoni]

I think this problem is better addressed as a something on the function since there are other places on the Administration that WP will count comments.

[boonebgorges]

There is already a filter in the function that lets you disable the call to get_comment_count(). https://core.trac.wordpress.org/browser/tags/4.3.1/src/wp-includes/comment.php?marks=1725-1727#L1694

[justindocanto]

@bordoni - I disagree. As mentioned in #32366, there are ways to disable almost all of those occurrences (e.g., the comments menu item in the left admin menu, the comments column in the 'all posts' screen', etc.) have ways to disable them in a way that also stops the associated queries from happening.

The code in discussion here, in the top admin menu, does not have a way to disable it and that is why I keep pushing for the issue in #32366 (some sort of way to disable it other than 'user can edit posts') to be considered, since it got merged.

[bordoni]

After I've checked the code I saw that it's possible to fix the problem by adding a filter to wp_count_comments.
Keep in mind that you might need to add a few more conditionals to make it work.

<?php
function filter_comments_counter_admin( $original, $post_id ){
    if ( is_admin() ) {
        return array(
                'approved'            => 0,
                'awaiting_moderation' => 0,
                'spam'                => 0,
                'trash'               => 0,
                'post-trashed'        => 0,
                'total_comments'      => 0,
                'all'                 => 0,
        );
    }
}
add_filter( 'wp_count_comments' 'filter_comments_counter_admin' );

As @boonebgorges said.

[sboisvert]

@boonebgorges My apologies, I was reading the get_comment_count() code by mistake. Thanks for pointing it out. @bordoni is correct that it's easy to implement a solution using that filter.

[justindocanto]

Replying to boonebgorges:

There is already a filter in the function that lets you disable the call to get_comment_count(). https://core.trac.wordpress.org/browser/tags/4.3.1/src/wp-includes/comment.php?marks=1725-1727#L1694

This functionality uses wp_count_comments() not get_comment_count().

The query that functionality runs is $count = $wpdb->get_results( "SELECT comment_approved, COUNT( * ) AS num_comments FROM {$wpdb->comments} {$where} GROUP BY comment_approved", ARRAY_A ); which is a resource intensive query to run on every single page view when you have a large amount of comments.

[boonebgorges]

Replying to justindocanto:

Replying to boonebgorges:

There is already a filter in the function that lets you disable the call to get_comment_count(). https://core.trac.wordpress.org/browser/tags/4.3.1/src/wp-includes/comment.php?marks=1725-1727#L1694

This functionality uses wp_count_comments() not get_comment_count().

The query that functionality runs is $count = $wpdb->get_results( "SELECT comment_approved, COUNT( * ) AS num_comments FROM {$wpdb->comments} {$where} GROUP BY comment_approved", ARRAY_A ); which is a resource intensive query to run on every single page view when you have a large amount of comments.

Please look more carefully at the link I provided. get_comment_count() is where the SQL query lives. wp_count_comments() is a wrapper that provides caching, as well as a filter for short-circuiting the call to get_comment_count(). I concede that it's not a very fine-grained filter, but it will allow for workarounds like the one described by @bordoni in 18.

[justindocanto]

@bordoni - I understand that this would stop the query, but 1) Also stop the function in the entire WordPress area and 2) you would have a broken 'pending comment count' in the top admin menu.

That suggestion is more of a duck tape job and not a proper solution. There should be a way to remove the item from the menu that also stops the query and allows the function to run anywhere else it is used.

[bordoni]

@justindocanto the point is, the solution I provided, as I said requires some polishing around the edges but it's doable, no need for an extra filter there.

[justindocanto]

@boonebgorges I disagree. They are similar, but not the same function. See line 1738 within wp_count_comments(). This is the query that runs that causes slow queries.

[boonebgorges]

Sorry, I forgot that wp_count_comments() was made a wrapper only in 4.4. https://core.trac.wordpress.org/browser/trunk/src/wp-includes/comment-functions.php#L1011 [33822] #19903

[justindocanto]

@bordoni I disagree. Even if you hacked together something to only disable it when it runs on wp-admin/menu.php you would still have a broken pending comment count because the code blindly uses $awaiting_mod.

Again, the idea of stopping the wp_comment_count query is a duck tape solution, and not a proper answer to the overall issue detailed on #32366.

There needs to be a way to disable this menu item, and it's associated query. The easiest answer is a simple flag.

[bordoni]

@justindocanto It's not a duck tape solution because that's how it's currently dealt with, the label for comments is always there, but if you have 0 comments it will be hidden by CSS.

On the file /src/wp-admin/css/admin-menu.css there is a ref to:

#adminmenu li span.count-0 {
    display: none;
}

You just got to add a new css rule somewhere to prevent it from showing up on the admin menu, maybe:

#wpadminbar #wp-admin-bar-comments .count-0 {
    display: none;
}

[justindocanto]

@bordoni Again - I disagree.

To do your method, a user would have to figure out a way to...

1) Figure out a way to only trigger your wp_count_comments filter when being used in /wp-admin/menu.php and send the function false information

2) This then depends on a css rule, meant to hide the pending comment count bubble when theres 0 pending comment, irregardless if there ARE pending comments.

1 and 2 are both using inaccurate info to trick WP to do what you want. That's a hack to me, not a solution. Other menu items have ways to disable them. I don't understand why we wouldn't want a way to properly disable this as well.

[boonebgorges]

@justindocanto I'm not opposed to a more explicit flag. I think what this flag would do is disable the display of the comment count within the Comments menu item, *not* disable the menu item altogether. No other top-level menu items can be disabled via filter at the moment, so I don't see a reason to introduce this functionality here. (Coming up with a more sane method for registering and rendering menus would be nice, but is way beyond the scope of this ticket.) I'd be glad to see a patch that does this.

As for whether the wp_count_comments filter is a "hack" for this purpose: Yes, of course it is. I don't think anyone is defending it as a robust API. My only point in bringing it up in the context of this ticket was that someone said we needed a filter at the level of the wp_count_comments filter, and I wanted to point out that one already exists there. This can be used as a workaround until a more robust solution is in place.

19372.1.patch​ should go in either way.

[justindocanto]

@boonebgorges Alright, I can get behind that. I'll get started on figuring out a way to use that function only on the menu file. Thanks everybody.

[boonebgorges]

A new filter is not going to happen for 4.4. If people want it (and I think it's a decent idea), please open a new ticket.

[boonebgorges]

In 35691:

Eliminate a db query when building the admin menu for non-privileged users.

Users who cannot edit_posts do not see the Comments nav item anyway, so
don't bother running a query that gets a comment count to display in the menu.

Props bordoni, johnbillion.
Fixes #19372.

[justindocanto]

Replying to boonebgorges:

A new filter is not going to happen for 4.4. If people want it (and I think it's a decent idea), please open a new ticket.

I did. Please reopen #32366.

[boonebgorges]

Replying to justindocanto:

Replying to boonebgorges:

A new filter is not going to happen for 4.4. If people want it (and I think it's a decent idea), please open a new ticket.

I did. Please reopen #32366.

Reopened.