diff --git a/UPGRADE.md b/UPGRADE.md index e5402dc492f..d3836188cf9 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,5 +1,9 @@ # Upgrade to 3.0 +## BC BREAK `Statement::execute()` with redundant parameters. + +Similarly to the drivers based on `pdo_pgsql` and `pdo_sqlsrv`, `OCI8Statement::execute()` and `MySQLiStatement::execute()` do not longer ignore redundant parameters. + ## BC BREAK: `Doctrine\DBAL\Types\Type::getDefaultLength()` removed The `Doctrine\DBAL\Types\Type::getDefaultLength()` method has been removed as it served no purpose. diff --git a/lib/Doctrine/DBAL/Driver/Mysqli/MysqliStatement.php b/lib/Doctrine/DBAL/Driver/Mysqli/MysqliStatement.php index 0414524447e..f8557e4490e 100644 --- a/lib/Doctrine/DBAL/Driver/Mysqli/MysqliStatement.php +++ b/lib/Doctrine/DBAL/Driver/Mysqli/MysqliStatement.php @@ -46,7 +46,7 @@ class MysqliStatement implements IteratorAggregate, Statement protected $_rowBindedValues; /** @var mixed[] */ - protected $_bindedValues; + protected $_bindedValues = []; /** @var string */ protected $types; @@ -138,18 +138,18 @@ public function bindValue($param, $value, $type = ParameterType::STRING) */ public function execute($params = null) { - if ($this->_bindedValues !== null) { - if ($params !== null) { - if (! $this->_bindValues($params)) { - throw new MysqliException($this->_stmt->error, $this->_stmt->errno); - } - } else { - [$types, $values, $streams] = $this->separateBoundValues(); - if (! $this->_stmt->bind_param($types, ...$values)) { - throw new MysqliException($this->_stmt->error, $this->_stmt->sqlstate, $this->_stmt->errno); - } - $this->sendLongData($streams); + if ($params !== null && count($params) > 0) { + if (! $this->_bindValues($params)) { + throw new MysqliException($this->_stmt->error, $this->_stmt->errno); } + } else { + [$types, $values, $streams] = $this->separateBoundValues(); + + if (count($values) > 0 && ! $this->_stmt->bind_param($types, ...$values)) { + throw new MysqliException($this->_stmt->error, $this->_stmt->sqlstate, $this->_stmt->errno); + } + + $this->sendLongData($streams); } if (! $this->_stmt->execute()) { diff --git a/lib/Doctrine/DBAL/Driver/OCI8/OCI8Statement.php b/lib/Doctrine/DBAL/Driver/OCI8/OCI8Statement.php index 1a8de4a9c4b..8a7e1736730 100644 --- a/lib/Doctrine/DBAL/Driver/OCI8/OCI8Statement.php +++ b/lib/Doctrine/DBAL/Driver/OCI8/OCI8Statement.php @@ -359,9 +359,13 @@ public function execute($params = null) $hasZeroIndex = array_key_exists(0, $params); foreach ($params as $key => $val) { if ($hasZeroIndex && is_numeric($key)) { - $this->bindValue($key + 1, $val); + $param = $key + 1; } else { - $this->bindValue($key, $val); + $param = $key; + } + + if (! $this->bindValue($param, $val)) { + throw OCI8Exception::fromErrorInfo($this->errorInfo()); } } } diff --git a/tests/Doctrine/Tests/DBAL/Driver/OCI8/OCI8StatementTest.php b/tests/Doctrine/Tests/DBAL/Driver/OCI8/OCI8StatementTest.php index b51f3bf2f5b..61b88f003fb 100644 --- a/tests/Doctrine/Tests/DBAL/Driver/OCI8/OCI8StatementTest.php +++ b/tests/Doctrine/Tests/DBAL/Driver/OCI8/OCI8StatementTest.php @@ -41,24 +41,15 @@ public function testExecute(array $params) ->disableOriginalConstructor() ->getMock(); - $statement->expects($this->at(0)) - ->method('bindValue') - ->with( - $this->equalTo(1), - $this->equalTo($params[0]) - ); - $statement->expects($this->at(1)) - ->method('bindValue') - ->with( - $this->equalTo(2), - $this->equalTo($params[1]) - ); - $statement->expects($this->at(2)) - ->method('bindValue') - ->with( - $this->equalTo(3), - $this->equalTo($params[2]) - ); + foreach ($params as $index => $value) { + $statement->expects($this->at($index)) + ->method('bindValue') + ->with( + $this->equalTo($index + 1), + $this->equalTo($value) + ) + ->willReturn(true); + } // can't pass to constructor since we don't have a real database handle, // but execute must check the connection for the executeMode diff --git a/tests/Doctrine/Tests/DBAL/Functional/StatementTest.php b/tests/Doctrine/Tests/DBAL/Functional/StatementTest.php index 7aa374b461b..f545ebba2e2 100644 --- a/tests/Doctrine/Tests/DBAL/Functional/StatementTest.php +++ b/tests/Doctrine/Tests/DBAL/Functional/StatementTest.php @@ -2,6 +2,7 @@ namespace Doctrine\Tests\DBAL\Functional; +use Doctrine\DBAL\DBALException; use Doctrine\DBAL\Driver\Statement; use Doctrine\DBAL\FetchMode; use Doctrine\DBAL\ParameterType; @@ -9,6 +10,7 @@ use Doctrine\DBAL\Types\Type; use Doctrine\Tests\DbalFunctionalTestCase; use function base64_decode; +use function sprintf; use function stream_get_contents; class StatementTest extends DbalFunctionalTestCase @@ -292,4 +294,40 @@ public function testFetchInColumnMode() : void self::assertEquals(1, $result); } + + public function testExecWithRedundantParameters() : void + { + $driver = $this->connection->getDriver()->getName(); + + switch ($driver) { + case 'pdo_mysql': + case 'pdo_oracle': + case 'pdo_sqlsrv': + self::markTestSkipped(sprintf( + 'PDOStatement::execute() implemented in the "%s" driver does not report redundant parameters', + $driver + )); + + return; + case 'ibm_db2': + self::markTestSkipped('db2_execute() does not report redundant parameters'); + + return; + case 'sqlsrv': + self::markTestSkipped('sqlsrv_prepare() does not report redundant parameters'); + + return; + } + + $platform = $this->connection->getDatabasePlatform(); + $query = $platform->getDummySelectSQL(); + $stmt = $this->connection->prepare($query); + + // we want to make sure the exception is thrown by the DBAL code, not by PHPUnit due to a PHP-level error, + // but the wrapper connection wraps everything in a DBAL exception + $this->iniSet('error_reporting', 0); + + self::expectException(DBALException::class); + $stmt->execute([null]); + } }