uc_shipping SQL query weakness

Project: 
Ubercart
Category: 
bug report
Version: 
Ubercart 1.0
Priority: 
minor
Assigned: 
Unassigned
Status: 
fixed

There is a query in uc_shipping that perhaps doesn't account for everything one would expect it to. It is in the function uc_shipping_order_packages().

The query is: "SELECT op.order_product_id, SUM(op.qty) AS total, SUM(pp.qty) AS packaged FROM {uc_order_products} AS op LEFT JOIN {uc_packaged_products} AS pp ON op.order_product_id = pp.order_product_id WHERE op.order_id = %d GROUP BY op.order_product_id HAVING SUM(pp.qty) IS NULL OR SUM(op.qty) > SUM(pp.qty)"

This query appears to determine if the "Create Packages" link should appear and in many cases does as it should and prevents it from showing up when all products in an order are packaged. I noticed, however, that when an order contains a product with a quantity other than 1 it doesn't work as expected. Consider this scenario to see problem. An order contains 2 of product X. If 2 packages are created, each containing one product X, the "create packages" button should disappear, as all items are packaged. It doesn't, however, because the aforementioned query's use of SUM(op.qty) results in a value double what it should, because it is being JOINED with the uc_packaged_products table in which there are two rows (packages) that correspond to the one product (product X). SUM(op.qty) then returns a value double what it should and in the comparison at the end of query it looks like all of the product is not packaged. I would rewrite this query for you, but I'm unsure how this can be corrected. Perhaps it can't be done with one query. Eitherway, it's not a huge issue and I hope I explained it properly.

Thanks.

Re: uc_shipping SQL query weakness

As much as we use databases to store data, it's really easy to forget that they will do basic math, as well. The basic problem is that the "total" column is too big by a factor equal to the number of packages. Therefore the solution is to divide "total" by the number of packages, which we can get with COUNT(pp.qty).

Here's the complete query:

<?php
  $result
= db_query("SELECT op.order_product_id, CAST(SUM(op.qty) / COUNT(pp.qty) AS UNSIGNED) AS total, SUM(pp.qty) AS packaged FROM {uc_order_products} AS op LEFT JOIN {uc_packaged_products} AS pp ON op.order_product_id = pp.order_product_id WHERE op.order_id = %d GROUP BY op.order_product_id HAVING SUM(pp.qty) IS NULL OR CAST(SUM(op.qty) / COUNT(pp.qty) AS UNSIGNED) > SUM(pp.qty)", $order_id);
?>

I put in the CAST() calls because dividing gives float values. I'm not sure exactly how important that is, but it makes me feel better. It may turn out to be more processing than needs to be done.

Re: Re: uc_shipping SQL query weakness

Thanks. That turned out to be more logical than I thought. As you said, its easy to forget about the math databases can do.