From cc1f5a0286e4ffcf1026766419232b403f03b1f7 Mon Sep 17 00:00:00 2001 From: Alex Pott <alex.a.pott@googlemail.com> Date: Sun, 12 Jan 2025 09:58:20 +0000 Subject: [PATCH] Issue #3490200 by mondrake, daffie, alexpott: StatementPrefetchIterator::fetchCol fails silently if the column index is out of range --- .../Core/Database/StatementInterface.php | 3 + .../Database/StatementPrefetchIterator.php | 27 ++--- .../KernelTests/Core/Database/FetchTest.php | 98 ++++++++++++++++++- 3 files changed, 113 insertions(+), 15 deletions(-) diff --git a/core/lib/Drupal/Core/Database/StatementInterface.php b/core/lib/Drupal/Core/Database/StatementInterface.php index b3cccd1c0c03..f39426db4358 100644 --- a/core/lib/Drupal/Core/Database/StatementInterface.php +++ b/core/lib/Drupal/Core/Database/StatementInterface.php @@ -168,6 +168,9 @@ public function fetchAll($mode = NULL, $column_index = NULL, $constructor_argume * * @return array * An indexed array, or an empty array if there is no result set. + * + * @throws \ValueError + * If there is at least one record but the column index is not defined. */ public function fetchCol($index = 0); diff --git a/core/lib/Drupal/Core/Database/StatementPrefetchIterator.php b/core/lib/Drupal/Core/Database/StatementPrefetchIterator.php index ba968be11a5a..8d6a1b3906dc 100644 --- a/core/lib/Drupal/Core/Database/StatementPrefetchIterator.php +++ b/core/lib/Drupal/Core/Database/StatementPrefetchIterator.php @@ -256,20 +256,24 @@ public function fetch($fetch_style = NULL, $cursor_orientation = \PDO::FETCH_ORI } /** - * {@inheritdoc} + * @deprecated in drupal:11.2.0 and is removed from drupal:12.0.0. Use + * ::fetchField() instead. + * + * @see https://www.drupal.org/node/3490312 */ public function fetchColumn($index = 0) { - if ($row = $this->fetch(\PDO::FETCH_ASSOC)) { - return $this->assocToColumn($row, $this->columnNames, $index); - } - return FALSE; + @trigger_error(__METHOD__ . '() is deprecated in drupal:11.2.0 and is removed from drupal:12.0.0. Use ::fetchField() instead. See https://www.drupal.org/node/3490312', E_USER_DEPRECATED); + return $this->fetchField($index); } /** * {@inheritdoc} */ public function fetchField($index = 0) { - return $this->fetchColumn($index); + if ($row = $this->fetch(\PDO::FETCH_ASSOC)) { + return $this->assocToColumn($row, $this->columnNames, $index); + } + return FALSE; } /** @@ -319,14 +323,11 @@ public function fetchAll($mode = NULL, $column_index = NULL, $constructor_argume * {@inheritdoc} */ public function fetchCol($index = 0) { - if (isset($this->columnNames[$index])) { - $result = []; - while ($row = $this->fetch(\PDO::FETCH_ASSOC)) { - $result[] = $row[$this->columnNames[$index]]; - } - return $result; + $result = []; + while (($columnValue = $this->fetchField($index)) !== FALSE) { + $result[] = $columnValue; } - return []; + return $result; } /** diff --git a/core/tests/Drupal/KernelTests/Core/Database/FetchTest.php b/core/tests/Drupal/KernelTests/Core/Database/FetchTest.php index 286d51865d1f..62c87e32d130 100644 --- a/core/tests/Drupal/KernelTests/Core/Database/FetchTest.php +++ b/core/tests/Drupal/KernelTests/Core/Database/FetchTest.php @@ -6,7 +6,9 @@ use Drupal\Core\Database\RowCountException; use Drupal\Core\Database\StatementInterface; +use Drupal\Core\Database\StatementPrefetchIterator; use Drupal\Tests\system\Functional\Database\FakeRecord; +use PHPUnit\Framework\Attributes\IgnoreDeprecations; /** * Tests the Database system's various fetch capabilities. @@ -33,6 +35,28 @@ public function testQueryFetchDefault(): void { $this->assertCount(1, $records, 'There is only one record.'); } + /** + * Confirms that we can fetch a single column value. + */ + public function testQueryFetchColumn(): void { + $statement = $this->connection + ->query('SELECT [name] FROM {test} WHERE [age] = :age', [':age' => 25]); + $statement->setFetchMode(\PDO::FETCH_COLUMN, 0); + $this->assertSame('John', $statement->fetch()); + } + + /** + * Confirms that an out of range index throws an error. + */ + public function testQueryFetchColumnOutOfRange(): void { + $this->expectException(\ValueError::class); + $this->expectExceptionMessage('Invalid column index'); + $statement = $this->connection + ->query('SELECT [name] FROM {test} WHERE [age] = :age', [':age' => 25]); + $statement->setFetchMode(\PDO::FETCH_COLUMN, 200); + $statement->fetch(); + } + /** * Confirms that we can fetch a record to an object explicitly. */ @@ -161,6 +185,62 @@ public function testQueryFetchCol(): void { } } + /** + * Tests ::fetchCol() for edge values returned. + */ + public function testQueryFetchColEdgeCases(): void { + $this->connection->insert('test_null') + ->fields([ + 'name' => 'Foo', + 'age' => 0, + ]) + ->execute(); + + $this->connection->insert('test_null') + ->fields([ + 'name' => 'Bar', + 'age' => NULL, + ]) + ->execute(); + + $this->connection->insert('test_null') + ->fields([ + 'name' => 'Qux', + 'age' => (int) FALSE, + ]) + ->execute(); + + $statement = $this->connection->select('test_null') + ->fields('test_null', ['age']) + ->orderBy('id') + ->execute(); + + $this->assertSame(['0', NULL, '0'], $statement->fetchCol()); + + // Additional fetch returns FALSE since the result set is finished. + $this->assertFalse($statement->fetchField()); + } + + /** + * Confirms that an out of range index in fetchCol() throws an error. + */ + public function testQueryFetchColIndexOutOfRange(): void { + $this->expectException(\ValueError::class); + $this->expectExceptionMessage('Invalid column index'); + $this->connection + ->query('SELECT [name] FROM {test} WHERE [age] > :age', [':age' => 25]) + ->fetchCol(200); + } + + /** + * Confirms empty result set prevails on out of range index in fetchCol(). + */ + public function testQueryFetchColIndexOutOfRangeOnEmptyResultSet(): void { + $this->assertSame([], $this->connection + ->query('SELECT [name] FROM {test} WHERE [age] = :age', [':age' => 255]) + ->fetchCol(200)); + } + /** * Tests ::fetchAllAssoc(). */ @@ -274,7 +354,7 @@ public function testQueryFetchFieldEdgeCases(): void { } /** - * Confirms that an out of range index throws an error. + * Confirms that an out of range index in fetchField() throws an error. */ public function testQueryFetchFieldIndexOutOfRange(): void { $this->expectException(\ValueError::class); @@ -285,7 +365,7 @@ public function testQueryFetchFieldIndexOutOfRange(): void { } /** - * Confirms that empty result set prevails on out of range index. + * Confirms empty result set prevails on out of range index in fetchField(). */ public function testQueryFetchFieldIndexOutOfRangeOnEmptyResultSet(): void { $this->assertFalse($this->connection @@ -308,4 +388,18 @@ public function testRowCount(): void { $this->assertTrue($exception, 'Exception was thrown'); } + /** + * Confirms deprecation of StatementPrefetchIterator::fetchColumn(). + */ + #[IgnoreDeprecations] + public function testLegacyFetchColumn(): void { + $statement = $this->connection->query('SELECT [name] FROM {test} WHERE [age] = :age', [':age' => 25]); + if (!$statement instanceof StatementPrefetchIterator) { + $this->markTestSkipped('This test is for StatementPrefetchIterator statements only.'); + } + + $this->expectDeprecation('Drupal\Core\Database\StatementPrefetchIterator::fetchColumn() is deprecated in drupal:11.2.0 and is removed from drupal:12.0.0. Use ::fetchField() instead. See https://www.drupal.org/node/3490312'); + $statement->fetchColumn(); + } + } -- GitLab