Merge pull request #3417 from morozov/issues/3358

Statement::fetchColumn() will throw an exception in the case of invalid index

UPGRADE.md

 # Upgrade to 3.0
 
+## MINOR BC BREAK `Statement::fetchColumn()` with an invalid index.
+
+Similarly to `PDOStatement::fetchColumn()`, DBAL statements throw an exception in case of an invalid column index.
+
 ## 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.

lib/Doctrine/DBAL/Cache/ArrayStatement.php

@@ -3,10 +3,12 @@
 namespace Doctrine\DBAL\Cache;
 
 use ArrayIterator;
+use Doctrine\DBAL\DBALException;
 use Doctrine\DBAL\Driver\ResultStatement;
 use Doctrine\DBAL\FetchMode;
 use InvalidArgumentException;
 use IteratorAggregate;
+use function array_key_exists;
 use function array_merge;
 use function array_values;
 use function count;
@@ -130,7 +132,14 @@ class ArrayStatement implements IteratorAggregate, ResultStatement
     {
         $row = $this->fetch(FetchMode::NUMERIC);
 
-        // TODO: verify that return false is the correct behavior
-        return $row[$columnIndex] ?? false;
+        if ($row === false) {
+            return false;
+        }
+
+        if (! array_key_exists($columnIndex, $row)) {
+            throw DBALException::invalidColumnIndex($columnIndex, count($row));
+        }
+
+        return $row[$columnIndex];
     }
 }

lib/Doctrine/DBAL/Cache/ResultCacheStatement.php

@@ -4,14 +4,17 @@ namespace Doctrine\DBAL\Cache;
 
 use ArrayIterator;
 use Doctrine\Common\Cache\Cache;
+use Doctrine\DBAL\DBALException;
 use Doctrine\DBAL\Driver\ResultStatement;
 use Doctrine\DBAL\Driver\Statement;
 use Doctrine\DBAL\FetchMode;
 use InvalidArgumentException;
 use IteratorAggregate;
+use function array_key_exists;
 use function array_merge;
 use function array_values;
 use function assert;
