Ambiguous SQL query during shipment [#742664]

The following query fails under PostgreSQL:


query: SELECT op.order_product_id, pp.qty, pp.qty * op.weight AS weight,  
p.weight_units, op.nid, op.title, op.model, op.price, op.data FROM  
uc_packaged_products AS pp LEFT JOIN uc_order_products AS op ON  
op.order_product_id = pp.order_product_id LEFT JOIN uc_products AS p ON  
op.nid = p.nid WHERE pp.package_id = 8 GROUP BY op.order_product_id

in
/home/html/gooze.eu/www/sites/all/modules/ubercart/shipping/uc_shipping/uc_shipping.module
on line 396.

Read MySQL/PostgreSQL for explainations:http://drupal.org/node/555514
You cannot GROUP BY on one single column:http://drupal.org/node/555530

But when looking at PHP code, it seems that we are calculating weight.

385 function uc_shipping_package_load($package_id) {
386   static $packages = array();
387 
388   if (!isset($packages[$package_id])) {
389     $result = db_query("SELECT * FROM {uc_packages} WHERE package_id = %d", $package_id);
390     if ($package = db_fetch_object($result)) {
391       $products = array();
392       $descripion = '';
393       $weight = 0;
394       $units = variable_get('uc_weight_unit', 'lb');
395       $addresses = array();
396       $result = db_query("SELECT op.order_product_id, pp.qty, pp.qty * op.weight AS weight, p.weight_units, op.nid, op.title, op.model, op.price, op.data     FROM {uc_packaged_products} AS pp LEFT JOIN {uc_order_products} AS op ON op.order_product_id = pp.order_product_id LEFT JOIN {uc_products} AS p ON op.ni    d = p.nid WHERE pp.package_id = %d GROUP BY op.order_product_id", $package_id);
397       while ($product = db_fetch_object($result)) {
398         $address = uc_quote_get_default_shipping_address($product->nid);
399         // TODO: Lodge complaint that array_unique() compares as strings.
400         if (!in_array($address, $addresses)) {
401           $addresses[] = $address;
402         }
403         $description .= ', '. $product->qty .' x '. $product->model;
404         // Normalize all weights to default units.
405         $weight += $product->weight * uc_weight_conversion($product->weight_units, $units);
406         $product->data = unserialize($product->data);
407         $products[$product->order_product_id] = $product;
408       }
409       $package->addresses = $addresses;
410       $package->description = substr($description, 2);
411       $package->weight = $weight;
412       $package->weight_units = $units;
413       $package->products = $products;
414       $packages[$package_id] = $package;
415     }
416     else {
417       return FALSE;
418     }
419   }

Let's take the example of a parcel containing two similar products A.
In MySQL, if a GROUP BY is encountered, it may choose an arbitrary record.
So this query seems to be ambiguous.

MySQL wron't complain, PostgreSQL does complain.

Did you mean ORDER BY instead of GROUP BY?

Please note that I may be wrong.
What is your opinion?

Kind regards,
Jean-Michel

Comment	File	Size	Author
#7	patch.diff	1.28 KB	grub3
#7

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

TR CreditAttribution: TR commented 15 March 2010 at 15:33

Yes, this looks like a bug - it works properly for MySQL but not for PostgreSQL, as explained in http://drupal.org/node/555530 . It will take some research to figure out how to rewrite the query so it works for both.

Comment #2

grub3 CreditAttribution: grub3 commented 15 March 2010 at 16:02

Assigned:

grub3

» Unassigned

If you explain what you are trying to do, I can rewrite the query for you.
The GROUP BY will fail to produce good results, even on MySQL.

What were you looking for? DISTINCT results ? ORDER results?
Just explain a little bit more and I rewrite an SQL 99 query for you.

Comment #3

grub3 CreditAttribution: grub3 commented 15 March 2010 at 16:01

Assigned:

Unassigned

» grub3

Comment #4

TR CreditAttribution: TR commented 15 March 2010 at 16:23

Assigned:

Unassigned

» grub3

I didn't write the code, so I'm not exactly sure what the original intent was. That's why I said I would have to do research, by looking into the tables. I'm *guessing* that, since the purpose of the function is to group products into packages based on the ship-from address stored in the product object, GROUP BY was intended to select only one of each product nid, even if multiple products with that nid have been ordered. You only need to examine the ship-from address from one instance of the product, it doesn't matter which one because all products with that nid will have the same ship-from address.

Comment #5

Island Usurper CreditAttribution: Island Usurper commented 19 March 2010 at 16:07

Yeah, I think I expected it to work as a more efficient DISTINCT. The joins that are done can give duplicate rows for a particular product in the package, but I only want to count the weight and address once. I think the row duplication comes from having multiple versions of the product node. I didn't want to have to do a HAVING MAX(p.vid) or join to {node} to restrict the results to one particular version, but maybe that's what needs to be done.

Suggestions are welcome.

Comment #6

grub3 CreditAttribution: grub3 commented 27 May 2010 at 14:21

I think this should simply work:

SELECT op.order_product_id, pp.qty, pp.qty * op.weight AS weight, 
p.weight_units, op.nid, op.title, op.model, op.price, op.data FROM 
uc_packaged_products AS pp 

LEFT JOIN uc_order_products AS op ON 
op.order_product_id = pp.order_product_id 

LEFT JOIN uc_products AS p ON 
op.nid = p.nid WHERE pp.package_id = 62 

ORDER BY op.order_product_id

There is no need to group by.

Comment #7

grub3 CreditAttribution: grub3 commented 27 May 2010 at 14:28

Status:

Active

» Needs review

File	Size
patch.diff	1.28 KB

Atttached patch. This works fine under PostgreSQL and on my store (reviewed 10 orders and ran SQL commands).

Comment #8

grub3 CreditAttribution: grub3 commented 25 June 2010 at 15:50

Any news. Can you commit to repository quickly. My log is full of errors and I cannot handle this any longuer.

Comment #9

grub3 CreditAttribution: grub3 commented 25 June 2010 at 15:50

Priority:

Normal

» Critical

Any news. Can you commit to repository quickly. My log is full of errors and I cannot handle this any longuer.

Comment #10

Island Usurper CreditAttribution: Island Usurper commented 7 July 2010 at 19:01

Status:

Needs review

» Fixed

OK, I just had to remember what I was thinking when that was written. I believe I was worried about having rows with the same order_product_id, which would cause some weird issues in package operations. But since there's the WHERE clause selecting one specific package_id, I don't think we have to worry about it. order_product_id should always be distinct on that query, so ORDER BY is all we need.

Thanks, and committed.

Comment #11

21 July 2010 at 19:10

Status:

Fixed

» Closed (fixed)

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

Comment #12

grub3 CreditAttribution: grub3 commented 14 August 2010 at 15:41

Thanks.

Comment #13

joep.hendrix CreditAttribution: joep.hendrix commented 4 November 2010 at 21:55

Status:

Closed (fixed)

» Active

I am not sure whether to re-open this issue or create a new one.
The problem is that this query is not correct when revisions are enabled on the product node type.
You will get a result for each revision. I guess the join on uc_products should be on the vid instead of the nid. Hower, it will then not show the actual product revision model!

SELECT op.order_product_id                           
,      pp.qty                                        
,      pp.qty * op.weight AS weight                  
,      p.weight_units                                
,      op.nid                                        
,      op.title                                      
,      op.model                                      
,      op.price                                      
,      op.data                                       
FROM uc_packaged_products AS pp                      
LEFT JOIN uc_order_products AS op                    
  ON op.order_product_id = pp.order_product_id       
LEFT JOIN uc_products AS p                           
  ON op.nid = p.vid                                  
WHERE pp.package_id = 36 ORDER BY op.order_product_id