DB Transaction is not rolling back properly

I have some wierd thing going on. I wrote a migration system similar to Magento upgrade mechanism. Each module have version and can have Migration class. Runner is responsible for finding out what has not been ported to database yet and run those migration. Code is stupid simple:

        foreach ($versionTable as $migrationVer => $migration) {
            try {
                var_dump($this->db->begin());
                /** @var Migration $migrationClass */
                $migrationClass = new $migration($this->db);
                $migrationClass->up();
                $this->upgradeDbToVersion($moduleName, $migrationVer);
                $this->db->commit();
            } catch (\Exception $e) {
                var_dump($this->db->rollback());
                throw $e;
            }
        };

But the rollback does not work. For example if I have multiple table creation in Migration class and suddenly last table create fails, rollback returns true but previous tables stays in database. Did I miss something?



3.5k
edited Feb '18

I have multiple table creation

I guess, that you use MySQL as your DB, so transactions don't include queries like "CREATE TABLE" More: https://dev.mysql.com/doc/refman/5.7/en/implicit-commit.html

You have to implement yourself, for example my piece of code responsible for creating tables:

        foreach ($sqlArray as $sql) {
            try {
                $db->execute($sql);
            } catch (Exception $e) {
                //because rollback is impossible in that case
                $this->removeTables($config, $db);
                throw new Exception($e);
            }
        }

When error occurs all tables are removed.

edited Mar '18

Each iteration of that loop is a separate transaction. The previous tables still exist because the transaction that was created in previous loops has already been committed. If you want either all the tables to be created or none, then move the transaction and try/catch outside your loop:

$this->db->begin();
try{
    foreach ($versionTable as $migrationVer => $migration) {
        $migrationClass = new $migration($this->db);
        $migrationClass->up();
        $this->upgradeDbToVersion($moduleName, $migrationVer);
    }
    // If execution has reached this point, all the tables have been created
    $this->db->commit();
}
// any failure will break out of the foreach and get us here
catch (\Exception $e) {
    $this->db->rollback();
    throw $e;
}

Nah, it doesn't work like this. It is very close to magento upgrade system so each loop is one migration that upgrades module to specific version. Let me give you an example what migrationClass can be:

<?php

namespace ABC\Task\Migrations;

use Phalcon\Db\Column;
use Phalcon\Db\Index;
use Phalcon\Db\Reference;
use ABC\Core\Migrations\Migration;
use ABC\Task\Models\TaskStatus;

/**
 * Class Migration_109
 */
class Migration_109 extends Migration
{
    const VERSION = '1.0.9';

    /**
     * Define the table structure
     *
     * @return void
     */
    public function up()
    {
        $this->db->createTable('task_statuses', null, [
            'columns' => [
                new Column('id', [
                    'type'          => Column::TYPE_INTEGER,
                    'unsigned'      => true,
                    'notNull'       => true,
                    'autoIncrement' => true,
                    'size'          => 11,
                    'first'         => true
                ]),
                new Column('label', [
                    'type'    => Column::TYPE_VARCHAR,
                    'notNull' => true,
                    'size'    => 255,
                    'after'   => 'id'
                ]),
                new Column('icon', [
                    'type'    => Column::TYPE_VARCHAR,
                    'notNull' => true,
                    'size'    => 255,
                    'after'   => 'label'
                ]),
                new Column('color', [
                    'type'    => Column::TYPE_VARCHAR,
                    'notNull' => true,
                    'size'    => 20,
                    'after'   => 'icon'
                ]),
                new Column('isDefault', [
                    'type'    => Column::TYPE_BOOLEAN,
                    'notNull' => true,
                    'default' => 0,
                ])
            ],
            'indexes' => [
                new Index('PRIMARY', ['id'], 'PRIMARY')
            ],
            'options' => [
                'TABLE_TYPE'      => 'BASE TABLE',
                'ENGINE'          => 'InnoDB',
                'TABLE_COLLATION' => 'utf8_general_ci'
            ],
        ]);

        $newStatus = new TaskStatus();
        $newStatus->color = '#00FF00';
        $newStatus->isDefault = true;
        $newStatus->icon = 'fa fa-glass';
        $newStatus->label = 'Nowy';
        $newStatus->save();

        $defaultStatusId = $newStatus->id;

        $this->db->execute("ALTER TABLE `tasks` CHANGE COLUMN `type` `typeId` INT(11) UNSIGNED NOT NULL;");
        $this->db->execute("ALTER TABLE `tasks` CHANGE COLUMN `priority` `priorityId` INT(11) UNSIGNED NOT NULL;");

        $this->db->addColumn('tasks', null, new Column('statusId', [
            'type'     => Column::TYPE_INTEGER,
            'unsigned' => true,
            'size'     => 11,
            'after'    => 'priorityId',
        ]));

        $this->db->execute("UPDATE `tasks` SET statusId = {$defaultStatusId}");

        $this->db->modifyColumn('tasks', null, new Column('statusId', [
            'type'     => Column::TYPE_INTEGER,
            'unsigned' => true,
            'notNull'  => true,
            'size'     => 11,
            'after'    => 'priorityId',
        ]));

        $this->db->addIndex('tasks', null, new Index('FK_TASKS_TYPE', ['typeId']));
        $this->db->addIndex('tasks', null, new Index('FK_TASKS_PRIORITY', ['priorityId']));
        $this->db->addIndex('tasks', null, new Index('FK_TASKS_STATUS', ['statusId']));

        $this->db->addForeignKey('tasks', null, new Reference('FK_TASKS_TYPE', [
            'referencedTable'   => 'task_types',
            'columns'           => ['typeId'],
            'referencedColumns' => ['id'],
            'onUpdate'          => 'NO ACTION',
            'onDelete'          => 'RESTRICT'
        ]));
        $this->db->addForeignKey('tasks', null, new Reference('FK_TASKS_PRIORITY', [
            'referencedTable'   => 'task_priorities',
            'columns'           => ['priorityId'],
            'referencedColumns' => ['id'],
            'onUpdate'          => 'NO ACTION',
            'onDelete'          => 'RESTRICT'
        ]));
        $this->db->addForeignKey('tasks', null, new Reference('FK_TASKS_STATUS', [
            'referencedTable'   => 'task_statuses',
            'columns'           => ['statusId'],
            'referencedColumns' => ['id'],
            'onUpdate'          => 'NO ACTION',
            'onDelete'          => 'RESTRICT'
        ]));
    }

}

Michal is right, Mysql can't rollback table creation. And I was pretty sure magento could rollback this somewhow but I was probably mistaken.