[wp-trac] [WordPress Trac] #37094: wp_get_nav_menu_items should use tax_query

[WordPress Trac]

#37094: wp_get_nav_menu_items should use tax_query
-----------------------------------+--------------------------
 Reporter:  spacedmonkey           |       Owner:
     Type:  enhancement            |      Status:  new
 Priority:  normal                 |   Milestone:  4.9
Component:  Menus                  |     Version:  3.0
 Severity:  normal                 |  Resolution:
 Keywords:  has-patch 2nd-opinion  |     Focuses:  performance
-----------------------------------+--------------------------
Changes (by boonebgorges):

 * keywords:  has-patch needs-unit-tests => has-patch 2nd-opinion
 * milestone:  Awaiting Review => 4.9


Comment:

 Replying to [comment:4 spacedmonkey]:
 > @ocean90 I didn't manage to replicate your error message. I don't see
 how there is a problem, tax_query is a well tested part of core. The only
 reason it didn't use tax_query when this function was created, was because
 it didn't exists.

 What @ocean90 is seeing is not errors, but the results of `EXPLAIN`, which
 indicate that a move to `get_posts()` forces `Using temporary`. This is
 bad, and gets increasingly bad as the site of your posts and
 term_relationships tables get bigger. The main ID query in `get_posts()`
 often has to use temporary tables when `tax_query` is used, mostly because
 `GROUP BY` is needed to ensure uniqueness (the same post could match more
 than one of the terms). There is no easy way to fix this, and given that
 this part of `WP_Query` has no caching at all, I'm not convinced that
 there's a benefit to switching to `get_posts()` here; there may in fact be
 a loss in performance.

 That being said, it is a good idea to improve performance here. How about
 adding caching to `get_objects_in_term()`? [attachment:37094.diff] shows
 how this can be done. This actually has bigger impact on query reduction
 than the original patch, since `get_objects_in_term()` is used more
 broadly, while `get_posts()` always requires at least one query. Tests are
 also attached that show invalidation when: (a) term assignment changes,
 (b) a term is deleted, (c) a post is deleted. Thoughts?

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