Dropped handling of one-based numeric arrays of parameters in Stateme…

…nt::execute()
doctrine · Jan 12, 2020 · d0fe572 · d0fe572
1 parent bfc8bb9
commit d0fe572
Show file tree

Hide file tree

Showing 5 changed files with 7 additions and 78 deletions.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -1,5 +1,9 @@
 # Upgrade to 3.0
 
+## BC BREAK: Dropped handling of one-based numeric arrays of parameters in `Statement::execute()`
+
+The statement implementations no longer detect whether `$params` is a zero- or one-based array. A zero-based numeric array is expected.
+
 ## BC BREAK: `ServerInfoAwareConnection::requiresQueryForServerVersion()` is removed.
 
 The `ServerInfoAwareConnection::requiresQueryForServerVersion()` method has been removed as an implementation detail which is the same for almost all supported drivers.

diff --git a/lib/Doctrine/DBAL/Driver/OCI8/OCI8Statement.php b/lib/Doctrine/DBAL/Driver/OCI8/OCI8Statement.php
@@ -351,10 +351,8 @@ public function columnCount() : int
     public function execute(?array $params = null) : void
     {
         if ($params) {
-            $hasZeroIndex = array_key_exists(0, $params);
-
             foreach ($params as $key => $val) {
-                if ($hasZeroIndex && is_int($key)) {
+                if (is_int($key)) {
                     $param = $key + 1;
                 } else {
                     $param = $key;

diff --git a/lib/Doctrine/DBAL/Driver/SQLSrv/SQLSrvStatement.php b/lib/Doctrine/DBAL/Driver/SQLSrv/SQLSrvStatement.php
@@ -204,10 +204,8 @@ public function columnCount() : int
     public function execute(?array $params = null) : void
     {
         if ($params) {
-            $hasZeroIndex = array_key_exists(0, $params);
-
             foreach ($params as $key => $val) {
-                if ($hasZeroIndex && is_int($key)) {
+                if (is_int($key)) {
                     $this->bindValue($key + 1, $val);
                 } else {
                     $this->bindValue($key, $val);

diff --git a/lib/Doctrine/DBAL/Driver/Statement.php b/lib/Doctrine/DBAL/Driver/Statement.php
@@ -68,7 +68,7 @@ public function bindParam($param, &$variable, int $type = ParameterType::STRING,
      * if any, of their associated parameter markers or pass an array of input-only
      * parameter values.
      *
-     * @param mixed[]|null $params An array of values with as many elements as there are
+     * @param mixed[]|null $params A numeric array of values with as many elements as there are
      *                             bound parameters in the SQL statement being executed.
      *
      * @throws DriverException

diff --git a/tests/Doctrine/Tests/DBAL/Driver/OCI8/OCI8StatementTest.php b/tests/Doctrine/Tests/DBAL/Driver/OCI8/OCI8StatementTest.php
@@ -4,15 +4,10 @@
 
 namespace Doctrine\Tests\DBAL\Driver\OCI8;
 
-use Doctrine\DBAL\Driver\OCI8\OCI8Connection;
 use Doctrine\DBAL\Driver\OCI8\OCI8Exception;
 use Doctrine\DBAL\Driver\OCI8\OCI8Statement;
 use Doctrine\Tests\DbalTestCase;
-use PHPUnit\Framework\MockObject\MockObject;
-use ReflectionProperty;
-use const OCI_NO_AUTO_COMMIT;
 use function extension_loaded;
-use function fopen;
 
 class OCI8StatementTest extends DbalTestCase
 {
@@ -25,72 +20,6 @@ protected function setUp() : void
         parent::setUp();
     }
 
-    /**
-     * This scenario shows that when the first parameter is not null
-     * it properly sets $hasZeroIndex to 1 and calls bindValue starting at 1.
-     *
-     * This also verifies that the statement will check with the connection to
-     * see what the current execution mode is.
-     *
-     * The expected exception is due to oci_execute failing due to no valid connection.
-     *
-     * @param mixed[] $params
-     *
-     * @dataProvider executeDataProvider
-     */
-    public function testExecute(array $params) : void
-    {
-        /** @var OCI8Statement|MockObject $statement */
-        $statement = $this->getMockBuilder(OCI8Statement::class)
-            ->onlyMethods(['bindValue'])
-            ->disableOriginalConstructor()
-            ->getMock();
-
-        foreach ($params as $index => $value) {
-            $statement->expects($this->at($index))
-                ->method('bindValue')
-                ->with(
-                    $this->equalTo($index + 1),
-                    $this->equalTo($value)
-                );
-        }
-
-        // can't pass to constructor since we don't have a real database handle,
-        // but execute must check the connection for the executeMode
-        $conn = $this->createMock(OCI8Connection::class);
-        $conn->expects($this->once())
-            ->method('getExecuteMode')
-            ->willReturn(OCI_NO_AUTO_COMMIT);
-
-        $connectionReflection = new ReflectionProperty($statement, '_conn');
-        $connectionReflection->setAccessible(true);
-        $connectionReflection->setValue($statement, $conn);
-
-        $handleReflection = new ReflectionProperty($statement, '_sth');
-        $handleReflection->setAccessible(true);
-        $handleReflection->setValue($statement, fopen('php://temp', 'r'));
-
-        $this->expectException(OCI8Exception::class);
-        $statement->execute($params);
-    }
-
-    /**
-     * @return array<int, array<int, mixed>>
-     */
-    public static function executeDataProvider() : iterable
-    {
-        return [
-            // $hasZeroIndex = isset($params[0]); == true
-            [
-                [0 => 'test', 1 => null, 2 => 'value'],
-            ],
-            // $hasZeroIndex = isset($params[0]); == false
-            [
-                [0 => null, 1 => 'test', 2 => 'value'],
-            ],
-        ];
-    }
-
     /**
      * @dataProvider nonTerminatedLiteralProvider
      */