diff --git a/UPGRADE.md b/UPGRADE.md index f2e30f3ed97..2a383739bc7 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -6,6 +6,14 @@ awareness about deprecated code. - Use of our low-overhead runtime deprecation API, details: https://github.com/doctrine/deprecations/ +# Upgrade to 4.2 + +## Minor BC break: incompatible query cache format + +The query cache format has been changed to address the issue where a cached result with no rows would miss the metadata. +This change is not backwards compatible. If you are using the query cache, you should clear the cache before the +upgrade. + # Upgrade to 4.1 ## Deprecated `TableDiff` methods diff --git a/src/Cache/ArrayResult.php b/src/Cache/ArrayResult.php index 16878c37778..ff54865d343 100644 --- a/src/Cache/ArrayResult.php +++ b/src/Cache/ArrayResult.php @@ -8,24 +8,29 @@ use Doctrine\DBAL\Driver\Result; use Doctrine\DBAL\Exception\InvalidColumnIndex; -use function array_keys; -use function array_values; +use function array_combine; use function count; -use function reset; /** @internal The class is internal to the caching layer implementation. */ final class ArrayResult implements Result { - private readonly int $columnCount; private int $num = 0; - /** @param list> $data */ - public function __construct(private array $data) + /** + * @param list $columnNames The names of the result columns. Must be non-empty. + * @param list> $rows The rows of the result. Each row must have the same number of columns + * as the number of column names. + */ + public function __construct(private readonly array $columnNames, private array $rows) { - $this->columnCount = $data === [] ? 0 : count($data[0]); } public function fetchNumeric(): array|false + { + return $this->fetch(); + } + + public function fetchAssociative(): array|false { $row = $this->fetch(); @@ -33,12 +38,7 @@ public function fetchNumeric(): array|false return false; } - return array_values($row); - } - - public function fetchAssociative(): array|false - { - return $this->fetch(); + return array_combine($this->columnNames, $row); } public function fetchOne(): mixed @@ -49,7 +49,7 @@ public function fetchOne(): mixed return false; } - return reset($row); + return $row[0]; } /** @@ -78,32 +78,31 @@ public function fetchFirstColumn(): array public function rowCount(): int { - return count($this->data); + return count($this->rows); } public function columnCount(): int { - return $this->columnCount; + return count($this->columnNames); } public function getColumnName(int $index): string { - return array_keys($this->data[0] ?? [])[$index] - ?? throw InvalidColumnIndex::new($index); + return $this->columnNames[$index] ?? throw InvalidColumnIndex::new($index); } public function free(): void { - $this->data = []; + $this->rows = []; } - /** @return array|false */ + /** @return list|false */ private function fetch(): array|false { - if (! isset($this->data[$this->num])) { + if (! isset($this->rows[$this->num])) { return false; } - return $this->data[$this->num++]; + return $this->rows[$this->num++]; } } diff --git a/src/Connection.php b/src/Connection.php index 3bb951445ff..f4ba0963307 100644 --- a/src/Connection.php +++ b/src/Connection.php @@ -811,15 +811,24 @@ public function executeCacheQuery(string $sql, array $params, array $types, Quer } if (isset($value[$realKey])) { - return new Result(new ArrayResult($value[$realKey]), $this); + [$columnNames, $rows] = $value[$realKey]; + + return new Result(new ArrayResult($columnNames, $rows), $this); } } else { $value = []; } - $data = $this->fetchAllAssociative($sql, $params, $types); + $result = $this->executeQuery($sql, $params, $types); + + $columnNames = []; + for ($i = 0; $i < $result->columnCount(); $i++) { + $columnNames[] = $result->getColumnName($i); + } + + $rows = $result->fetchAllNumeric(); - $value[$realKey] = $data; + $value[$realKey] = [$columnNames, $rows]; $item->set($value); @@ -830,7 +839,7 @@ public function executeCacheQuery(string $sql, array $params, array $types, Quer $resultCache->save($item); - return new Result(new ArrayResult($data), $this); + return new Result(new ArrayResult($columnNames, $rows), $this); } /** diff --git a/tests/Cache/ArrayResultTest.php b/tests/Cache/ArrayResultTest.php new file mode 100644 index 00000000000..ce293f87d4c --- /dev/null +++ b/tests/Cache/ArrayResultTest.php @@ -0,0 +1,108 @@ +result = new ArrayResult(['username', 'active'], [ + ['jwage', true], + ['romanb', false], + ]); + } + + public function testFree(): void + { + self::assertSame(2, $this->result->rowCount()); + + $this->result->free(); + + self::assertSame(0, $this->result->rowCount()); + } + + public function testColumnCount(): void + { + self::assertSame(2, $this->result->columnCount()); + } + + public function testColumnNames(): void + { + self::assertSame('username', $this->result->getColumnName(0)); + self::assertSame('active', $this->result->getColumnName(1)); + } + + #[TestWith([2])] + #[TestWith([-1])] + public function testColumnNameWithInvalidIndex(int $index): void + { + $this->expectException(InvalidColumnIndex::class); + + $this->result->getColumnName($index); + } + + public function testRowCount(): void + { + self::assertSame(2, $this->result->rowCount()); + } + + public function testFetchAssociative(): void + { + self::assertSame([ + 'username' => 'jwage', + 'active' => true, + ], $this->result->fetchAssociative()); + } + + public function testFetchNumeric(): void + { + self::assertSame(['jwage', true], $this->result->fetchNumeric()); + } + + public function testFetchOne(): void + { + self::assertSame('jwage', $this->result->fetchOne()); + self::assertSame('romanb', $this->result->fetchOne()); + } + + public function testFetchAllAssociative(): void + { + self::assertSame([ + [ + 'username' => 'jwage', + 'active' => true, + ], + [ + 'username' => 'romanb', + 'active' => false, + ], + ], $this->result->fetchAllAssociative()); + } + + public function testEmptyResult(): void + { + $result = new ArrayResult(['a'], []); + self::assertSame('a', $result->getColumnName(0)); + } + + public function testSameColumnNames(): void + { + $result = new ArrayResult(['a', 'a'], [[1, 2]]); + + self::assertSame('a', $result->getColumnName(0)); + self::assertSame('a', $result->getColumnName(1)); + + self::assertEquals([1, 2], $result->fetchNumeric()); + } +} diff --git a/tests/Cache/ArrayStatementTest.php b/tests/Cache/ArrayStatementTest.php deleted file mode 100644 index 19ec81cea77..00000000000 --- a/tests/Cache/ArrayStatementTest.php +++ /dev/null @@ -1,104 +0,0 @@ -> */ - private array $users = [ - [ - 'username' => 'jwage', - 'active' => true, - ], - [ - 'username' => 'romanb', - 'active' => false, - ], - ]; - - public function testCloseCursor(): void - { - $statement = $this->createTestArrayStatement(); - - self::assertSame(2, $statement->rowCount()); - - $statement->free(); - - self::assertSame(0, $statement->rowCount()); - } - - public function testColumnCount(): void - { - $statement = $this->createTestArrayStatement(); - - self::assertSame(2, $statement->columnCount()); - } - - public function testColumnNames(): void - { - $statement = $this->createTestArrayStatement(); - - self::assertSame('username', $statement->getColumnName(0)); - self::assertSame('active', $statement->getColumnName(1)); - } - - #[TestWith([2])] - #[TestWith([-1])] - public function testColumnNameWithInvalidIndex(int $index): void - { - $statement = $this->createTestArrayStatement(); - $this->expectException(InvalidColumnIndex::class); - - $statement->getColumnName($index); - } - - public function testRowCount(): void - { - $statement = $this->createTestArrayStatement(); - - self::assertSame(2, $statement->rowCount()); - } - - public function testFetchAssociative(): void - { - $statement = $this->createTestArrayStatement(); - - self::assertSame($this->users[0], $statement->fetchAssociative()); - } - - public function testFetchNumeric(): void - { - $statement = $this->createTestArrayStatement(); - - self::assertSame(array_values($this->users[0]), $statement->fetchNumeric()); - } - - public function testFetchOne(): void - { - $statement = $this->createTestArrayStatement(); - - self::assertSame('jwage', $statement->fetchOne()); - self::assertSame('romanb', $statement->fetchOne()); - } - - public function testFetchAll(): void - { - $statement = $this->createTestArrayStatement(); - - self::assertSame($this->users, $statement->fetchAllAssociative()); - } - - private function createTestArrayStatement(): ArrayResult - { - return new ArrayResult($this->users); - } -} diff --git a/tests/Connection/CachedQueryTest.php b/tests/Connection/CachedQueryTest.php index ae82a3c8c25..7c908a15541 100644 --- a/tests/Connection/CachedQueryTest.php +++ b/tests/Connection/CachedQueryTest.php @@ -16,31 +16,30 @@ class CachedQueryTest extends TestCase public function testCachedQuery(): void { $cache = new ArrayAdapter(); - $data = [['foo' => 'bar']]; - $connection = $this->createConnection(1, $data); + $connection = $this->createConnection(1, ['foo'], [['bar']]); $qcp = new QueryCacheProfile(0, __FUNCTION__, $cache); - self::assertSame($data, $connection->executeCacheQuery('SELECT 1', [], [], $qcp)->fetchAllAssociative()); - self::assertSame($data, $connection->executeCacheQuery('SELECT 1', [], [], $qcp)->fetchAllAssociative()); + self::assertSame([['foo' => 'bar']], $connection->executeCacheQuery('SELECT 1', [], [], $qcp) + ->fetchAllAssociative()); + self::assertSame([['foo' => 'bar']], $connection->executeCacheQuery('SELECT 1', [], [], $qcp) + ->fetchAllAssociative()); self::assertCount(1, $cache->getItem(__FUNCTION__)->get()); } public function testCachedQueryWithChangedImplementationIsExecutedTwice(): void { - $data = [['baz' => 'qux']]; + $connection = $this->createConnection(2, ['baz'], [['qux']]); - $connection = $this->createConnection(2, $data); - - self::assertSame($data, $connection->executeCacheQuery( + self::assertSame([['baz' => 'qux']], $connection->executeCacheQuery( 'SELECT 1', [], [], new QueryCacheProfile(0, __FUNCTION__, new ArrayAdapter()), )->fetchAllAssociative()); - self::assertSame($data, $connection->executeCacheQuery( + self::assertSame([['baz' => 'qux']], $connection->executeCacheQuery( 'SELECT 1', [], [], @@ -48,14 +47,17 @@ public function testCachedQueryWithChangedImplementationIsExecutedTwice(): void )->fetchAllAssociative()); } - /** @param list> $data */ - private function createConnection(int $expectedQueryCount, array $data): Connection + /** + * @param list $columnNames + * @param list> $rows + */ + private function createConnection(int $expectedQueryCount, array $columnNames, array $rows): Connection { $connection = $this->createMock(Driver\Connection::class); $connection->expects(self::exactly($expectedQueryCount)) ->method('query') - ->willReturnCallback(static function () use ($data): ArrayResult { - return new ArrayResult($data); + ->willReturnCallback(static function () use ($columnNames, $rows): ArrayResult { + return new ArrayResult($columnNames, $rows); }); $driver = $this->createMock(Driver::class); diff --git a/tests/ConnectionTest.php b/tests/ConnectionTest.php index 5f966599099..29922683d4e 100644 --- a/tests/ConnectionTest.php +++ b/tests/ConnectionTest.php @@ -569,7 +569,7 @@ public function testConnectionParamsArePassedToTheQueryCacheProfileInExecuteCach { $cacheItemMock = $this->createMock(CacheItemInterface::class); $cacheItemMock->method('isHit')->willReturn(true); - $cacheItemMock->method('get')->willReturn(['realKey' => []]); + $cacheItemMock->method('get')->willReturn(['realKey' => [[], []]]); $resultCacheMock = $this->createMock(CacheItemPoolInterface::class); diff --git a/tests/Functional/Connection/FetchTest.php b/tests/Functional/Connection/FetchTest.php index cc274eac45a..dc2b849f6ee 100644 --- a/tests/Functional/Connection/FetchTest.php +++ b/tests/Functional/Connection/FetchTest.php @@ -17,19 +17,10 @@ class FetchTest extends FunctionalTestCase public function setUp(): void { - $this->query = TestUtil::generateResultSetQuery([ - [ - 'a' => 'foo', - 'b' => 1, - ], - [ - 'a' => 'bar', - 'b' => 2, - ], - [ - 'a' => 'baz', - 'b' => 3, - ], + $this->query = TestUtil::generateResultSetQuery(['a', 'b'], [ + ['foo', 1], + ['bar', 2], + ['baz', 3], ], $this->connection->getDatabasePlatform()); } diff --git a/tests/TestUtil.php b/tests/TestUtil.php index 354ce38ef87..ba54b07fd4e 100644 --- a/tests/TestUtil.php +++ b/tests/TestUtil.php @@ -19,9 +19,7 @@ use InvalidArgumentException; use PHPUnit\Framework\Assert; -use function array_keys; use function array_map; -use function array_values; use function assert; use function extension_loaded; use function file_exists; @@ -265,11 +263,13 @@ public static function isDriverOneOf(string ...$names): bool /** * Generates a query that will return the given rows without the need to create a temporary table. * - * @param array> $rows + * @param list $columnNames The names of the result columns. Must be non-empty. + * @param list> $rows The rows of the result. Each row must have the same number of columns + * as the number of column names. */ - public static function generateResultSetQuery(array $rows, AbstractPlatform $platform): string + public static function generateResultSetQuery(array $columnNames, array $rows, AbstractPlatform $platform): string { - return implode(' UNION ALL ', array_map(static function (array $row) use ($platform): string { + return implode(' UNION ALL ', array_map(static function (array $row) use ($columnNames, $platform): string { return $platform->getDummySelectSQL( implode(', ', array_map(static function (string $column, $value) use ($platform): string { if (is_string($value)) { @@ -277,7 +277,7 @@ public static function generateResultSetQuery(array $rows, AbstractPlatform $pla } return $value . ' ' . $platform->quoteIdentifier($column); - }, array_keys($row), array_values($row))), + }, $columnNames, $row)), ); }, $rows)); }