Description
Preconditions and environment
Tested on 2.4.5_p10 and 2.4.6
Put all indexers into On Save mode.
Use sample data
Steps to reproduce
- Navigate to backend product edit page for any product. I used Proteus Fitness Jackshirt-XS-Black SKU:MJ12-XS-Black
- Change any attribute that shouldn't impact the price. I set the Material attibute to 'Burlap'
- Click on Save
Expected result
The price index is not updated
Actual result
Any change of any attribute results in price reindexing (and therefore cache flushing). This potentially results in a huge amount of unneccesary reindexing and cache flushing which potentially directly impacts the 'speed' of ALL Magento websites and probably adds to the general belief that Magento is 'resource hungry' and requires super powerful servers.
Additional information
It appears that Magento has logic to only reindex the price index when certain attributes are changed (in save mode)
In vendor/magento/module-catalog/Model/Product.php we see the following code that appears to imply this
/**
* Callback for entity reindex
*
* @return void
*/
public function priceReindexCallback()
{
if ($this->isObjectNew() || **$this->_catalogProduct->isDataForPriceIndexerWasChanged($this)**) {
$this->_productPriceIndexerProcessor->reindexRow($this->getEntityId());
}
}
In vendor/magento/module-catalog/Helper/Product.php we can find the method isDataForPriceIndexerWasChanged()
/**
* Retrieve data for price indexer update
*
* @param \Magento\Catalog\Model\Product|array $data
* @return bool
*/
public function isDataForPriceIndexerWasChanged($data)
{
if ($data instanceof ModelProduct) {
foreach ($this->_reindexPriceIndexerData['byDataResult'] as $param) {
if ($data->getData($param)) {
return true;
}
}
foreach ($this->_reindexPriceIndexerData['byDataChange'] as $param) {
if ($data->dataHasChangedFor($param)) {
return true;
}
}
} elseif (is_array($data)) {
foreach ($this->_reindexPriceIndexerData['byDataChange'] as $param) {
if (isset($data[$param])) {
return true;
}
}
}
return false;
}
THIS METHOD ALWAYS RETURNS TRUE and so a price reindex always gets kicked off when any attribute is changed !!!
This is because of a bug in the dataHasChangedFor method in vendor/magento/framework/Model/AbstractModel.php
/**
* Compare object data with original data
*
* @param string $field
* @return bool
*/
public function dataHasChangedFor($field)
{
$newData = $this->getData($field);
$origData = $this->getOrigData($field);
**return $newData != $origData;** THIS CODE DOES NOT WORK FOR ARRAYS
}
For the price index the attributes that appear to affect the price reindexing are status, price, special_price, special_to_date, special_from_date, website_ids, gift_wrapping_price, tax_class_id, msrp, and msrp_display_actual_price_type
However, the website_ids variable is actually an array. Whilst the values of website_ids for $newData, and $origData are both ["1"], the array indices are different
$newData is [ 1 => "1" ] and $origData is [ 0 => "1"] and because of this return $newData != $origData returns TRUE even though there has been no change in website ids !! This then kicks off unnecessary price reindexing (and possibly cache flushing)


I'm not sure how this passed testing and how no one has spotted this considering the impact it has on indexing and the full page cache and given that it appears to potentially affect every Magento store and has done for years.
A quick and dirty way of fixing this would be to change the code to something like:
public function dataHasChangedFor($field)
{
$newData = $this->getData($field);
$origData = $this->getOrigData($field);
if (is_array($newData)) {
sort($newData);
}
if (is_array($origData)) {
sort($origData);
}
return $newData != $origData;
}
Release note
No response
Triage and priority
- Severity: S0 - Affects critical data or functionality and leaves users without workaround.
- Severity: S1 - Affects critical data or functionality and forces users to employ a workaround.
- Severity: S2 - Affects non-critical data or functionality and forces users to employ a workaround.
- Severity: S3 - Affects non-critical data or functionality and does not force users to employ a workaround.
- Severity: S4 - Affects aesthetics, professional look and feel, “quality” or “usability”.
Metadata
Metadata
Assignees
Labels
Type
Projects
Status