Project:
UbercartCategory:
bug reportVersion:
Ubercart 1.5Priority:
normalAssigned:
UnassignedStatus:
activeAt line 265, this function:
<?php
function uc_product_kit_forms($saved_args) {
$forms = array();
if (substr($saved_args[0], 0, 31) == 'uc_product_kit_add_to_cart_form' || substr($saved_args[0], 0, 27) == 'uc_product_add_to_cart_form' || substr($saved_args[0], 0, 26) == 'uc_catalog_buy_it_now_form') {
$products = db_query("SELECT DISTINCT nid FROM {uc_product_kits} WHERE nid = %d", $saved_args[1]->nid);
while ($prodrow = db_fetch_object($products)) {
$forms['uc_product_kit_add_to_cart_form_'. $prodrow->nid] = array('callback' => 'uc_product_kit_add_to_cart_form');
$forms['uc_product_add_to_cart_form_'. $prodrow->nid] = array('callback' => 'uc_product_kit_add_to_cart_form');
$forms['uc_catalog_buy_it_now_form_'. $prodrow->nid] = array('callback' => 'uc_product_kit_buy_it_now_form');
}
}
return $forms;
}
?>This query, db_query("SELECT DISTINCT nid FROM {uc_product_kits} WHERE nid = %d", $saved_args[1]->nid); is unnecessary because all it's returning is a nid value that the function already has access to. And then, performing a db_fetch_object on it, returning the result a third unnecessary time.
I bring this up because this query gets made a LOT (as is shown in our slow_query.log file for mysql) and I'm wondering if it's in there for some reason that I'm missing? It seems rather redundant to have a nid value get passed to a function, and then to SELECT that same exact (distinct) nid from the table one more time. My suggestion is to remove the query and just act on the $saved_args[1]->nid element. Thoughts?
Re: Product Kits - unecessary query in uc_product_kit.module
Bleh. This is one of those places where I need to make sure I knew what I was thinking. I think the point of the query is to make sure that $save_args[1] really is a product kit node. But there's definitely an easier way to do that. The same issue would also come up on uc_product_forms().
I don't have an answer yet, but I'll come back to this.
Re: Re: Product Kits - unecessary query in uc_product_kit.module
Cool, thanks Lyle. I'd say it's just the placement of that query. For some reason that function gets called for every form, correct? It seems like the product_kit form function should only get called if the result returns true first... I'm not sure how much refactoring it'd take but it seems like that would cut down a lot of unnecessary queries...
I say this because it seems like the function is getting called on all of our products, which isn't a whole lot at the moment, but it seems to be noticeable enough by our db that it's logging it as a slow query (or a cacheable / redundant query?).. so if there is some kind of check on the node first, to make sure it's part of a product kit, then call the function this query resides in currently. I'm not sure if that refactoring is easy or not (you know the system better than I do) but I thought i'd suggest it.
EDIT: Also for a quick fix, do you think putting LIMIT 1 would be faster than a DISTINCT? I'm not sure if DISTINCT requires an examination of all rows in a way that LIMIT might not.. if there's even any difference there. Just a thought.