+use function count;
 use function reset;
 
 /**
@@ -187,8 +190,15 @@ class ResultCacheStatement implements IteratorAggregate, ResultStatement
     {
         $row = $this->fetch(FetchMode::NUMERIC);
 
-        // TODO: verify that return false is the correct behavior
-        return $row[$columnIndex] ?? false;
+        if ($row === false) {
+            return false;
+        }
+
+        if (! array_key_exists($columnIndex, $row)) {
+            throw DBALException::invalidColumnIndex($columnIndex, count($row));
+        }
+
+        return $row[$columnIndex];
     }
 
     /**

lib/Doctrine/DBAL/DBALException.php

@@ -291,4 +291,14 @@ class DBALException extends Exception
             sprintf('Type of the class %s@%s is already registered.', get_class($type), spl_object_hash($type))
         );
     }
+
+    public static function invalidColumnIndex(int $index, int $count) : self
+    {
+        return new self(sprintf(
+            'Invalid column index %d. The statement result contains %d column%s.',
+            $index,
+            $count,
+            $count === 1 ? '' : 's'
+        ));
+    }
 }

lib/Doctrine/DBAL/Driver/IBMDB2/DB2Statement.php

@@ -2,6 +2,7 @@
 
 namespace Doctrine\DBAL\Driver\IBMDB2;
 
+use Doctrine\DBAL\DBALException;
 use Doctrine\DBAL\Driver\Statement;
 use Doctrine\DBAL\Driver\StatementIterator;
 use Doctrine\DBAL\FetchMode;
@@ -18,6 +19,7 @@ use const DB2_LONG;
 use const DB2_PARAM_FILE;
 use const DB2_PARAM_IN;
 use function array_change_key_case;
+use function array_key_exists;
 use function count;
 use function db2_bind_param;
 use function db2_execute;
@@ -338,7 +340,11 @@ class DB2Statement implements IteratorAggregate, Statement
             return false;
         }
 
-        return $row[$columnIndex] ?? null;
+        if (! array_key_exists($columnIndex, $row)) {
+            throw DBALException::invalidColumnIndex($columnIndex, count($row));
+        }
+
+        return $row[$columnIndex];
     }
 
     /**

lib/Doctrine/DBAL/Driver/Mysqli/MysqliStatement.php

@@ -2,6 +2,7 @@
 
 namespace Doctrine\DBAL\Driver\Mysqli;
 
+use Doctrine\DBAL\DBALException;
 use Doctrine\DBAL\Driver\Statement;
 use Doctrine\DBAL\Driver\StatementIterator;
 use Doctrine\DBAL\Exception\InvalidArgumentException;
@@ -12,6 +13,7 @@ use mysqli;
 use mysqli_stmt;
 use function array_combine;
 use function array_fill;
+use function array_key_exists;
 use function assert;
 use function count;
 use function feof;
@@ -380,7 +382,11 @@ class MysqliStatement implements IteratorAggregate, Statement
             return false;
         }
 
-        return $row[$columnIndex] ?? null;
+        if (! array_key_exists($columnIndex, $row)) {
+            throw DBALException::invalidColumnIndex($columnIndex, count($row));
+        }
+
+        return $row[$columnIndex];
     }
 
     /**

lib/Doctrine/DBAL/Driver/OCI8/OCI8Statement.php

@@ -2,6 +2,7 @@
 
 namespace Doctrine\DBAL\Driver\OCI8;
 
+use Doctrine\DBAL\DBALException;
 use Doctrine\DBAL\Driver\Statement;
 use Doctrine\DBAL\Driver\StatementIterator;
 use Doctrine\DBAL\FetchMode;
@@ -521,7 +522,11 @@ class OCI8Statement implements IteratorAggregate, Statement
             return false;
         }
 
-        return $row[$columnIndex] ?? null;
+        if (! array_key_exists($columnIndex, $row)) {
+            throw DBALException::invalidColumnIndex($columnIndex, count($row));
+        }
+
+        return $row[$columnIndex];
     }
 
     /**

lib/Doctrine/DBAL/Driver/PDOStatement.php

@@ -2,6 +2,7 @@
 
 namespace Doctrine\DBAL\Driver;
 
+use Doctrine\DBAL\DBALException;
 use Doctrine\DBAL\FetchMode;
 use Doctrine\DBAL\ParameterType;
 use IteratorAggregate;
@@ -193,7 +194,17 @@ class PDOStatement implements IteratorAggregate, Statement
     public function fetchColumn($columnIndex = 0)
     {
         try {
-            return $this->stmt->fetchColumn($columnIndex);
+            $value = $this->stmt->fetchColumn($columnIndex);
+
+            if ($value === null) {
+                $columnCount = $this->columnCount();
+
+                if ($columnIndex < 0 || $columnIndex >= $columnCount) {
+                    throw DBALException::invalidColumnIndex($columnIndex, $columnCount);
+                }
+            }
+
+            return $value;
         } catch (\PDOException $exception) {
             throw new PDOException($exception);
         }

lib/Doctrine/DBAL/Driver/SQLAnywhere/SQLAnywhereStatement.php

@@ -2,6 +2,7 @@
 
 namespace Doctrine\DBAL\Driver\SQLAnywhere;
 
+use Doctrine\DBAL\DBALException;
 use Doctrine\DBAL\Driver\Statement;
 use Doctrine\DBAL\Driver\StatementIterator;
 use Doctrine\DBAL\FetchMode;
@@ -284,7 +285,9 @@ class SQLAnywhereStatement implements IteratorAggregate, Statement
             return false;
         }
 
-        return $row[$columnIndex] ?? null;
+        if (! array_key_exists($columnIndex, $row)) {
+            throw DBALException::invalidColumnIndex($columnIndex, count($row));
+        }
     }
 
     /**

lib/Doctrine/DBAL/Driver/SQLSrv/SQLSrvStatement.php

@@ -2,6 +2,7 @@
 
 namespace Doctrine\DBAL\Driver\SQLSrv;
 
+use Doctrine\DBAL\DBALException;
 use Doctrine\DBAL\Driver\Statement;
 use Doctrine\DBAL\Driver\StatementIterator;
 use Doctrine\DBAL\FetchMode;
@@ -412,7 +413,11 @@ class SQLSrvStatement implements IteratorAggregate, Statement
             return false;
         }
 
-        return $row[$columnIndex] ?? null;
+        if (! array_key_exists($columnIndex, $row)) {
+            throw DBALException::invalidColumnIndex($columnIndex, count($row));
+        }
+
+        return $row[$columnIndex];
     }
 
     /**

phpstan.neon.dist

@@ -62,3 +62,8 @@ parameters:
 
         # https://github.com/doctrine/dbal/issues/3237
         - '~^Call to an undefined method Doctrine\\DBAL\\Driver\\PDOStatement::nextRowset\(\)~'
+
+        # https://github.com/phpstan/phpstan/pull/1886
+        -
+            message: '~^Strict comparison using === between string|false and null will always evaluate to false\.~'
+            path: %currentWorkingDirectory%/lib/Doctrine/DBAL/Driver/PDOStatement.php

tests/Doctrine/Tests/DBAL/Functional/StatementTest.php

@@ -3,6 +3,7 @@
 namespace Doctrine\Tests\DBAL\Functional;
 
 use Doctrine\DBAL\DBALException;
+use Doctrine\DBAL\Driver\PDOConnection;
 use Doctrine\DBAL\Driver\PDOOracle\Driver as PDOOracleDriver;
 use Doctrine\DBAL\Driver\Statement;
 use Doctrine\DBAL\FetchMode;
@@ -357,4 +358,34 @@ EOF
         self::expectException(DBALException::class);
         $stmt->execute([null]);
     }
+
+    /**
+     * @throws DBALException
+     *
+     * @dataProvider nonExistingIndexProvider
+     */
+    public function testFetchColumnNonExistingIndex(int $index) : void
+    {
+        if ($this->connection->getWrappedConnection() instanceof PDOConnection) {
+            $this->markTestSkipped('PDO supports this behavior natively but throws a different exception');
+        }
+
+        $platform = $this->connection->getDatabasePlatform();
+        $query    = $platform->getDummySelectSQL();
+        $stmt     = $this->connection->query($query);
+
+        self::expectException(DBALException::class);
+        $stmt->fetchColumn($index);
+    }
+
+    /**
+     * @return mixed[][]
+     */
+    public static function nonExistingIndexProvider() : iterable
+    {
+        return [
+            [1],
+            [-1],
+        ];
+    }
 }