Opened 16 months ago

Last modified 13 months ago

#19903 new enhancement

wp_count_comments() and get_comments_count() both do SQL queries

Reported by: markjaquith Owned by:
Priority: normal Milestone: Future Release
Component: Comments Version: 3.3.1
Severity: normal Keywords:
Cc:

Description (last modified by markjaquith)

wp_count_comments() and get_comments_count() are similar functions, with a few differences:

  • Their results are returned in different formats:
    • wp_count_comments() returns an object of:
      • spam
      • approved
      • moderated
      • total_comments
      • trash
      • post-trashed
    • get_comment_count() returns an array of:
      • spam
      • approved
      • awaiting_moderation
      • total_comments
  • wp_count_comments() caches, but get_comment_count() always hits the database.
  • get_comment_count() is used nowhere in WordPress core.

I propose that we add 'trash' and 'post-trashed' reporting to get_comment_count(), and then have wp_count_comments() use get_comment_count() instead of doing its own SQL queries.

See #6884 and #19901

Attachments (1)

19903.diff (3.9 KB) - added by markjaquith 16 months ago.

Download all attachments as: .zip

Change History (9)

Hi Mark,

Wouldn't it make better sense to simply get rid of get_comment_count and make wp_count_comments count in the optimised way (see ticket #19901)?

Keeping them both around is very confusing, especially as get_comment_count is not used in core. Duplicate functions have been a bit of a bugbear for us.

Alec

Wouldn't it make better sense to simply get rid of get_comment_count and make wp_count_comments count in the optimised way

Yeah, probably would. I think I was just thinking we needed both because I'd assumed that core needed the non-cached one, which isn't the case... it doesn't use it at all.

Patch deprecates get_comment_count().

Could the deprecated version become a wrapper for wp_count_comments()? We'd simply need to rename the returned array keys.

comment:5 follow-up: ↓ 6   markjaquith16 months ago

  • Description modified (diff)

Fixing the description to note that wp_count_comments() returns an object.

Could the deprecated version become a wrapper for wp_count_comments()? We'd simply need to rename the returned array keys.

Maybe. Unless people are using it to get "hot" data from the database. The cached counts might sometimes be wrong.

comment:6 in reply to: ↑ 5   nacin16 months ago

Replying to markjaquith:

Maybe. Unless people are using it to get "hot" data from the database. The cached counts might sometimes be wrong.

If getting hot data from the database is a valid use case, then we shouldn't deprecate it, even if the function isn't used by core. Otherwise, we should either ensure that the cache is invalidated, modify any cached data directly when wp_update_comment_count() is called, or decide that the non-persistence of the counts cache group is enough to ensure that the data is warm enough.

Or, as a fourth option, do what is proposed in the ticket description — keep both functions and have wp_count_comments() wrap and cache get_comments_count() (and have that function's SQL changed in #19901).

  • Milestone changed from 3.4 to Future Release
Note: See TracTickets for help on using tickets.