Download & Extend

Taxonomy doing unnecessary queries because of not sharing static

Project:Drupal core
Version:7.x-dev
Component:taxonomy.module
Category:bug report
Priority:normal
Assigned:Unassigned
Status:closed (duplicate)

Issue Summary

In taxonomy you can get terms on two ways (there are some more):

function taxonomy_get_term($tid) {
  static $terms = array();

  if (!isset($terms[$tid])) {
    $terms[$tid] = db_fetch_object(db_query('SELECT * FROM {term_data} WHERE tid = %d', $tid));
  }

  return $terms[$tid];
}

function taxonomy_node_get_terms($node, $key = 'tid') {
  static $terms;

  if (!isset($terms[$node->vid][$key])) {
    $result = db_query(db_rewrite_sql('SELECT t.* FROM {term_node} r INNER JOIN {term_data} t ON r.tid = t.tid INNER JOIN {vocabulary} v ON t.vid = v.vid WHERE r.vid = %d ORDER BY v.weight, t.weight, t.name', 't', 'tid'), $node->vid);
    $terms[$node->vid][$key] = array();
    while ($term = db_fetch_object($result)) {
      $terms[$node->vid][$key][$term->$key] = $term;
    }
  }
  return $terms[$node->vid][$key];
}

Problem with this is the two seperated static vars. Default for node's is the load from nodeapi with 'taxonomy_node_get_terms()'. In other modules the data for taxonomy is loaded with: 'taxonomy_get_term()'. For example the forum module uses it's main content of taxonomy from the nodeapi loaded data. However in the 'link_alter':

function forum_link_alter(&$links, $node) {
  foreach ($links as $module => $link) {
    if (strstr($module, 'taxonomy_term')) {
      // Link back to the forum and not the taxonomy term page. We'll only
      // do this if the taxonomy term in question belongs to forums.
      $tid = str_replace('taxonomy/term/', '', $link['href']);
      $vid = variable_get('forum_nav_vocabulary', '');
      $term = taxonomy_get_term($tid);
      if ($term->vid == $vid) {
        $links[$module]['href'] = str_replace('taxonomy/term', 'forum', $link['href']);
      }
    }
  }
}

It gets the terms with taxonomy_get_term. Question is or you have to change this in forum module or in taxonomy. I think the best way to go is in taxonomy because more modules will do this.... the following function is added with this patch:

function taxonomy_set_term($tid, $term = null, $reset = null) {
  static $terms = array();
 
  if ($reset == true) {
    $terms = true;
  }
 
  if (!isset($terms[$tid]) && isset($term)) {
    $terms[$tid] = $term;
  }
 
  if (isset($terms[$tid])) {
    return $terms[$tid];
  }
 
  return false;
}

taxonomy_get_term changes to:

function taxonomy_get_term($tid) { 
  if (!$term = taxonomy_set_term($tid)) {
    $term = taxonomy_set_term($tid, db_fetch_object(db_query('SELECT * FROM {term_data} WHERE tid = %d', $tid)));
  }
 
  return $term;
}

and to taxonomy_node_get_terms the taxonomy_set_term is added:

function taxonomy_node_get_terms($node, $key = 'tid') {
  static $terms;

  if (!isset($terms[$node->vid][$key])) {
    $result = db_query(db_rewrite_sql('SELECT t.* FROM {term_node} r INNER JOIN {term_data} t ON r.tid = t.tid INNER JOIN {vocabulary} v ON t.vid = v.vid WHERE r.vid = %d ORDER BY v.weight, t.weight, t.name', 't', 'tid'), $node->vid);
    $terms[$node->vid][$key] = array();
    while ($term = db_fetch_object($result)) {
      $terms[$node->vid][$key][$term->$key] = $term;
      taxonomy_set_term($term->tid, $term);
    }
  }
  return $terms[$node->vid][$key];
}

Comments

#1

Forgot the patch..

AttachmentSizeStatusTest resultOperations
taxonomy_set_term.patch1.9 KBIgnored: Check issue status.NoneNone

#2

Status:needs review» needs work

This is a great idea. The lack of a central storage mechanism for taxonomy terms can be quite wasteful.

There are a few code style issues with this patch though:
- true, false, and null should all be capitalized.
- There's some extra white space after the function taxonomy_get_term($tid) { line.

Looks great though!

#3

subscribing....

#4

The reason I posted this patch was the amount of taxonomy_get_terms done after generating nodes on a clean drupal 7 dev install with all core modules enabled. The patch is a quick fix for that problem.

Now I'm looking deeper into taxonomy and find alot of more places where data should be shared. I'll be looking more into this the next few days.

#5

Looks great! Thanks! Subscribing.

#6

I am still planning to work on this. I'm waiting for the Static Cache Facility though. Would be quite useless to fix this and have to rewrite it after the static cache is comitted.

#7

Status:needs work» postponed

Let's mark this as postponed pending the static cache facility. Looks great :)

#8

Subscribing.

#9

Minor, but when you work on this again, comment lines should consistently end in full stops (periods).

#10

Postponed still, but with a few more periods at the end of comments, etc.

AttachmentSizeStatusTest resultOperations
taxonomy_set_term_2.patch2.16 KBIgnored: Check issue status.NoneNone

#11

Title:Taxonomy doing unnesaccary queries because of not sharing static» Taxonomy doing unnecessary queries because of not sharing static

(Correcting title.)

#12

Status:postponed» closed (duplicate)

This has been fixed by the introduction of taxonomy_term_load_multiple() in #324313: Load multiple nodes and terms at once.