Skip to content
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

Rebuild Tag aggregation data not working #2857

Closed
sreichel opened this issue Dec 27, 2022 · 20 comments · Fixed by #2858
Closed

Rebuild Tag aggregation data not working #2857

sreichel opened this issue Dec 27, 2022 · 20 comments · Fixed by #2858
Labels

Comments

@sreichel
Copy link
Contributor

sreichel commented Dec 27, 2022

Preconditions (*)

  1. 1.9.4.x
  2. ...

Steps to reproduce (*)

  1. backend index management
  2. tags ...

Actual result (*)

2022-12-27T22:20:03+00:00 ERR (3): 
PDOException: SQLSTATE[42S22]: Column not found: 1054 Unknown column 'COUNT(tr.customer_id) + MIN(IF(tp.base_popularity IS NOT NULL, tp.base_popularity, 0))' in 'field list' in /var/www/html/vendor/shardj/zf1-future/library/Zend/Db/Statement/Pdo.php:228
Stack trace:
#0 /var/www/html/vendor/shardj/zf1-future/library/Zend/Db/Statement/Pdo.php(228): PDOStatement->execute(Array)
#1 /var/www/html/lib/Varien/Db/Statement/Pdo/Mysql.php(104): Zend_Db_Statement_Pdo->_execute(Array)
#2 /var/www/html/vendor/shardj/zf1-future/library/Zend/Db/Statement.php(303): Varien_Db_Statement_Pdo_Mysql->_execute(Array)
#3 /var/www/html/vendor/shardj/zf1-future/library/Zend/Db/Adapter/Abstract.php(480): Zend_Db_Statement->execute(Array)
#4 /var/www/html/vendor/shardj/zf1-future/library/Zend/Db/Adapter/Pdo/Abstract.php(238): Zend_Db_Adapter_Abstract->query('INSERT INTO `ta...', Array)
#5 /var/www/html/lib/Varien/Db/Adapter/Pdo/Mysql.php(492): Zend_Db_Adapter_Pdo_Abstract->query('INSERT INTO `ta...', Array)
#6 /var/www/html/app/code/core/Mage/Tag/Model/Resource/Indexer/Summary.php(215): Varien_Db_Adapter_Pdo_Mysql->query('INSERT INTO `ta...')
#7 /var/www/html/app/code/core/Mage/Tag/Model/Resource/Indexer/Summary.php(125): Mage_Tag_Model_Resource_Indexer_Summary->aggregate()
#8 /var/www/html/app/code/core/Mage/Index/Model/Indexer/Abstract.php(145): Mage_Tag_Model_Resource_Indexer_Summary->reindexAll()

Related code:

Mage_Tag_Model_Resource_Indexer_Summary::aggregate()

                ->from(
                    ['tr' => $this->getTable('tag/relation')],
                    [
                        'tr.tag_id',
                        'tr.store_id',
                        'customers'         => 'COUNT(DISTINCT tr.customer_id)',
                        'products'          => 'COUNT(DISTINCT tr.product_id)',
                        'popularity'        => 'COUNT(tr.customer_id) + MIN('
                            . $writeAdapter->getCheckSql(
                                'tp.base_popularity IS NOT NULL',
                                'tp.base_popularity',
                                '0'
                            )
                            . ')',
                        'uses'              => new Zend_Db_Expr(0), // deprecated since 1.4.0.1
                        'historical_uses'   => new Zend_Db_Expr(0), // deprecated since 1.4.0.1
                        'base_popularity'   => new Zend_Db_Expr(0)  // deprecated since 1.4.0.1
                    ]
                )

Query:

