phalcon / cphalcon

High performance, full-stack PHP framework delivered as a C extension.
https://phalcon.io
BSD 3-Clause "New" or "Revised" License
10.79k stars 1.96k forks source link

[BUG]: `Phalcon\Mvc\Model\Query` does not handle internal transaction on PDOException #16604

Closed poz closed 5 months ago

poz commented 5 months ago

Describe the bug When a PDOException is throw by Phalcon\Mvc\Model\Query::executeUpdate(), the internal transaction did not rollback correctly and mess up the subsequent queries.

To Reproduce Steps to reproduce the behavior:

  1. Create & populate a table with about 5 row of data (see example script below).
  2. Use $this->modelsManager->executeQuery to update for each single row.
  3. Force the 3rd update to hit a PDOException by updating a string value that is longer than varchar(10) (mysql strict mode)
  4. Catch and handle the exeception above. Continue updating remaining rows

Result:

Provide minimal script to reproduce the issue

<?php

use Phalcon\Mvc\Model;
use Phalcon\Di\FactoryDefault;
use Phalcon\DI\Injectable;
use Phalcon\Db\Adapter\Pdo\MySQL;

/*
Setup Model
set sql_mode=STRICT_ALL_TABLES;

CREATE TABLE `test_table`  (
  `id` INT UNSIGNED NOT NULL AUTO_INCREMENT,
  `name` VARCHAR(10) NOT NULL,
  PRIMARY KEY (`id`)
) ENGINE=INNODB;
*/
class TestModel extends Model
{
    public $id;
    public $name; // varchar 10

    public function initialize() {
        $this->setSource('test_table');
    }
}

// Setup db
$options = [
    'host'     => 'mysql',
    'dbname'   => 'test-phalcon',
    'port'     => 3306,
    'username' => 'user',
    'password' => 'password',
];
$connection = new MySQL($options);

// Create a DI
$di = new FactoryDefault();
$di->setShared('db', $connection);

// Main testing code
class TestComponent extends Injectable
{
    public function runTest() {

        // Reset / Repopulate 5 rows of data
        $this->modelsManager->executeQuery('DELETE FROM TestModel');
        $this->modelsManager->executeQuery('INSERT INTO TestModel (id) VALUES (1)');
        $this->modelsManager->executeQuery('INSERT INTO TestModel (id) VALUES (2)');
        $this->modelsManager->executeQuery('INSERT INTO TestModel (id) VALUES (3)');
        $this->modelsManager->executeQuery('INSERT INTO TestModel (id) VALUES (4)');
        $this->modelsManager->executeQuery('INSERT INTO TestModel (id) VALUES (5)');

        $this->update(1, 'test 1 OK');
        $this->update(2, 'test 2 OK');
        $this->update(3, 'test 3 DATA TRUNCATED ERROR');
        $this->update(4, 'test 4 OK');
        $this->update(5, 'test 5 OK');
    }

    public function update($id, $name) {
        try {
            $query = 'UPDATE TestModel SET name = :name: WHERE id = :id:';
            $result = $this->modelsManager->executeQuery($query, ['id' => $id, 'name' => $name]);
            echo "Update test $id OK" . PHP_EOL;
        } catch (Exception $ex) {
            // just print and ignore the exception here
            echo "Update test $id " . get_class($ex) . ':' . $ex->getMessage() . PHP_EOL;
        }
    }
}

$test = new TestComponent();
$test->runTest();

Expected behavior

Update test 1 OK
Update test 2 OK
Update test 3 PDOException:SQLSTATE[22001]: String data, right truncated: 1406 Data too long for column 'name' at row 1
Update test 4 OK
Update test 5 OK

Details

Additional context I did some investigation to the source and notice that in Phalcon\Mvc\Model\Query , it does not catch any exception.

https://github.com/phalcon/cphalcon/blob/master/phalcon/Mvc/Model/Query.zep#L1485-L1515

I'm not familiar with zephir I've assumed that the exception is not handled properly there. It seems to leave the function instantly without calling connection->rollback when exception happens.

