[wp-trac] [WordPress Trac] #35192: Comments_clauses filter

WordPress Trac noreply at wordpress.org
Sat Jan 9 03:20:01 UTC 2016


#35192: Comments_clauses filter
--------------------------+--------------------
 Reporter:  firebird75    |       Owner:
     Type:  defect (bug)  |      Status:  new
 Priority:  normal        |   Milestone:  4.4.2
Component:  Comments      |     Version:  4.4
 Severity:  normal        |  Resolution:
 Keywords:                |     Focuses:
--------------------------+--------------------
Changes (by boonebgorges):

 * keywords:  2nd-opinion =>


Comment:

 Thanks for the feedback, @firebird75 .

 > My suggestion would be to have one and only one query for comments
 display

 This is the crux of the issue. It used to be that a post's comments were
 always fetched with a single query. But this meant fetching all of a
 post's comments on every comment-page, which could mean loading thousands
 of comment objects just to display 20 or 50 of them. While it's true that
 the overhauled logic in 4.4 sometimes results in a larger number of
 queries, they are now far more performant and far more cacheable than they
 previously were. See #8071 for a very extensive backstory.

 An additional filter in `fill_descendants()` seems to me like the wrong
 way to target this problem. The vast majority of the time, the parameters
 used to fetch top-level comments should also be used to fetch the non-top-
 level comments. Thus we should only require plugins to filter the
 parameters in one place.

 The "ideal" solution would be for us to pass an array of 'where' sub-
 clauses to the 'comments_clauses' filter, rather than the imploded clause.
 But this is a non-starter for backward compatibility reasons.

 So, while I'm not a huge fan of the string manipulation option I've
 described here, I think it's the only way to ensure compatibility with
 existing uses of the filter, without requiring an unreasonable amount of
 work from plugin authors. And I don't see how waiting until 4.5 is going
 to change anything: as I noted earlier, a move back to a single query is
 not in the cards. I'll plug away and try to improve the implementation
 suggested in [attachment:35192.diff].

--
Ticket URL: <https://core.trac.wordpress.org/ticket/35192#comment:4>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform


More information about the wp-trac mailing list