INSERT INTO `tag_summary` (`tag_id`, `store_id`, `customers`, `products`, `popularity`, `uses`, `historical_uses`, `base_popularity`) SELECT `tr`.`tag_id`, `tr`.`store_id`, COUNT(DISTINCT tr.customer_id) AS `customers`, COUNT(DISTINCT tr.product_id) AS `products`, `COUNT(tr.customer_id) + MIN(IF(tp.base_popularity IS NOT NULL, tp`.`base_popularity, 0))` AS `popularity`, 0 AS `uses`, 0 AS `historical_uses`, 0 AS `base_popularity` FROM `tag_relation` AS `tr`
 INNER JOIN `core_store` AS `cs` ON cs.store_id = tr.store_id
 INNER JOIN `catalog_product_website` AS `pw` ON cs.website_id = pw.website_id AND tr.product_id = pw.product_id
 INNER JOIN `catalog_product_entity` AS `e` ON tr.product_id = e.entity_id
 LEFT JOIN `tag_properties` AS `tp` ON tp.tag_id = tr.tag_id AND tp.store_id = tr.store_id
 INNER JOIN `catalog_product_entity_int` AS `tad_status` ON tad_status.entity_id = e.entity_id AND tad_status.attribute_id = 96 AND tad_status.store_id = 0
 LEFT JOIN `catalog_product_entity_int` AS `tas_status` ON tas_status.entity_id = e.entity_id AND tas_status.attribute_id = 96 AND tas_status.store_id = cs.store_id
 INNER JOIN `catalog_product_entity_int` AS `tad_visibility` ON tad_visibility.entity_id = e.entity_id AND tad_visibility.attribute_id = 102 AND tad_visibility.store_id = 0
 LEFT JOIN `catalog_product_entity_int` AS `tas_visibility` ON tas_visibility.entity_id = e.entity_id AND tas_visibility.attribute_id = 102 AND tas_visibility.store_id = cs.store_id
 INNER JOIN `cataloginventory_stock_status` AS `ciss` ON ciss.product_id = e.entity_id AND ciss.website_id = cs.website_id WHERE (tr.active = 1) AND (IF(IFNULL(tas_status.value_id, -1) > 0, tas_status.value, tad_status.value)=1) AND (IF(IFNULL(tas_visibility.value_id, -1) > 0, tas_visibility.value, tad_visibility.value)!=1) AND (ciss.stock_status = 1) GROUP BY `tr`.`tag_id`,
	`tr`.`store_id` ON DUPLICATE KEY UPDATE `tag_id` = VALUES(`tag_id`), `store_id` = VALUES(`store_id`), `customers` = VALUES(`customers`), `products` = VALUES(`products`), `popularity` = VALUES(`popularity`), `uses` = VALUES(`uses`), `historical_uses` = VALUES(`historical_uses`), `base_popularity` = VALUES(`base_popularity`) in /var/www/html/vendor/shardj/zf1-future/library/Zend/Db/Statement/Pdo.php:235

@sreichel sreichel added the bug label Dec 27, 2022
@fballiano
Copy link
Contributor

there's an error in the quoting of this part:

`COUNT(tr.customer_id) + MIN(IF(tp.base_popularity IS NOT NULL, tp`.`base_popularity, 0))` AS `popularity`

it's all wrong but most of all the ` caracter before the COUNT

@elidrissidev
Copy link
Member

Could not reproduce this problem, I'm able to index tag data just fine!?

@fballiano
Copy link
Contributor

weird, doing a reindex it should immediately. mmmm :-\

@elidrissidev
Copy link
Member

There is something with zf1-future that's causing this, and I'm afraid it might affect other parts we're not aware of.

Take this simplified example of the same query:

<?php

require 'app/Mage.php';

Mage::app();

$resource = Mage::getSingleton('core/resource');
$writeAdapter = $resource->getConnection('core_write');

$select = $writeAdapter->select()
    ->from(
        ['tr' => 'tag_relation'],
        [
            'popularity' => 'COUNT(tr.customer_id) + MIN(0, 1)',
        ]
    )
    ->group([
        'tr.tag_id',
        'tr.store_id'
    ])
    ->where('tr.active = 1');

var_dump($select->__toString());

With latest chagnes it outputs:

SELECT `COUNT(tr`.`customer_id) + MIN(0, 1)` AS `popularity`
FROM `tag_relation` AS `tr`
WHERE (tr.active = 1)
GROUP BY `tr`.`tag_id`, `tr`.`store_id`

Whereas when checking out the commit 2bd4e09 (before migration to zf1-future), it outputs:

SELECT COUNT(tr.customer_id) + MIN(0, 1) AS `popularity`
FROM `tag_relation` AS `tr`
WHERE (tr.active = 1)
GROUP BY `tr`.`tag_id`, `tr`.`store_id`

@ADDISON74 ADDISON74 reopened this Dec 28, 2022
@ADDISON74
Copy link
Contributor

@elidrissidev - if you take a look to the error message that Sven's posted 4 lines refer to ZF1 files. I test his PR and it seems to fix this error. I agreed we should do intensive tests over Frontend/Backend after switching to ZF1. Till now I combed the Backend all sections and I did all the actions on a page getting no errors. I admin I did not test the Tags module because this module is disabled as output by default. I will test the others disabled like Backup and Polls.

@elidrissidev
Copy link
Member

I found the cause of the problem. It seems that when cloning the repository for development purposes, the files in app/code/core/Zend are not overriding their counterpart from zf1-future, and those had the patch for this issue, and possibly others too. I tried the same snippet on an OM instance installed with composer, and it works fine.

