"DDL statements are not allowed in transactions" when cleaning the table

[real34]

We noticed an issue when truncating table in the clearTemporaryIndexTable as part of a transaction (Magento 1.9.1.0-patch2).
The query is considered as a DDL statement and magento triggers an error (appearing in the logs). It fills the log when not in developer mode, and breaks when in developer mode.

Expected: no errors are triggered by Magento

Here is the stack trace of the call:

#1  Varien_Db_Adapter_Pdo_Mysql->query() called at [/path/to/app/htdocs/lib/Varien/Db/Adapter/Pdo/Mysql.php:2497]
#2  Varien_Db_Adapter_Pdo_Mysql->truncateTable() called at [/path/to/app/.modman/Dngdev_Indexfix/app/code/community/Dngdev/Indexer/Model/Resource/Indexer/Stock.php:22]
#3  Dngdev_Indexer_Model_Resource_Indexer_Stock->clearTemporaryIndexTable() called at [/path/to/app/htdocs/app/code/core/Mage/CatalogInventory/Model/Resource/Indexer/Stock.php:245]
#4  Mage_CatalogInventory_Model_Resource_Indexer_Stock->reindexAll() called at [/path/to/app/htdocs/app/code/core/Mage/Index/Model/Indexer/Abstract.php:143]
#5  Mage_Index_Model_Indexer_Abstract->reindexAll() called at [/path/to/app/htdocs/app/code/core/Mage/Index/Model/Process.php:212]
#6  Mage_Index_Model_Process->reindexAll() called at [/path/to/app/htdocs/app/code/community/EcomDev/PHPUnit/Model/Mysql4/Fixture/AbstractEav.php:80]
#7  EcomDev_PHPUnit_Model_Mysql4_Fixture_AbstractEav->runRequiredIndexers() called at [/path/to/app/htdocs/app/code/community/EcomDev/PHPUnit/Model/Fixture/Processor/Eav.php:106]
#8  EcomDev_PHPUnit_Model_Fixture_Processor_Eav->apply() called at [/path/to/app/htdocs/app/code/community/EcomDev/PHPUnit/Model/Fixture.php:458]
#9  EcomDev_PHPUnit_Model_Fixture->apply() called at [/path/to/app/htdocs/app/code/community/EcomDev/PHPUnit/Test/Listener.php:151]
#10 EcomDev_PHPUnit_Test_Listener->startTest() called at [/path/to/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:387]
#11 PHPUnit_Framework_TestResult->startTest() called at [/path/to/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:624]
#12 PHPUnit_Framework_TestResult->run() called at [/path/to/app/vendor/phpunit/phpunit/src/Framework/TestCase.php:760]
#13 PHPUnit_Framework_TestCase->run() called at [/path/to/app/vendor/phpunit/phpunit/src/Framework/TestSuite.php:699]
#14 PHPUnit_Framework_TestSuite->run() called at [/path/to/app/vendor/phpunit/phpunit/src/Framework/TestSuite.php:699]
#15 PHPUnit_Framework_TestSuite->run() called at [/path/to/app/vendor/phpunit/phpunit/src/Framework/TestSuite.php:699]
#16 PHPUnit_Framework_TestSuite->run() called at [/path/to/app/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:426]
#17 PHPUnit_TextUI_TestRunner->doRun() called at [/path/to/app/vendor/phpunit/phpunit/src/TextUI/Command.php:179]
#18 PHPUnit_TextUI_Command->run() called at [/path/to/app/vendor/phpunit/phpunit/src/TextUI/Command.php:132]
#19 PHPUnit_TextUI_Command::main() called at [/path/to/app/vendor/phpunit/phpunit/phpunit:57]

[dng-dev]

Hey @real34 i will look into it. I already have an idea how to fix it. Hopefully i will find some time at the weekend to fix this.
Thanks for reporting the issue

[real34]

Thanks a lot! I am a bit short on time these days to stay longer on this.
Of course, the issue is also in \Dngdev_Indexer_Model_Resource_Product_Indexer_Price::clearTemporaryIndexTable

[real34]

Hey @dng-dev could you make any advancement? Do you have a rough plan about the solution, we could try to give it a try if you think it is feasible.

[real34]

I ended up considering to rewrite the reindexAll() method directly instead of both the clearTemporaryTable() and syncData().

Doing it there would allow for instance to change the useIdxTable() strategy to have another table filled with the data, and outside the transaction just swap it with the real idx table. From what I saw, it seems that MySQL handles multiple operations correctly (so if the second or third rename fails it will rename all the tables back).

Here is a draft (pseudo code) for \Mage_Catalog_Model_Resource_Product_Indexer_Price::reindexAll():

    public function reindexAll()
    {
        $this->useIdxTable('another_tmp_suffix');
        $this->beginTransaction();
        try {
            $this->clearTemporaryIndexTable();
            $this->_prepareWebsiteDateTable();
            $this->_prepareTierPriceIndex();
            $this->_prepareGroupPriceIndex();

            $indexers = $this->getTypeIndexers();
            foreach ($indexers as $indexer) {
                /** @var $indexer Mage_Catalog_Model_Resource_Product_Indexer_Price_Interface */
                $indexer->reindexAll();
            }

            $this->syncData();
            $this->commit();

            $tmpTable = $this->useIdxTable();
            $this->useIdxTable(false);
            $this->_getIndexAdapter()->query(sprintf(
                'RENAME TABLE %s TO %s, %s TO %s',
                $this->useIdxTable(),
                $this->useIdxTable() . '_old',
                $tmpTable,
                $this->useIdxTable()
            ));
        } catch (Exception $e) {
            $this->rollBack();
            throw $e;
        }
        return $this;
    }

Do you think it would work? Is it what you started to consider?

[dng-dev]

Hi @real34 i have completly rewriten all the stuff with ddl and also for other index events. you find it here https://github.com/smart-devs/smartdevs-indexer/.
Anyway thank you for your contribution. i will implement the pull request there also.