Download & Extend

taxonomy_get_synonym_root needs static caching.

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

Issue Summary

The not-so-often used taxonomy_get_synonym_root needs static caching. When it does get used, it suffers from duplicate queries.

<?php
function taxonomy_get_synonym_root($synonym) {
-  return
db_fetch_object(db_query("SELECT * FROM {term_synonym} s, {term_data} t WHERE t.tid = s.tid AND s.name = '%s'", $synonym));
+  static
$synonyms = array();
+  if (empty(
$synonyms[$synonym])) {
+   
$synonyms[$synonym] = db_fetch_object(db_query("SELECT * FROM {term_synonym} s, {term_data} t WHERE t.tid = s.tid AND s.name = '%s'", $synonym));
+  }
+  return
$synonyms[$synonym];
}
?>
AttachmentSizeStatusTest resultOperations
taxonomy-synonym-cache.patch950 bytesIgnored: Check issue status.NoneNone

Comments

#1

db_fetch_object returns a 0... so I cast it to an object so it gets cached. Other ideas are welcome.

<?php
function taxonomy_get_synonym_root($synonym) {
-  return
db_fetch_object(db_query("SELECT * FROM {term_synonym} s, {term_data} t WHERE t.tid = s.tid AND s.name = '%s'", $synonym));
+  static
$synonyms = array();
+  if (
$synonym == '') {
+    return
NULL;
+  }
+  if (empty(
$synonyms[$synonym])) {
+   
$synonyms[$synonym] = (object)db_fetch_object(db_query("SELECT * FROM {term_synonym} s, {term_data} t WHERE t.tid = s.tid AND s.name = '%s'", $synonym));
+  }
+  return
$synonyms[$synonym];
}
?>
AttachmentSizeStatusTest resultOperations
taxonomy-synonym-cache_0.patch1006 bytesIgnored: Check issue status.NoneNone

#2

This also still applies, with offset.

#3

There are no examples of this being used in contrib, but I ran into the caching issue with a client who has a proprietary module depending on this function. Rerolled patch to apply without offsets.

AttachmentSizeStatusTest resultOperations
taxonomy-synonym-cache_0.patch.txt958 bytesIgnored: Check issue status.NoneNone

#4

This still works against HEAD.

#5

Still applies with offset of 120 lines

#6

Status:needs review» needs work

Bumping to D7, +1, needs a re-roll for dbtng since db_fetch_object is deprecated.

#7

Status:needs work» needs review

Patch doesn't actually touch the query, so this could be done with a wider conversion. Attached patch adds a reset parameter and some doc.

AttachmentSizeStatusTest resultOperations
taxonomy-synonym-cache_0.patch.txt1.27 KBIgnored: Check issue status.NoneNone

#8

Version:6.x-dev» 7.x-dev

grr

#9

Fixed the documentation, converted the query to PDO, removed the ugly cast to object.

AttachmentSizeStatusTest resultOperations
162678-taxonomy-get-synonym-root-static.patch1.27 KBIgnored: Check issue status.NoneNone

#10

Status:needs review» postponed

Postponing for #254491: Standardize static caching to commit.

#11

Status:postponed» needs review

Please don't postpone little patches like this one in benefit of that kitten-killer. webchick groans about committing small patches, but in fact we know that she loves that.

#12

Status:needs review» reviewed & tested by the community

Still applies cleanly, changes are good. RTBC.

#13

Status:reviewed & tested by the community» fixed

Hm? I never groan about small patches. :D I groan about adding more $reset params to functions, but I agree that that other issue is going to take some time to sort out properly, and this function can benefit while we wait.

Committed with a couple small changes to the line spacing because I'm just that way. ;) Looking forward to seeing some or all of you in #254491: Standardize static caching so we can fix this properly.

#14

Status:fixed» closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.

nobody click here