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

 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];
 }
CommentFileSizeAuthor
#9 162678-taxonomy-get-synonym-root-static.patch1.27 KBdamien tournoud
#7 taxonomy-synonym-cache_0.patch.txt1.27 KBcatch
#3 taxonomy-synonym-cache_0.patch.txt958 bytesrobertdouglass
#1 taxonomy-synonym-cache_0.patch1006 bytesrobertdouglass
taxonomy-synonym-cache.patch950 bytesrobertdouglass

Comments

robertdouglass’s picture

robertdouglass commented
StatusFileSize
new taxonomy-synonym-cache_0.patch1006 bytes

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

 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];
 }
catch’s picture

catch
he/him
Primary language English
commented

This also still applies, with offset.

robertdouglass’s picture

robertdouglass commented
StatusFileSize
new taxonomy-synonym-cache_0.patch.txt958 bytes

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.

birdmanx35’s picture

birdmanx35 commented

This still works against HEAD.

drawk’s picture

drawk commented

Still applies with offset of 120 lines

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Needs work

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

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Needs review
StatusFileSize
new taxonomy-synonym-cache_0.patch.txt1.27 KB

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.

catch’s picture

catch
he/him
Primary language English
commented
Version: 6.x-dev » 7.x-dev

grr

damien tournoud’s picture

damien tournoud commented
StatusFileSize
new 162678-taxonomy-get-synonym-root-static.patch1.27 KB

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

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs review » Postponed

Postponing for #254491: Standardize static caching to commit.

damien tournoud’s picture

damien tournoud commented
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.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Still applies cleanly, changes are good. RTBC.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
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.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

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