I'm facing an interesting problem regarding a custom modification to the backend Catalog Product grid.

NB: I'm currently using a module adding columns to this grid via an observer.

Nevertheless, the problem is also happening if I create a copy of app/code/core/Mage/Adminhtml/Block/Catalog/Product/Grid.php under app/code/local.

Here are the modifications I have made, I've changed the attribute I'm using with the native tax_class_id attribute so anyone can replicate the issue:

Modified the _prepareCollection method:

Before:

$collection = Mage::getModel('catalog/product')->getCollection()
            ->addAttributeToSelect('sku')
            ->addAttributeToSelect('name')
            ->addAttributeToSelect('attribute_set_id')
            ->addAttributeToSelect('type_id');

After:

$collection = Mage::getModel('catalog/product')->getCollection()
            ->addAttributeToSelect('sku')
            ->addAttributeToSelect('name')
            ->addAttributeToSelect('attribute_set_id')
            ->addAttributeToSelect('tax_class_id')
            ->addAttributeToSelect('type_id');

Modified the _prepareColumns method by adding:

// Get the attribute corresponding to my custom attribute
$attribute = Mage::getResourceModel('eav/entity_attribute_collection')
        ->setCodeFilter("tax_class_id")
        ->getFirstItem();

// Get the attribute options
$attrOptions = $attribute->getSource()->getAllOptions(false);

// Convert the options to an array that can be used as the 'options' parameter for the addColumn method
$optionsArr = array();
foreach ($attrOptions as $option)
{
    $optionsArr[$option['value']] = $option['label'];
}

$this->addColumn('tax_class_id',
    array(
        'header'=> Mage::helper('catalog')->__('Tax'),
        'width' => '60px',
        'index' => 'tax_class_id',
        'type'  => 'options',
        'filter_index' => 'tax_class_id',
        'options' => $optionsArr
));

That works perfectly fine in the following cases:

  • Access Catalog > Product grid
  • Filter by product type, attribute set, sku, price, quantity, visibility, status as well as the new custom column.
  • Sort by product type, attribute set, sku, price, quantity, visibility, status as well as the new custom column.
  • Browse via pager
  • Change page size

However as soon as I start dealing with the name column, every cell of my custom column become empty/the product value is not displayed anymore, so when I:

  • Filter by name
  • Sort by name

How is that specific name column affecting my current custom column and how can I fix the issue ?

up vote 6 down vote accepted

After digging very deep into Magento code and getting my hands dirty as I expected, I've found the root cause of the problem.

When the collection is loaded (implicitely via getFirstItem in @Simon test script), the _loadAttributes method from Mage_Eav_Model_Entity_Collection_Abstract is triggered.

The goal of this method is to load the attributes we've requested via the addAttributeToSelect method.

Here is a quick summary of the method code flow:

  • Get the attributes type and attributes corresponding tables for each attributes to be selected (in our test script it would be int / catalog_product_entity_id and decimal / catalog_product_entity_decimal )
  • Generate one SQL query per distinct attributes table (in our test, there is two different SQL queries : one for price attribute and one for the tax_class_id attribute)
  • Finally it loops through the select groups previously generated and for each select group, proceed to an UNION ALL before setting the corresponding values to the entity.

That very last part of the code is very important, as this is where the bug is located (if you did not guess until now, it is a Magento bug).

Ready for some reading ?

So let's look at the code now, in our _loadAttributes method, we have the following code:

$selectGroups = Mage::getResourceHelper('eav')->getLoadAttributesSelectGroups($selects);
foreach ($selectGroups as $selects) {
    if (!empty($selects)) {
        try {
            $select = implode(' UNION ALL ', $selects);
            $values = $this->getConnection()->fetchAll($select);
        } catch (Exception $e) {
            Mage::printException($e, $select);
            $this->printLogQuery(true, true, $select);
            throw $e;
        }

        foreach ($values as $value) {
            $this->_setItemAttributeValue($value);
        }
    }
}

Problem is, right before the loop, the getLoadAttributesSelectGroups method from Mage_Eav_Model_Resource_Helper_Mysql4 looks like this:

public function getLoadAttributesSelectGroups($selects)
{
    $mainGroup  = array();
    foreach ($selects as $eavType => $selectGroup) {
        $mainGroup = array_merge($mainGroup, $selectGroup);
    }
    return array($mainGroup);
}

Did you spot the return statement ? We return an array of arrays. So the loop in our _loadAttributes method is never really used as there will always be one and only item (containing arrays) in the $selectGroups variable.

