Skip to content

Fixed getAttributeRawValue when there are no value in the "default" store #2964

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Apr 18, 2024
Merged

Fixed getAttributeRawValue when there are no value in the "default" store #2964

merged 6 commits into from
Apr 18, 2024

Conversation

luigifab
Copy link
Contributor

@luigifab luigifab commented Jan 17, 2023

Description

Reworked query in getAttributeRawValue so that it returns a store specific value even if there is no default value.

See also #23369 from Magento2.
I tested this PR with #572, and with PHP 8.0 / 8.1 / 8.2.

Manual testing scenarios

  • Create a product
  • Create a new attribute, for example with bonjour code and store view scope
  • Update the value of the attribute on the previous product at store view level (not at default/global scope)
  • Update app/design/frontend/rwd/default/template/catalog/product/view.phtml, add:
<h1>{ <?php
 echo $_product->getResource()->getAttributeRawValue($_product->getId(), 'bonjour', $_product->getStoreId())
?> }</h1>

Then go to the product page, without the PR, your value is out there, with the PR, the value is displayed.

If you use now the same attribute in checkout/cart/item/default.phtml with:

<config>
 <global>
  <sales>
   <quote>
    <item>
     <product_attributes>
      <bonjour />
     </product_attributes>
    </item>
   </quote>
  </sales>
 </global>
</config>

and:

<h2>{ <?php echo $this->getItem()->getProduct()->getData('bonjour') ?> }</h2>

And now it works!

SQL queries

The order of UNIQUE index is: entity_id, attribute_id, store_id
in catalog_product_entity_varchar...

the data

SELECT * FROM `catalog_product_entity_varchar`
WHERE `entity_id` = 1018
 AND `attribute_id` IN (87,1286,1287,1289,1291,1293,1294,1556)
 AND `store_id` IN (0,1)
ORDER BY `attribute_id`, `store_id`;

value_id  entity_type_id  attribute_id   store_id  entity_id  value
2592440   4               87             0         1018       default
2592443   4               87             1         1018       store
2579585   4               1286           0         1018       NULL
2592797   4               1286           1         1018       NULL
2579586   4               1287           0         1018       default
2579695   4               1287           1         1018       NULL
2579588   4               1289           0         1018       NULL
2579697   4               1289           1         1018       store
2579589   4               1291           0         1018       NULL
2579591   4               1293           1         1018       NULL
2579700   4               1294           1         1018       store_alone
2592444   4               1556           0         1018       detault_alone

getAttributeRawValue

-- before default ----------------------

SELECT
 `default_value`.`attribute_id`,
 `default_value`.`value` AS `attr_value`

FROM `catalog_product_entity_varchar` AS `default_value`

WHERE
 (default_value.attribute_id IN (87, 1286, 1287, 1289, 1291, 1293, 1294, 1556))
 AND (default_value.entity_type_id = 4)
 AND (default_value.entity_id = 1018)
 AND (default_value.store_id = 0);

-- after default ------ ----------------

SELECT
 `default_value`.`attribute_id`,
 `default_value`.`value` AS `attr_value`

FROM `catalog_product_entity_varchar` AS `default_value`

WHERE
 (default_value.entity_id = 1018)
 AND (default_value.attribute_id IN (87, 1286, 1287, 1289, 1291, 1293, 1294, 1556))
 AND (default_value.store_id = 0)
 AND (default_value.entity_type_id = 4);

-- result ------------------------------

attribute_id  attr_value
87            store
1286          NULL
1287          default
1289          store
1291          NULL
1556          detault_alone

-- before by store ---------------------

SELECT
     default_value.attribute_id AS debug_id_default,
     default_value.value AS debug_val_default,
     store_value.attribute_id AS debug_id_store,
     store_value.value AS debug_val_store,
 `default_value`.`attribute_id`,
 IF(store_value.value IS NULL, default_value.value, store_value.value) AS `attr_value`

FROM `catalog_product_entity_varchar` AS `default_value`

LEFT JOIN `catalog_product_entity_varchar` AS `store_value`
 ON store_value.attribute_id = default_value.attribute_id
 AND store_value.entity_type_id = 4
 AND store_value.entity_id = 1018
 AND store_value.store_id = 1

WHERE
 (default_value.attribute_id IN (87, 1286, 1287, 1289, 1291, 1293, 1294, 1556))
 AND (default_value.entity_type_id = 4)
 AND (default_value.entity_id = 1018)
 AND (default_value.store_id = 0);

-- after by store ----------------------

SELECT
     default_value.attribute_id AS debug_id_default,
     default_value.value AS debug_val_default,
     store_value.attribute_id AS debug_id_store,
     store_value.value AS debug_val_store,
 IF(store_value.attribute_id IS NULL, default_value.attribute_id, store_value.attribute_id) AS `attribute_id`,
 IF(store_value.attribute_id IS NULL, default_value.value, store_value.value) AS `attr_value`

FROM `catalog_product_entity` AS `e`

INNER JOIN `catalog_product_entity_varchar` AS `attr`
 ON attr.entity_id = e.entity_id
 AND attr.attribute_id IN (87, 1286, 1287, 1289, 1291, 1293, 1294, 1556)
 AND attr.store_id IN (0, 1)
 AND attr.entity_type_id = 4

LEFT JOIN `catalog_product_entity_varchar` AS `default_value`
 ON default_value.entity_id = e.entity_id
 AND default_value.attribute_id = attr.attribute_id
 AND default_value.store_id = 0