@fballiano
Copy link
Contributor

is the composer's autoloader being executed before OM's autoloader?

@fballiano
Copy link
Contributor

why do we have 3 autoloaders anyway? 2 of them seems not to be called ever

Screenshot 2022-12-28 alle 14 08 36

@elidrissidev
Copy link
Member

is the composer's autoloader being executed before OM's autoloader?

no it's the last one:

Screen Shot 2022-12-28 at 3 16 40 PM

@sreichel
Copy link
Contributor Author

the files in app/code/core/Zend are not overriding their counterpart from zf1-future, and those had the patch for this issue, and possibly others too.

So we can apply these changes as patches, too?

@fballiano
Copy link
Contributor

it would be great to have them as patches and get rid of app/code/core/Zend, but maybe it would be worth understand why this happens

@elidrissidev
Copy link
Member

elidrissidev commented Dec 28, 2022

This specific problem is originating from the ZF1 implementation of the method Zend_Db_Select::_tableCols(), but the same class overridden in app/code/core/Zend includes a fix to make it possible to correctly parse a string such as COUNT(tr.customer_id) + MIN(0, 1).

I agree with having them as patches and getting rid of app/code/core/Zend altogether if that's possible.

@sreichel
Copy link
Contributor Author

if that's possible.

I've no time today ... its really easy to create patches ... see https://github.com/symplify/vendor-patches

Note: Statement.php is already patched.

@fballiano
Copy link
Contributor

I'm getting on a plane but I'm also trying to take a look at this :-)

@ADDISON74
Copy link
Contributor

I confirm this issue, OM 19 latest version installed from scratch + all Composer stuff.

  1. In Backend >Index Management if I click on Reindex Data link for "Tag Aggregation Data" I am getting "There was a problem with reindexing process".

  2. In /var/log/exceptions.log file I am getting the following content

2022-12-28T22:08:22+00:00 ERR (3): 
PDOException: SQLSTATE[42S22]: Column not found: 1054 Unknown column 'COUNT(tr.customer_id) + MIN(IF(tp.base_popularity IS NOT NULL, tp.base_popularity, 0))' in 'field list' in /var/www/html/vendor/shardj/zf1-future/library/Zend/Db/Statement/Pdo.php:228
Stack trace:
#0 /var/www/html/vendor/shardj/zf1-future/library/Zend/Db/Statement/Pdo.php(228): PDOStatement->execute(Array)
#1 /var/www/html/lib/Varien/Db/Statement/Pdo/Mysql.php(104): Zend_Db_Statement_Pdo->_execute(Array)
#2 /var/www/html/vendor/shardj/zf1-future/library/Zend/Db/Statement.php(303): Varien_Db_Statement_Pdo_Mysql->_execute(Array)
#3 /var/www/html/vendor/shardj/zf1-future/library/Zend/Db/Adapter/Abstract.php(480): Zend_Db_Statement->execute(Array)
#4 /var/www/html/vendor/shardj/zf1-future/library/Zend/Db/Adapter/Pdo/Abstract.php(238): Zend_Db_Adapter_Abstract->query('INSERT INTO `ta...', Array)
#5 /var/www/html/lib/Varien/Db/Adapter/Pdo/Mysql.php(492): Zend_Db_Adapter_Pdo_Abstract->query('INSERT INTO `ta...', Array)
#6 /var/www/html/app/code/core/Mage/Tag/Model/Resource/Indexer/Summary.php(213): Varien_Db_Adapter_Pdo_Mysql->query('INSERT INTO `ta...')
#7 /var/www/html/app/code/core/Mage/Tag/Model/Resource/Indexer/Summary.php(125): Mage_Tag_Model_Resource_Indexer_Summary->aggregate()
#8 /var/www/html/app/code/core/Mage/Index/Model/Indexer/Abstract.php(145): Mage_Tag_Model_Resource_Indexer_Summary->reindexAll()
#9 /var/www/html/app/code/core/Mage/Index/Model/Process.php(207): Mage_Index_Model_Indexer_Abstract->reindexAll()
#10 /var/www/html/app/code/core/Mage/Index/Model/Process.php(255): Mage_Index_Model_Process->reindexAll()
#11 /var/www/html/app/code/core/Mage/Index/controllers/Adminhtml/ProcessController.php(135): Mage_Index_Model_Process->reindexEverything()
#12 /var/www/html/app/code/core/Mage/Core/Controller/Varien/Action.php(428): Mage_Index_Adminhtml_ProcessController->reindexProcessAction()
#13 /var/www/html/app/code/core/Mage/Core/Controller/Varien/Router/Standard.php(262): Mage_Core_Controller_Varien_Action->dispatch('reindexProcess')
#14 /var/www/html/app/code/core/Mage/Core/Controller/Varien/Front.php(188): Mage_Core_Controller_Varien_Router_Standard->match(Object(Mage_Core_Controller_Request_Http))
#15 /var/www/html/app/code/core/Mage/Core/Model/App.php(371): Mage_Core_Controller_Varien_Front->dispatch()
#16 /var/www/html/app/Mage.php(745): Mage_Core_Model_App->run(Array)
#17 /var/www/html/index.php(71): Mage::run('', 'store')
#18 {main}

