Hi,

At the moment, tagadelic will run this query a NUMBER of times:

SELECT v.vid, v.*, n.type FROM vocabulary v LEFT JOIN vocabulary_node_types n ON v.vid = n.vid WHERE n.type = 'WHATEVER' ORDER BY v.weight, v.name

This is because the function tagadelic_node_get_terms($node) calls taxonomy_get_vocabularies, which generates a query each single time. In my home page, this means several extra queries in pages with several nodes.

This patch makes sure that tagadelic_node_get_terms($node) caches the result of taxonomy_get_vocabularies, therefore avoiding several useless DB calls.

Bye!

Merc.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mercmobily’s picture

Hi,

Apologies... I've just noticed this:

http://drupal.org/node/185017

Now... I have this patch ready, and it works. However, that other issue is marked as "postponed". Is there any particular reason not to accept this patch?

Bye,

Merc.

mercmobily’s picture

Hi,

Is anybody maintaining this module?
Is this patch likely to make it?

Bye,

Merc.

Bèr Kessels’s picture

Status: Needs review » Needs work

why are you not reusing the variable $vocs?

mercmobily’s picture

Status: Needs work » Needs review

Hi,

I do not understand your question.
The patch brings minimal disruption to the module's logic. Basically, rather than having:

$vocs = taxonomy_get_vocabularies($node->type);

You check that you didn't make that call already:

$vocs = taxonomy_get_vocabularies($node->type);

+ if(!isset($vocabulary[$node->type])){
+ $vocabulary[$node->type]=taxonomy_get_vocabularies($node->type);
+ }
+ $vocs = $vocabulary[$node->type];

The variable $voc is being "used" just the same as before, anda new variable - the cache, $vocabulary - is being added.

If you ask me, this patch makes sense and it's being used on a production server right now.

Bye,

Merc.

mercmobily’s picture

...?

Bèr Kessels’s picture

The code still seems odd to me. I really don't see the advantage of adding extra variables that contain exactly the original content, for the sake of "less disruption". Lets focus on good code. Not on explaining history --codewise--

mercmobily’s picture

Hi

I proposed a solution to a tangible problem of the code.
You didn't like the solution I proposed, which is fair enough.

I can't think of a better way of doing it. In fact, I think the code I wrote is exactly how it should be done, disruprion or not-disruption.

Right now the module repeats queries, which is a problem. If you don't like my solution (I don't see why not, but you must have some reasons), the only solution is for you to think of a better way to fix the module's problem, which is tangible and "
there".

Bye,

Merc.

mercmobily’s picture

Hi,

Any news for a better solution to this bug?

Merc.

Jo Wouters’s picture

Version: 5.x-1.x-dev » 5.x-1.0-rc1
FileSize
1.06 KB

This patch reuses $vocs

Bèr Kessels’s picture

Did anyone test this on the RC1?
If not, I will release 5.x-1.0 first and then include this patch in the new HEAD. Making it a feature for the 5.x-1.1 (or, in case it comes earlier, the 6.x-1.0) release.

neochief’s picture

Status: Needs review » Reviewed & tested by the community

Patched my module with this patch and all seems to be fine. Static caching rocks :)

wmostrey’s picture

Version: 5.x-1.0-rc1 » 5.x-1.0

I can confirm that this patch applies cleanly on 5.x-1.0. For us this made the difference between patching the module and keeping it enabled on a high traffic site, or disabling it.

mrfelton’s picture

+1. This vastly reduces the number of duplicate queries from about 150 to 8 in my case. Patch at #9 ported to Drupal 6.

Bèr Kessels’s picture

Status: Reviewed & tested by the community » Fixed

committed to 6, HEAD (to be 7) and closing. Thanks

Status: Fixed » Closed (fixed)

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