Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#35431 closed defect (bug) (maybelater)

Hierarchical comment queries do not work for interrupted hierarchies

Reported by: boonebgorges Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.4
Component: Comments Keywords:
Focuses: Cc:


Previously: #35192

Since 4.4, WP_Comment_Query has supported a hierarchical method of querying for comments. Notably, the hierarchical strategy is used in the comments_template() chain. This strategy makes comment queries far more efficient than the previous technique of querying for all of a post's comments, and then letting Walker_Comment handle threading and pagination. See #8071.

A side effect of the hierarchical query strategy is that the grandchild C of a comment A will not be returned if the intermediate comment B is not included in the query results for some reason (say, comment__not_in=B). This is a change in behavior from 4.3 and earlier, at least in the case of comments_template(). (Direct 'hierarchical' queries were not available before 4.4.)

So the question is: Is this a bug? If I have a comment hierarchy like this:


and I query comments like this:

$q = new WP_Comment_Query( array(
    'hierarchical' => 'flat',
    'comment__not_in' => array( $b ),
) );

would you expect to get C? Perhaps not by default, but should there be a way to force interrupted hierarchies to be traversed?

Feedback welcome.

Change History (4)

#1 @needle
5 years ago

Perhaps this calls for an extra param that allows either traversal method to be used? Something that defines "skip children if parent excluded"?

#2 @firebird75
5 years ago

@needle +1, this would be a great idea. Maybe just a parameter that can be set via the filter in comments_clauses.

#3 @swissspidy
5 years ago

  • Component changed from General to Comments

#4 @boonebgorges
5 years ago

  • Keywords 2nd-opinion removed
  • Milestone Awaiting Review deleted
  • Resolution set to maybelater
  • Status changed from new to closed

Implementation here is going to be tricky. We don't want to force lots of additional cache-missing queries. We'll need a patch, or at least a description of an approach, before proceeding. If anyone wants to champion the change, please reopen.

Note: See TracTickets for help on using tickets.