Posted by robertDouglass on July 29, 2007 at 8:57pm
| Project: | Drupal core |
| Version: | 6.x-dev |
| Component: | taxonomy.module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | reviewed & tested by the community |
Issue Summary
This is an API function but if you use it you risk duplicating queries + processing. This patch adds static caching.
<?php
function taxonomy_get_vocabularies($type = NULL) {
+ static $vocabularies_cache = array();
+ static $vocabularies_null;
+
if ($type) {
+ if (is_array($vocabularies_cache[$type])) {
+ return $vocabularies_cache[$type];
+ }
$result = db_query(db_rewrite_sql("SELECT v.vid, v.*, n.type FROM {vocabulary} v LEFT JOIN {vocabulary_node_types} n ON v.vid = n.vid WHERE n.type = '%s' ORDER BY v.weight, v.name", 'v', 'vid'), $type);
}
else {
+ if (is_array($vocabularies_null)) {
+ return $vocabularies_null;
+ }
$result = db_query(db_rewrite_sql('SELECT v.*, n.type FROM {vocabulary} v LEFT JOIN {vocabulary_node_types} n ON v.vid = n.vid ORDER BY v.weight, v.name', 'v', 'vid'));
}
@@ -695,7 +704,15 @@ function taxonomy_get_vocabularies($type
$vocabularies[$voc->vid] = $voc;
}
+ if ($type) {
+ $vocabularies_cache[$type] = $vocabularies;
+ }
+ else {
+ $vocabularies_null = $vocabularies;
+ }
+
return $vocabularies;
+
}
?>| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| taxonomy_get_vocabularies_cache.patch | 1.48 KB | Idle | FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch taxonomy_get_vocabularies_cache.patch. | View details | Re-test |
Comments
#1
I looked through this patch and everything looks good. I agree this should be cached. The only thing I wasn't sure of was the variable name $vocabularies_null, which as I understand it, gets set if a type isn't specified.
What about $vocabularies_all or something like that? Maybe someone else with a weightier opinion can give an opinion and change the patch code status.
Attaching a patch rolled against latest HEAD without offsets.
#2
patch still applies with offset...
#3
Is there a or two modules out thee that calls this function repeatedly? Just want some justification for the cache.
#4
grep -R taxonomy_get_vocabularies . | wc -l244
that's on the D5 contrib repository. Rerolled patch to apply without offsets.
#5
Still valid in D7.
We should go the whole hog and change this to taxonomy_vocabulary_load_multiple() so that hook_taxonomy_vocabulary_load() works with it and we can share the static between taxonomy_get_vocabulary() too.
#6
taxonomy_vocabulary_load_multiple() is in.
#7
This no longer applies to Drupal 7, but it's still valid for D6.
#8
The last submitted patch, taxonomy_get_vocabularies_cache2.patch, failed testing.
#9
Here's a re-roll that also turns taxonomy_vocabulary_load() into a wrapper around taxonomy_get_vocabularies() - this means more vocabularies loaded if you were to only try to load one, but that is not often the case.
#10
Fixed a notice in taxonomy_get_vocabularies().
#11
Patch is good. It saves over 150 queries on loading the home page of a site I am working on.
However I don't think its a good idea to change taxonomy_vocabulary_load() because it has a reset flag. Assuming it is used somewhere, every time you call the function with $reset = TRUE, it will just grab the cached taxonomies from taxonomy_get_vocabularies().