Skip to content

Commit

Permalink
redundant product_attribute queries
Browse files Browse the repository at this point in the history
- when adding product there is no need to remove any attributes belonging to it
- when updating the product the attributes related to the product are already cleared with
```php
$this->db->query("DELETE FROM " . DB_PREFIX . "product_attribute WHERE product_id = '" . (int)$product_id . "'");
```
no need to issue more specific deletion queries inside the loop
```php
$this->db->query("DELETE FROM " . DB_PREFIX . "product_attribute WHERE product_id = '" . (int)$product_id . "' AND attribute_id = '" . (int)$product_attribute['attribute_id'] . "'");
``
  • Loading branch information
maks feltrin committed Feb 13, 2015
1 parent 45fc863 commit e8f4700
Showing 1 changed file with 1 addition and 5 deletions.
6 changes: 1 addition & 5 deletions upload/admin/model/catalog/product.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ public function addProduct($data) {
if (isset($data['product_attribute'])) {
foreach ($data['product_attribute'] as $product_attribute) {
if ($product_attribute['attribute_id']) {
$this->db->query("DELETE FROM " . DB_PREFIX . "product_attribute WHERE product_id = '" . (int)$product_id . "' AND attribute_id = '" . (int)$product_attribute['attribute_id'] . "'");

foreach ($product_attribute['product_attribute_description'] as $language_id => $product_attribute_description) {
$this->db->query("INSERT INTO " . DB_PREFIX . "product_attribute SET product_id = '" . (int)$product_id . "', attribute_id = '" . (int)$product_attribute['attribute_id'] . "', language_id = '" . (int)$language_id . "', text = '" . $this->db->escape($product_attribute_description['text']) . "'");
}
Expand Down Expand Up @@ -153,8 +151,6 @@ public function editProduct($product_id, $data) {
if (!empty($data['product_attribute'])) {
foreach ($data['product_attribute'] as $product_attribute) {
if ($product_attribute['attribute_id']) {
$this->db->query("DELETE FROM " . DB_PREFIX . "product_attribute WHERE product_id = '" . (int)$product_id . "' AND attribute_id = '" . (int)$product_attribute['attribute_id'] . "'");

foreach ($product_attribute['product_attribute_description'] as $language_id => $product_attribute_description) {
$this->db->query("INSERT INTO " . DB_PREFIX . "product_attribute SET product_id = '" . (int)$product_id . "', attribute_id = '" . (int)$product_attribute['attribute_id'] . "', language_id = '" . (int)$language_id . "', text = '" . $this->db->escape($product_attribute_description['text']) . "'");
}
Expand Down Expand Up @@ -692,4 +688,4 @@ public function getTotalProductsByLayoutId($layout_id) {

return $query->row['total'];
}
}
}

4 comments on commit e8f4700

@eka7a
Copy link
Collaborator

@eka7a eka7a commented on e8f4700 Apr 11, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the same attribute twice selected

This change errors Error: Duplicate entry '62-19-0' for key 'PRIMARY'

@pine3ree
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HI @eka7a ,

in those cases it would still be better to cleanup post data for duplicate ids than doing that with db queries.

@pine3ree
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eka7a

i added this PR #2902
please download and give it a try
regards,
maks

@quincywu
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,
I know it might not be necessary or it might degrade the performance of the code, but wasn't the original idea of this piece of code

$this->db->query("DELETE FROM " . DB_PREFIX . "product_attribute WHERE product_id = '" . (int)$product_id . "' AND attribute_id = '" . (int)$product_attribute['attribute_id'] . "'");

is to prevent possible error to occur ? It can even correct the mistake made by developers. I think the idea was sanitary check anyway.

Please sign in to comment.