As this variable is not properly set, we keep going and reach the heart of the bug which is causing int being converted to decimals:

$select = implode(' UNION ALL ', $selects);

So our $selects variable contains two selects from two different tables of two different types (int and decimal).

What does the MySQL documentation has to tell us about this:

The column names from the first SELECT statement are used as the column names for the results returned. Selected columns listed in corresponding positions of each SELECT statement should have the same data type. (For example, the first column selected by the first statement should have the same type as the first column selected by the other statements.)

If the data types of corresponding SELECT columns do not match, the types and lengths of the columns in the UNION result take into account the values retrieved by all of the SELECT statements.

So, implicitely, MySQL converts our int values to decimal values as it is not possible to convert the other way around.

And that only happens if you try to select decimal and int attributes, as if you select varchar attributes for instance, the int and decimal results will be converted to varchar and the problem disappears.

So how do I fix that ?

First, we need to find what is buggy:

  • Is it the _loadAttributes method ?
  • Is it the getLoadAttributesSelectGroups method ?

Well, I have found only one other reference to the getLoadAttributesSelectGroups method, it is under the _loadModelAttributes of the Mage_Eav_Model_Entity_Abstract model and it looks like this:

$selectGroups = Mage::getResourceHelper('eav')->getLoadAttributesSelectGroups($selects);
foreach ($selectGroups as $selects) {
    if (!empty($selects)) {
        $select = $this->_prepareLoadSelect($selects);
        $values = $this->_getReadAdapter()->fetchAll($select);
        foreach ($values as $valueRow) {
            $this->_setAttributeValue($object, $valueRow);
        }
    }
}

So in Magento developers mind, that function should definitely be returning an array with several entries as everytime this function is called, a foreach statement is used to loop through it.

So I'll give you the fix I've applied:

In getLoadAttributesSelectGroups method of Mage_Eav_Model_Resource_Helper_Mysql4, replace:

return array($mainGroup);

With:

return $mainGroup;

In _loadAttributes method of Mage_Eav_Model_Entity_Collection_Abstract, replace:

$select = implode(' UNION ALL ', $selects);
$values = $this->getConnection()->fetchAll($select);

With:

if (is_array($selects)) {
    $select = implode(' UNION ALL ', $selects);
} else {
    $select = $selects;
}
$values = $this->getConnection()->fetchAll($select);

In the _loadModelAttribures of the Mage_Eav_Model_Entity_Abstract, replace:

$select = $this->_prepareLoadSelect($selects);
$values = $this->_getReadAdapter()->fetchAll($select);

With:

if (is_array($selects)) {
    $select = $this->_prepareLoadSelect($selects);
} else {
    $select = $selects;
}
$values = $this->_getReadAdapter()->fetchAll($select);

Well done you made it to the end and so did I.

Edit: regarding the bug on Magento 2, I've created a Pull Request that can be found here: https://github.com/magento/magento2/pull/3705 (related issue: https://github.com/magento/magento2/issues/3704)

Edit 2: regarding the bug on Magento 1, I've reported the bug you can find the report here: https://www.magentocommerce.com/bug-tracking/issue/index/id/1346

I digged into that a bit and found out that this is a really weird Magento behaviour. I did not find a solution yet, but wanted to share my findings anyway.

The issue is actually that if you have a product collection and:

  • have the flat catalog disabled (which is always the case in the backend)
  • filter/sort by name
  • select the tax_class_id and the price attribute

then the tax_class_id will not return integer values (e.g. 1), but floating values (e.g. 1.0000). That is why the options renderer (Mage_Adminhtml_Block_Widget_Grid_Column_Renderer_Options) does not detect the correct value and therefore does not show anything.

You can reproduce this weird behaviour with this little test script:

<?php

require_once 'app/Mage.php';

Mage::app();

$collection = Mage::getModel('catalog/product')->getCollection()
    ->addAttributeToSelect('tax_class_id')
    ->addAttributeToSort('name');

// toggle this to see the weird result
$collection->addAttributeToSelect('price');

// to make the script faster and lighter
$collection->setPageSize(1);

if ($collection->getSize() > 0) {
    var_dump($collection->getFirstItem()->getData('tax_class_id'));
}

// does not help, the SQL is exactly the same
// echo (string) $collection->getSelect();
  • Very helpful, on a side note, it's important to disable flat catalog when using your script directly in the browser /test.php. I'm investigating the problem right now, gotta feeling I'm about to get really deep into Magento core and get my hands dirty ^^ – Raphael at Digital Pianism Mar 9 '16 at 9:52

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.