WordPress.org

Make WordPress Core

Opened 20 months ago

Last modified 7 months ago

#21760 assigned enhancement

get_term_by() calls are not cached — at Version 11

Reported by: wonderboymusic Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 2.3
Component: Taxonomy Keywords: has-patch needs-unit-tests
Focuses: Cc:

Description (last modified by wonderboymusic)

get_term() is the simplest way to retrieve one term, but it requires term_id and taxonomy. Because of this, terms are cached with term_id as key and $taxonomy as bucket. As a result, you can't easily grab a term by slug, unless you use get_term_by( 'slug' ). get_term_by( 'slug' ) and get_term_by( 'name' ) don't even have a query cache, so they go to the database every time. Because you can't get a term by slug without hitting the db, every place you want to get a term by slug: you first have to transform it into a term_id where it will then be cached. This is inefficient because the user may query by slug constantly and never by term_id.

Change History (13)

comment:1 nacin20 months ago

Cache keys are cast to strings. That means there would be a collision of the term_id of 1, the slug of '1', and the name of '1'. Even without the casting woes, slug and name would conflict.

For users, we handle this with separate cache buckets: users (by ID), userlogins, useremail, and userslugs.

This can just as easily be handled in the same bucket with prefixes - IDs are stored as is, login-$login, email-$email, slug-$slug. Or for terms, you'd have IDs, slug-$slug, name-$name.

update_term_cache() should set slug and name cache keys, in addition to ID, and that should then be leveraged by get_term_by() and friends.

comment:2 nacin20 months ago

See also #11531.

We should probably rename these buckets. Rather than $taxonomy and {$taxonomy}_relationships, we should use something that isn't valid in taxonomy/CPT name. taxonomy:$taxonomy and relationships:$taxonomy seem sufficient. These are the only buckets I know of that have a dynamic component to them.

comment:3 wonderboymusic20 months ago

Per chat with Nacin - gonna work on separate cache buckets for each type and will resurrect get_term_by() which can leverage them. Also, get_term() would have a hard time distinguishing between id, name, and slug for the category named, slugged, term_id'd "1" etc

comment:4 scribu20 months ago

  • Keywords needs-patch added; has-patch removed

comment:5 scribu20 months ago

  • Summary changed from Allow get_term() to accept string as first argument to get_term_by() calls are not cached

comment:6 dd3220 months ago

FWIW, by overloading get_term( $id ... to accept int|string|object, it brings up conditions where

  • Slug is numeric
  • a term_id is passed in, but return term_ids as strings in some locations, so you can't rely upon type being an int for term_ids

comment:7 wonderboymusic20 months ago

Attached a new patch:

1) get_term_by() calls are cached
2) term_cache_group() abstracts cache bucket for terms
3) update_term_cache() adds to cache of all 3 buckets
4) delete_term_cache() deletes cache of all 3 buckets
5) get_terms() calls update_term_cache(), deprecates the buggy list_terms_excusions in favor of get_terms filter
6) Lots of cleanup and streamlining of clean_term_cache() calls

comment:8 scribu20 months ago

  • Keywords has-patch needs-unit-tests added; needs-patch removed

This looks like an awesome patch, but it's tiring to read, due to all the whitespace fixes. I'll try again in the morning.

In related news, the test suite doesn't seem to have a single call to get_term_by( 'slug', ... );

comment:9 scribu20 months ago

Also, the patch seems to fail an existing test:

There was 1 failure:

1) Tests_Term::test_set_object_terms_by_name
Failed asserting that '1' matches expected 5.

/home/scribu/svn/wp-tests/tests/term.php:135

Finally, we get some use out of these darn things. :)

comment:10 wonderboymusic20 months ago

Updated the patch - the problem was a $wpdb->prepare thing in the IN query in clean_term_cache(). All tests pass now.

WITH ONE CAVEAT:
tests-posts-tax-cache-group.diff needs to be applied to Unit Tests so that the Post tests read from the proper term cache group

comment:11 wonderboymusic20 months ago

  • Description modified (diff)
Note: See TracTickets for help on using tickets.