However if I manually called $this->db->rollback() right after catching the exception, the result is working as intended. I don't think this is a good workaround in case exception is happened elsewhere though.

Below are the (simplified) mysql general log, which also confirms my suspicion.


SELECT IF(COUNT(*) > 0, 1, 0) FROM `INFORMATION_SCHEMA`.`TABLES` WHERE `TABLE_NAME` = 'test_table' AND `TABLE_SCHEMA` = DATABASE()
DESCRIBE `test_table`
SELECT `test_table`.`id`, `test_table`.`name` FROM `test_table`
START TRANSACTION
DELETE FROM `test_table` WHERE `id` = 1
DELETE FROM `test_table` WHERE `id` = 2
DELETE FROM `test_table` WHERE `id` = 3
DELETE FROM `test_table` WHERE `id` = 4
DELETE FROM `test_table` WHERE `id` = 5
COMMIT
SELECT COUNT(*) "rowcount" FROM `test_table` WHERE `id` = 1
SELECT COUNT(*) "rowcount" FROM `test_table` WHERE `id` = 1
INSERT INTO `test_table` (`name`, `id`) VALUES ('test 1', 1)
SELECT COUNT(*) "rowcount" FROM `test_table` WHERE `id` = 2
SELECT COUNT(*) "rowcount" FROM `test_table` WHERE `id` = 2
INSERT INTO `test_table` (`name`, `id`) VALUES ('test 2', 2)
SELECT COUNT(*) "rowcount" FROM `test_table` WHERE `id` = 3
SELECT COUNT(*) "rowcount" FROM `test_table` WHERE `id` = 3
INSERT INTO `test_table` (`name`, `id`) VALUES ('test 3', 3)
SELECT COUNT(*) "rowcount" FROM `test_table` WHERE `id` = 4
SELECT COUNT(*) "rowcount" FROM `test_table` WHERE `id` = 4
INSERT INTO `test_table` (`name`, `id`) VALUES ('test 4', 4)
SELECT COUNT(*) "rowcount" FROM `test_table` WHERE `id` = 5
SELECT COUNT(*) "rowcount" FROM `test_table` WHERE `id` = 5
INSERT INTO `test_table` (`name`, `id`) VALUES ('test 5', 5)
SELECT `test_table`.`id`, `test_table`.`name` FROM `test_table` WHERE `test_table`.`id` = '1'
START TRANSACTION
UPDATE `test_table` SET `name` = 'test 1 OK' WHERE `id` = 1
COMMIT
SELECT `test_table`.`id`, `test_table`.`name` FROM `test_table` WHERE `test_table`.`id` = '2'
START TRANSACTION
UPDATE `test_table` SET `name` = 'test 2 OK' WHERE `id` = 2
COMMIT
SELECT `test_table`.`id`, `test_table`.`name` FROM `test_table` WHERE `test_table`.`id` = '3'
START TRANSACTION
UPDATE `test_table` SET `name` = 'test 3 DATA TRUNCATED ERROR' WHERE `id` = 3
SELECT `test_table`.`id`, `test_table`.`name` FROM `test_table` WHERE `test_table`.`id` = '4'
UPDATE `test_table` SET `name` = 'test 4 OK' WHERE `id` = 4
SELECT `test_table`.`id`, `test_table`.`name` FROM `test_table` WHERE `test_table`.`id` = '5'
UPDATE `test_table` SET `name` = 'test 5 OK' WHERE `id` = 5
ROLLBACK

Noticed how after 3rd row update, the was no more COMMIT because pdo adapater handles transaction level internally and it stays on level 2 until the end of script

poz commented 5 months ago

Additional findings:

This also happen to DELETE query. INSERT query are not affected because executeInsert() does not call connection->begin()

noone-silent commented 5 months ago

I look into this

niden commented 5 months ago

Resolved in https://github.com/phalcon/cphalcon/pull/16610

Thank you to @noone-silent and thank you @poz for reporting