[wp-trac] [WordPress Trac] #35381: Introduce `WP_Term_Query`

[WordPress Trac]

#35381: Introduce `WP_Term_Query`
-------------------------------------------------+-------------------------
 Reporter:  boonebgorges                         |       Owner:
     Type:  defect (bug)                         |      Status:  new
 Priority:  normal                               |   Milestone:  Future
Component:  Taxonomy                             |  Release
 Severity:  normal                               |     Version:
 Keywords:  has-patch needs-testing dev-         |  Resolution:
  feedback                                       |     Focuses:
-------------------------------------------------+-------------------------

Comment (by boonebgorges):

 @spacedmonkey I agree that it makes sense to move toward centralizing
 term-related queries, yes. Let's not do that in this specific ticket, but
 let's keep it in mind to make sure that the architectural decisions made
 here don't preclude the kind of improvements you're talking about.

 @flixos90 This is really excellent. Thanks for working on it.

 Running `$ phpunit --group taxonomy`, a bunch of stuff is failing. Most of
 the notices appear to be due to some variable names that weren't changed
 properly, but there also appears to be a bunch of tests related to
 hierarchical terms that are related to each other. I haven't had a chance
 to debug this in detail.

 Most of your improvements sound worth considering, but I'm concerned about
 doing them all at once, especially in the absence of unit tests that
 describe the issues:

 > I also added a $suppress_filters argument which should probably (when
 active) disable more filters than it currently does in this
 implementation.

 The only place where we currently have this feature is in `WP_Query`. I
 think we need a separate ticket to discuss whether it's a feature that we
 need here, and if so, how it should behave.

 > I found a bug that is possible to happen even in current versions of
 WordPress

 This is pretty complicated, and I'm very wary of touching it without
 tests. I suggest we fix it separately, either before or after
 `WP_Term_Query`. I wonder whether your fix is related to the unit test
 failures we're seeing. (It sounds like the fix might be straightforward
 enough to do *before* and then include in a refreshed patch here, but I'd
 like to hear what you think about it.)

 > Another problem in my opinion is the get_terms_fields filter. Since this
 class always queries for IDs (or a count), using the get_terms_fields
 filter to modify the requested fields in any way will most probably mess
 up the query

 Yeah. I'm not a fan of this filter, or 'fields' arguments in general,
 since they tend to result in cache misses. I would imagine that most
 people using the filter are doing it to remove fields from the `SELECT`
 statement, in which case there are no real compatibility concerns.
 However, it's possible that people are using it to *add* fields, possibly
 related to another table (which would be joined in another filter like
 'terms_clauses'). This needs research before we make a decision. A search
 through the plugin repo is probably a good place to start, as is some
 reading on the backstory behind 'get_terms_fields' and friends. See #9004.

 @flixos90 If you're able to debug the unit test issues and perhaps address
 some of the items above, that would be great. This ticket is something I
 would like to prioritize for 4.6, and I'll be able to dedicate time to
 refining the patch, but not until next week.

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