Statement::fetchColumn() will throw an exception in the case of inval…

…id index
doctrine · Dec 1, 2018 · 27ea5d3 · 27ea5d3
1 parent 4d40426
commit 27ea5d3
Show file tree

Hide file tree

Showing 11 changed files with 81 additions and 9 deletions.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -1,5 +1,9 @@
 # Upgrade to 2.9
 
+## MINOR BC BREAK `Statement::fetchColumn()`.
+
+Similarly to `PDOStatement::fetchColumn()` DBAL statements will throw an exception in case of an invalid column index.
+
 ## Deprecated `Statement::fetchColumn()` with an invalid index
 
 Calls to `Statement::fetchColumn()` with an invalid column index currently return `NULL`. In the future, such calls will result in a exception.

diff --git a/lib/Doctrine/DBAL/Cache/ArrayStatement.php b/lib/Doctrine/DBAL/Cache/ArrayStatement.php
@@ -3,6 +3,7 @@
 namespace Doctrine\DBAL\Cache;
 
 use ArrayIterator;
+use Doctrine\DBAL\DBALException;
 use Doctrine\DBAL\Driver\ResultStatement;
 use Doctrine\DBAL\FetchMode;
 use InvalidArgumentException;
@@ -131,7 +132,14 @@ public function fetchColumn($columnIndex = 0)
     {
         $row = $this->fetch(FetchMode::NUMERIC);
 
-        // TODO: verify that return false is the correct behavior
-        return $row[$columnIndex] ?? false;
+        if ($row === false) {
+            return false;
+        }
+
+        if ($columnIndex >= count($row)) {
+            throw DBALException::invalidColumnIndex($columnIndex, count($row));
+        }
+
+        return $row[$columnIndex];
     }
 }
diff --git a/lib/Doctrine/DBAL/Cache/ResultCacheStatement.php b/lib/Doctrine/DBAL/Cache/ResultCacheStatement.php
@@ -4,6 +4,7 @@
 
 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;
@@ -12,6 +13,7 @@
 use PDO;
 use function array_merge;
 use function array_values;
+use function count;
 use function reset;
 
 /**
@@ -176,8 +178,15 @@ public function fetchColumn($columnIndex = 0)
     {
         $row = $this->fetch(FetchMode::NUMERIC);
 
-        // TODO: verify that return false is the correct behavior
-        return $row[$columnIndex] ?? false;
+        if ($row === false) {
+            return false;
+        }
+
+        if ($columnIndex >= count($row)) {
+            throw DBALException::invalidColumnIndex($columnIndex, count($row));
+        }
+
+        return $row[$columnIndex];
     }
 
     /**

diff --git a/lib/Doctrine/DBAL/DBALException.php b/lib/Doctrine/DBAL/DBALException.php
@@ -282,4 +282,14 @@ public static function typeNotFound($name)
     {
         return new self('Type to be overwritten ' . $name . ' does not exist.');
     }
+
+    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'
+        ));
+    }
 }
diff --git a/lib/Doctrine/DBAL/Driver/IBMDB2/DB2Statement.php b/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;
@@ -19,6 +20,7 @@
 use const DB2_PARAM_FILE;
 use const DB2_PARAM_IN;
 use function array_change_key_case;
+use function count;
 use function db2_bind_param;
 use function db2_execute;
 use function db2_fetch_array;
@@ -347,7 +349,11 @@ public function fetchColumn($columnIndex = 0)
             return false;
         }
 
-        return $row[$columnIndex] ?? null;
+        if ($columnIndex >= count($row)) {
+            throw DBALException::invalidColumnIndex($columnIndex, count($row));
+        }
+
+        return $row[$columnIndex];
     }
 
     /**

diff --git a/lib/Doctrine/DBAL/Driver/Mysqli/MysqliStatement.php b/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;
@@ -388,7 +389,11 @@ public function fetchColumn($columnIndex = 0)
             return false;
         }
 
-        return $row[$columnIndex] ?? null;
+        if ($columnIndex >= count($row)) {
+            throw DBALException::invalidColumnIndex($columnIndex, count($row));
+        }
+
+        return $row[$columnIndex];
     }
 
     /**

diff --git a/lib/Doctrine/DBAL/Driver/OCI8/OCI8Statement.php b/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;
@@ -497,7 +498,11 @@ public function fetchColumn($columnIndex = 0)
             return false;
         }
 
-        return $row[$columnIndex] ?? null;
+        if ($columnIndex >= count($row)) {
+            throw DBALException::invalidColumnIndex($columnIndex, count($row));
+        }
+
+        return $row[$columnIndex];
     }
 
     /**

diff --git a/lib/Doctrine/DBAL/Driver/PDOStatement.php b/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 PDO;
@@ -177,6 +178,10 @@ public function fetchAll($fetchMode = null, $fetchArgument = null, $ctorArgs = n
      */
     public function fetchColumn($columnIndex = 0)
     {
+        if ($columnIndex >= $this->columnCount()) {
+            throw DBALException::invalidColumnIndex($columnIndex, $this->rowCount());
+        }
+
         try {
             return parent::fetchColumn($columnIndex);
         } catch (\PDOException $exception) {

diff --git a/lib/Doctrine/DBAL/Driver/SQLAnywhere/SQLAnywhereStatement.php b/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;
@@ -13,6 +14,7 @@
 use stdClass;
 use const SASQL_BOTH;
 use function array_key_exists;
+use function count;
 use function func_get_args;
 use function func_num_args;
 use function gettype;
@@ -279,7 +281,9 @@ public function fetchColumn($columnIndex = 0)
             return false;
         }
 
-        return $row[$columnIndex] ?? null;
+        if ($columnIndex >= count($row)) {
+            throw DBALException::invalidColumnIndex($columnIndex, count($row));
+        }
     }
 
     /**

diff --git a/lib/Doctrine/DBAL/Driver/SQLSrv/SQLSrvStatement.php b/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;
@@ -398,7 +399,11 @@ public function fetchColumn($columnIndex = 0)
             return false;
         }
 
-        return $row[$columnIndex] ?? null;
+        if ($columnIndex >= count($row)) {
+            throw DBALException::invalidColumnIndex($columnIndex, count($row));
+        }
+
+        return $row[$columnIndex];
     }
 
     /**

diff --git 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;
@@ -292,4 +293,14 @@ public function testFetchInColumnMode() : void
 
         self::assertEquals(1, $result);
     }
+
+    public function testFetchColumnNonExistingIndex() : void
+    {
+        $platform = $this->connection->getDatabasePlatform();
+        $query    = $platform->getDummySelectSQL();
+        $stmt     = $this->connection->query($query);
+
+        self::expectException(DBALException::class);
+        $stmt->fetchColumn(1);
+    }
 }