LEFT JOIN `catalog_product_entity_varchar` AS `store_value`
 ON store_value.entity_id = e.entity_id
 AND store_value.attribute_id = attr.attribute_id
 AND store_value.store_id = 1

WHERE (e.entity_id = 1018)
GROUP BY `attr`.`attribute_id`;

-- result before -----------------------

id_default  val_default    id_store  val_store   attribute_id   attr_value
87          default        87        store       87             store
1286        NULL           1286      NULL        1286           NULL
1287        default        1287      NULL        1287           default
1289        NULL           1289      store       1289           store
1291        NULL           NULL      NULL        1291           NULL
1556        detault_alone  NULL      NULL        1556           detault_alone

-- result after ------------------------

id_default  val_default    id_store  val_store    attribute_id  attr_value
87          default        87        store        87            store
1286        NULL           1286      NULL         1286          NULL
1287        default        1287      NULL         1287          NULL
1289        NULL           1289      store        1289          store
1291        NULL           NULL      NULL         1291          NULL
NULL        NULL           1293      NULL         1293          NULL
NULL        NULL           1294      store_alone  1294          store_alone
1556        detault_alone  NULL      NULL         1556          detault_alone

getLoadAttributesSelect (for cart and collections)

-- before by store ---------------------

SELECT
 `t_d`.`entity_id`,
 `t_d`.`attribute_id`

FROM `catalog_product_entity_varchar` AS `t_d`

LEFT JOIN `catalog_product_entity_varchar` AS `t_s`
 ON t_s.attribute_id = t_d.attribute_id
 AND t_s.entity_id = t_d.entity_id
 AND t_s.store_id = 1

WHERE (t_d.entity_type_id = 4)
 AND (t_d.entity_id IN (1018))
 AND (t_d.attribute_id IN (87, 1286, 1287, 1289, 1291, 1293, 1294, 1556))
 AND (t_d.store_id = 0);

-- after by store ----------------------

SELECT
 `e`.`entity_id`,
 IF(t_s.attribute_id IS NULL, t_d.attribute_id, t_s.attribute_id) AS `attribute_id`

FROM `catalog_product_entity` AS `e`

INNER JOIN `catalog_product_entity_varchar` AS `attr`
 ON attr.entity_id = e.entity_id
 AND attr.attribute_id IN (87, 1286, 1287, 1289, 1291, 1293, 1294, 1556)
 AND attr.store_id IN (0, 1)
 AND attr.entity_type_id = 4

LEFT JOIN `catalog_product_entity_varchar` AS `t_d`
 ON t_d.entity_id = e.entity_id
 AND t_d.attribute_id = attr.attribute_id
 AND t_d.store_id = 0

LEFT JOIN `catalog_product_entity_varchar` AS `t_s`
 ON t_s.entity_id = e.entity_id
 AND t_s.attribute_id = attr.attribute_id
 AND t_s.store_id = 1

WHERE (e.entity_id IN (1018))
GROUP BY `e`.`entity_id`, `attr`.`attribute_id`;

Problem: for my test (for getLoadAttributesSelect) I lost ~5 ms.
The EXPLAIN says:

  • with group by: Using where; Using index; Using temporary; Using filesort
  • width distinct: Using where; Using index; Using temporary

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)
  • Add yourself to contributors list

@github-actions github-actions bot added the Component: Catalog Relates to Mage_Catalog label Jan 17, 2023
@elidrissidev elidrissidev self-requested a review January 30, 2023 15:19
@luigifab luigifab changed the base branch from 1.9.4.x to main April 9, 2023 08:24
@fballiano
Copy link

is this a situation that happens often? to have attributes at store-level but without the default?

do we know the impact it has on the query?

anywa, if M2 merged this same code, we could probably use it too.

@github-actions github-actions bot added the Component: Eav Relates to Mage_Eav label May 30, 2023
fballiano
fballiano previously approved these changes Sep 18, 2023
Copy link

@fballiano fballiano left a comment

Choose a reason for hiding this comment

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

I've tested with text fields and multiselect and it works correctly.

The queries are more complex:
Screenshot 2023-09-18 alle 17 46 20

but from my knowledge of the "explain" it doesn't seem to have a big impact.

although problem that this PR solves shouldn't be common, the result (coming from this PR) is the expected result.

@fballiano fballiano changed the title Fix getAttributeRawValue when there are no default value Fixed getAttributeRawValue when there are no default value Sep 18, 2023
@luigifab
Copy link
Contributor Author

Yes, we can't be lower than M2.

@fballiano
Copy link

@luigifab do you use this in your stores? are we safe to merge it?

@luigifab
Copy link
Contributor Author

luigifab commented Apr 9, 2024

Yes I use it. I there are issues, I don't find them.

@fballiano fballiano changed the base branch from main to next April 18, 2024 21:25
@fballiano fballiano dismissed their stale review April 18, 2024 21:25

The base branch was changed.

@fballiano fballiano changed the title Fixed getAttributeRawValue when there are no default value Fixed getAttributeRawValue when there are no value in the "default" store Apr 18, 2024
@fballiano fballiano merged commit 157bb9a into OpenMage:next Apr 18, 2024
@fballiano
Copy link

I know it will be controversial but I think that technically we're modifying what's returned by a method that's probably used in custom code, so for that reason I think although it's a bugfix it should be treated as a breaking change. that's why I merged it in next.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Catalog Relates to Mage_Catalog Component: Eav Relates to Mage_Eav next-only
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants