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.

Sign up for GitHub

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

Jump to bottom

Fix inconsistency in PDO transaction state #6355

Closed
wants to merge 2 commits into from
Closed

Fix inconsistency in PDO transaction state #6355

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3a4cf02
Fix inconsistency in PDO transaction state
nikic Oct 20, 2020
9d55f8f
Add some tests for mixing transaction APIs and queries
nikic Oct 26, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions ext/pdo/pdo_dbh.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,14 @@ PHP_METHOD(PDO, prepare)
}
/* }}} */


static zend_bool pdo_is_in_transaction(pdo_dbh_t *dbh) {
if (dbh->methods->in_transaction) {
return dbh->methods->in_transaction(dbh);
}
return dbh->in_txn;
}

/* {{{ Initiates a transaction */
PHP_METHOD(PDO, beginTransaction)
{
Expand All @@ -586,7 +594,7 @@ PHP_METHOD(PDO, beginTransaction)

PDO_CONSTRUCT_CHECK;

if (dbh->in_txn) {
if (pdo_is_in_transaction(dbh)) {
zend_throw_exception_ex(php_pdo_get_exception(), 0, "There is already an active transaction");
RETURN_THROWS();
}
Expand Down Expand Up @@ -617,7 +625,7 @@ PHP_METHOD(PDO, commit)

PDO_CONSTRUCT_CHECK;

if (!dbh->in_txn) {
if (!pdo_is_in_transaction(dbh)) {
zend_throw_exception_ex(php_pdo_get_exception(), 0, "There is no active transaction");
RETURN_THROWS();
}
Expand All @@ -641,7 +649,7 @@ PHP_METHOD(PDO, rollBack)

PDO_CONSTRUCT_CHECK;

if (!dbh->in_txn) {
if (!pdo_is_in_transaction(dbh)) {
zend_throw_exception_ex(php_pdo_get_exception(), 0, "There is no active transaction");
RETURN_THROWS();
}
Expand All @@ -665,11 +673,7 @@ PHP_METHOD(PDO, inTransaction)

PDO_CONSTRUCT_CHECK;

if (!dbh->methods->in_transaction) {
RETURN_BOOL(dbh->in_txn);
}

RETURN_BOOL(dbh->methods->in_transaction(dbh));
RETURN_BOOL(pdo_is_in_transaction(dbh));
}
/* }}} */

Expand Down
11 changes: 8 additions & 3 deletions ext/pdo_mysql/tests/pdo_mysql_commit.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,14 @@ if (false == MySQLPDOTest::detect_transactional_mysql_engine($db))
// DDL will issue an implicit commit
$db->exec(sprintf('DROP TABLE IF EXISTS test_commit'));
$db->exec(sprintf('CREATE TABLE test_commit(id INT) ENGINE=%s', MySQLPDOTest::detect_transactional_mysql_engine($db)));
if (true !== ($tmp = $db->commit())) {
printf("[002] No commit allowed? [%s] %s\n",
$db->errorCode(), implode(' ', $db->errorInfo()));
try {
$db->commit();
$failed = false;
} catch (PDOException $e) {
$failed = true;
}
if (!$failed) {
printf("[002] Commit should have failed\n");
}

// pdo_transaction_transitions should check this as well...
Expand Down
53 changes: 49 additions & 4 deletions ext/pdo_mysql/tests/pdo_mysql_inTransaction.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ const BEGIN = ['BEGIN', 'START TRANSACTION'];
const END = ['COMMIT', 'ROLLBACK'];

$db = MySQLPDOTest::factory();
// $pdo->setAttribute(PDO::ATTR_EMULATE_PREPARES, false); // mysql does not support
for ($b = 0; $b < count(BEGIN); $b++) {
for ($e = 0; $e < count(END); $e++) {
// $db->setAttribute(PDO::ATTR_EMULATE_PREPARES, false); // mysql does not support
foreach (BEGIN as $begin) {
foreach (END as $end) {
foreach (['exec', 'query', 'execute'] as $w) {
foreach ([BEGIN[$b], END[$e]] as $command) {
foreach ([$begin, $end] as $command) {
switch ($w) {
case 'exec':
$db->exec($command);
Expand All @@ -38,6 +38,37 @@ for ($b = 0; $b < count(BEGIN); $b++) {
}
}
}
echo "\n";

// Mixing PDO transaction API and explicit queries.
foreach (END as $end) {
$db->beginTransaction();
var_dump($db->inTransaction());
$db->exec($end);
var_dump($db->inTransaction());
}

$db->exec('START TRANSACTION');
var_dump($db->inTransaction());
$db->rollBack();
var_dump($db->inTransaction());
$db->exec('START TRANSACTION');
var_dump($db->inTransaction());
$db->commit();
var_dump($db->inTransaction());
echo "\n";

// DDL query causes an implicit commit.
$db->beginTransaction();
var_dump($db->inTransaction());
$db->exec('DROP TABLE IF EXISTS test');
var_dump($db->inTransaction());

// We should be able to start a new transaction after the implicit commit.
$db->beginTransaction();
var_dump($db->inTransaction());
$db->commit();
var_dump($db->inTransaction());

?>
--EXPECT--
Expand Down Expand Up @@ -65,3 +96,17 @@ bool(true)
bool(false)
bool(true)
bool(false)

bool(true)
bool(false)
bool(true)
bool(false)
bool(true)
bool(false)
bool(true)
bool(false)

bool(true)
bool(false)
bool(true)
bool(false)
15 changes: 9 additions & 6 deletions ext/pdo_mysql/tests/pdo_mysql_rollback.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,15 @@ if (false == MySQLPDOTest::detect_transactional_mysql_engine($db))
$db->query('DROP TABLE IF EXISTS test2');
$db->query('CREATE TABLE test2(id INT)');
$num++;
$db->rollBack();
$row = $db->query('SELECT COUNT(*) AS _num FROM test')->fetch(PDO::FETCH_ASSOC);
if ($row['_num'] != $num)
printf("[002] ROLLBACK should have no effect because of the implicit COMMIT
triggered by DROP/CREATE TABLE\n");

try {
$db->rollBack();
$failed = false;
} catch (PDOException $e) {
$failed = true;
}
if (!$failed) {
printf("[003] Rollback should have failed\n");
}

$db->query('DROP TABLE IF EXISTS test2');
$db->query('CREATE TABLE test2(id INT) ENGINE=MyISAM');
Expand Down