From 62ef14f7118f6557637b428508569dc60cfdac6e Mon Sep 17 00:00:00 2001 From: NightjarNZ Date: Thu, 11 Oct 2018 00:02:12 +1300 Subject: [PATCH] FIX correct nesting level mismatches causing errors Transactions that used more than one level would cause errors if there were consecutive calls to start a transaction - because each query executed would clear the flag indicating that a transaction was already in progress. The comment for the logic to reset the nesting level on a query was indicating that DDL (data definition language) would not work within a transaction. This is untrue, and the module itself uses a transaction to alter table or field names. So this function has been converted to a no-op, deprecated to be removed in version 3 of this module. It is also no longer called upon each query. There have been some maintenance tidyups around this area also by abstracting the nested transaction flag manipulations into protected functions. --- code/SQLite3Database.php | 61 ++++++++++++++++++++++++++-------------- 1 file changed, 40 insertions(+), 21 deletions(-) diff --git a/code/SQLite3Database.php b/code/SQLite3Database.php index 006bc54..a85a6a3 100644 --- a/code/SQLite3Database.php +++ b/code/SQLite3Database.php @@ -470,12 +470,12 @@ class SQLite3Database extends Database public function transactionStart($transaction_mode = false, $session_characteristics = false) { - if ($this->transactionNesting > 0) { - $this->transactionSavepoint('NESTEDTRANSACTION' . $this->transactionNesting); + if ($this->transactionDepth()) { + $this->transactionSavepoint($this->getTransactionSavepointName()); } else { $this->query('BEGIN'); } - ++$this->transactionNesting; + $this->transactionDepthIncrease(); } public function transactionSavepoint($savepoint) @@ -483,6 +483,18 @@ class SQLite3Database extends Database $this->query("SAVEPOINT \"$savepoint\""); } + /** + * Fetch the name of the current savepoint + * {@see transactionDepth} should be greater than zero + * or the name will be invalid (because there are none). + * + * @return string + */ + protected function getTransactionSavepointName() + { + return 'NESTEDTRANSACTION' . $this->transactionDepth(); + } + public function transactionRollback($savepoint = false) { // Named transaction @@ -492,13 +504,13 @@ class SQLite3Database extends Database } // Fail if transaction isn't available - if (!$this->transactionNesting) { + if (!$this->transactionDepth()) { return false; } - --$this->transactionNesting; - if ($this->transactionNesting > 0) { - $this->transactionRollback('NESTEDTRANSACTION' . $this->transactionNesting); + $this->transactionDepthDecrease(); + if ($this->transactionDepth()) { + $this->transactionRollback($this->getTransactionSavepointName()); } else { $this->query('ROLLBACK;'); } @@ -513,17 +525,30 @@ class SQLite3Database extends Database public function transactionEnd($chain = false) { // Fail if transaction isn't available - if (!$this->transactionNesting) { + if (!$this->transactionDepth()) { return false; } - --$this->transactionNesting; - if ($this->transactionNesting <= 0) { - $this->transactionNesting = 0; - $this->query('COMMIT;'); - } + $this->query('COMMIT;'); + $this->transactionDepthDecrease(); return true; } + /** + * Increase the nested transaction level by one + */ + protected function transactionDepthIncrease() + { + ++$this->transactionNesting; + } + + /** + * Decrease the nested transaction level by one + */ + protected function transactionDepthDecrease() + { + --$this->transactionNesting; + } + /** * In error condition, set transactionNesting to zero */ @@ -534,28 +559,22 @@ class SQLite3Database extends Database public function query($sql, $errorLevel = E_USER_ERROR) { - $this->inspectQuery($sql); return parent::query($sql, $errorLevel); } public function preparedQuery($sql, $parameters, $errorLevel = E_USER_ERROR) { - $this->inspectQuery($sql); return parent::preparedQuery($sql, $parameters, $errorLevel); } /** * Inspect a SQL query prior to execution - * + * @deprecated 2..3 * @param string $sql */ protected function inspectQuery($sql) { - // Any DDL discards transactions. - $isDDL = $this->getConnector()->isQueryDDL($sql); - if ($isDDL) { - $this->resetTransactionNesting(); - } + // no-op } public function clearTable($table)