Next Zend_Db_Statement_Exception: SQLSTATE[42S22]: Column not found: 1054 Unknown column 'COUNT(tr.customer_id) + MIN(IF(tp.base_popularity IS NOT NULL, tp.base_popularity, 0))' in 'field list', query was: INSERT INTO `tag_summary` (`tag_id`, `store_id`, `customers`, `products`, `popularity`, `uses`, `historical_uses`, `base_popularity`) SELECT `tr`.`tag_id`, `tr`.`store_id`, COUNT(DISTINCT tr.customer_id) AS `customers`, COUNT(DISTINCT tr.product_id) AS `products`, `COUNT(tr.customer_id) + MIN(IF(tp.base_popularity IS NOT NULL, tp`.`base_popularity, 0))` AS `popularity`, 0 AS `uses`, 0 AS `historical_uses`, 0 AS `base_popularity` FROM `tag_relation` AS `tr`
 INNER JOIN `core_store` AS `cs` ON cs.store_id = tr.store_id
 INNER JOIN `catalog_product_website` AS `pw` ON cs.website_id = pw.website_id AND tr.product_id = pw.product_id
 INNER JOIN `catalog_product_entity` AS `e` ON tr.product_id = e.entity_id
 LEFT JOIN `tag_properties` AS `tp` ON tp.tag_id = tr.tag_id AND tp.store_id = tr.store_id
 INNER JOIN `catalog_product_entity_int` AS `tad_status` ON tad_status.entity_id = e.entity_id AND tad_status.attribute_id = 96 AND tad_status.store_id = 0
 LEFT JOIN `catalog_product_entity_int` AS `tas_status` ON tas_status.entity_id = e.entity_id AND tas_status.attribute_id = 96 AND tas_status.store_id = cs.store_id
 INNER JOIN `catalog_product_entity_int` AS `tad_visibility` ON tad_visibility.entity_id = e.entity_id AND tad_visibility.attribute_id = 102 AND tad_visibility.store_id = 0
 LEFT JOIN `catalog_product_entity_int` AS `tas_visibility` ON tas_visibility.entity_id = e.entity_id AND tas_visibility.attribute_id = 102 AND tas_visibility.store_id = cs.store_id
 INNER JOIN `cataloginventory_stock_status` AS `ciss` ON ciss.product_id = e.entity_id AND ciss.website_id = cs.website_id WHERE (tr.active = 1) AND (IF(IFNULL(tas_status.value_id, -1) > 0, tas_status.value, tad_status.value)=1) AND (IF(IFNULL(tas_visibility.value_id, -1) > 0, tas_visibility.value, tad_visibility.value)!=1) AND (ciss.stock_status = 1) GROUP BY `tr`.`tag_id`,
	`tr`.`store_id` ON DUPLICATE KEY UPDATE `tag_id` = VALUES(`tag_id`), `store_id` = VALUES(`store_id`), `customers` = VALUES(`customers`), `products` = VALUES(`products`), `popularity` = VALUES(`popularity`), `uses` = VALUES(`uses`), `historical_uses` = VALUES(`historical_uses`), `base_popularity` = VALUES(`base_popularity`) in /var/www/html/vendor/shardj/zf1-future/library/Zend/Db/Statement/Pdo.php:235
