[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