Stack trace:
#0 /var/www/html/lib/Varien/Db/Statement/Pdo/Mysql.php(104): Zend_Db_Statement_Pdo->_execute(Array)
#1 /var/www/html/vendor/shardj/zf1-future/library/Zend/Db/Statement.php(303): Varien_Db_Statement_Pdo_Mysql->_execute(Array)
#2 /var/www/html/vendor/shardj/zf1-future/library/Zend/Db/Adapter/Abstract.php(480): Zend_Db_Statement->execute(Array)
#3 /var/www/html/vendor/shardj/zf1-future/library/Zend/Db/Adapter/Pdo/Abstract.php(238): Zend_Db_Adapter_Abstract->query('INSERT INTO `ta...', Array)
#4 /var/www/html/lib/Varien/Db/Adapter/Pdo/Mysql.php(492): Zend_Db_Adapter_Pdo_Abstract->query('INSERT INTO `ta...', Array)
#5 /var/www/html/app/code/core/Mage/Tag/Model/Resource/Indexer/Summary.php(213): Varien_Db_Adapter_Pdo_Mysql->query('INSERT INTO `ta...')
#6 /var/www/html/app/code/core/Mage/Tag/Model/Resource/Indexer/Summary.php(125): Mage_Tag_Model_Resource_Indexer_Summary->aggregate()
#7 /var/www/html/app/code/core/Mage/Index/Model/Indexer/Abstract.php(145): Mage_Tag_Model_Resource_Indexer_Summary->reindexAll()
#8 /var/www/html/app/code/core/Mage/Index/Model/Process.php(207): Mage_Index_Model_Indexer_Abstract->reindexAll()
#9 /var/www/html/app/code/core/Mage/Index/Model/Process.php(255): Mage_Index_Model_Process->reindexAll()
#10 /var/www/html/app/code/core/Mage/Index/controllers/Adminhtml/ProcessController.php(135): Mage_Index_Model_Process->reindexEverything()
#11 /var/www/html/app/code/core/Mage/Core/Controller/Varien/Action.php(428): Mage_Index_Adminhtml_ProcessController->reindexProcessAction()
#12 /var/www/html/app/code/core/Mage/Core/Controller/Varien/Router/Standard.php(262): Mage_Core_Controller_Varien_Action->dispatch('reindexProcess')
#13 /var/www/html/app/code/core/Mage/Core/Controller/Varien/Front.php(188): Mage_Core_Controller_Varien_Router_Standard->match(Object(Mage_Core_Controller_Request_Http))
#14 /var/www/html/app/code/core/Mage/Core/Model/App.php(371): Mage_Core_Controller_Varien_Front->dispatch()
#15 /var/www/html/app/Mage.php(745): Mage_Core_Model_App->run(Array)
#16 /var/www/html/index.php(71): Mage::run('', 'store')
#17 {main}

Updating my local repository with #2858 I was able to fix this issue.

@ADDISON74
Copy link
Contributor

I agree to remove /app/code/core/Zend directory but as we have caught a few of us, without those custom changes made in the past by the Magento team OM does not work.

The are two options

  • the first one we will have to patch those two files directly in /lib/Zend as you suggested before. For those who use Composer there will be no issue, nor will they notice this aspect if things are done well, but an assessment must also be made for those who download the archive and copy it over what they currently have. With the transition to ZF1-Future, those who choose this update method must first delete the residues.

  • the second one would be to understand what needs to be changed in the OM code so that that patch is no longer needed.

@fballiano
Copy link
Contributor

So, I really don't know if we should port the patched versions of Zend_DB_Select and Statement, the ZF1 are more advanced and to directly port our patched version we're losing some of their new funcionalities, which is wrong in my opinion.

In the case of this issue, the error is triggered because of Shardj/zf1-future#200, REGEX_COLUMN_EXPR doesn't detect COUNT(tr.customer_id) + MIN(IF(tp.base_popularity IS NOT NULL, tp.base_popularity, 0)) as a function and doesn't wrap it in a Zend_DB_Expr.

@sreichel
Copy link
Contributor Author

sreichel commented Dec 29, 2022

So, I really don't know if we should port the patched versions of Zend_DB_Select and Statement, the ZF1 are more advanced and to directly port our patched version we're losing some of their new funcionalities, which is wrong in my opinion.

This is why both files are still are in core/Zend. No way to find out why Magento team changed it - and its hard to check which changes made at zf-1 - possibly fixing same issue in another way.

We can blindly patch zf1 to have same result as before, but then we maybe loose some changes/improvents made there.

@fballiano
Copy link
Contributor

I read the diff in depth and, IMHO, we should not port those modifications and we should keep the clean ZF1F version.

What we found in this issue is that the clean ZF1F doesn't recognise expressions when they have more than one parenthesis (Shardj/zf1-future#200 (comment)) but I wasn't even expecting that behaviour anyway, Zend_Db_Expr was created many years ago and it was always clear that expressions have to be "quoted" with Zend_Db_Expr.

It's not easy to search our codebase for possible points where we have the same situation we found for this issue, or if this may happen in 3rd party modules...

@elidrissidev
Copy link
Member

Let's move this discussion to the newly created PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants