Handle cached result column names and rows separately (#6504)

|      Q       |   A
|------------- | -----------
| Type         | bug

#### Summary

Prior to #6428, there were two problems with caching results by design:

1. If the result contains no rows, its cached version won't return the
number of columns.
2. If the result contains columns with the same name (e.g. `SELECT a.id,
b.id FROM a, b`), they will clash even if fetched in the numeric mode.

See #6428 (comment)
for reference.

The solution is:
1. Handle column names and rows in the cache result separately. This
way, the column names are available even if there is no data.
2. Store rows as tuples (lists) instead of maps (associative arrays).
This way, even if the result contains multiple columns with the same
name, they can be correctly fetched with `fetchNumeric()`.

**Additional notes**:
1. The changes in `TestUtil::generateResultSetQuery()` were necessary
for the integration testing of the changes in `ArrayResult`. However,
later, I realized that the modified code is database-agnostic, so
running it in integration with each database platform would effectively
test the same code path and would be redundant. I replaced the
integration tests with the unit ones but left the change in `TestUtil`
as it makes the calling code cleaner.
2. `ArrayStatementTest` was renamed to `ArrayResultTest`, which Git
doesn't seem to recognize due to the amount of changes.

**TODO**:
- [x] Decide on the release version. This pull request changes the cache
format, so the users will have to clear their cache during upgrade. I
wouldn't consider it a BC break. We could also migrate the cache at
runtime, but given that this cache can be cleared, I don't think it is
worth the effort.
- [x] Add upgrade documentation.

Author: morozov

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

src/Cache/ArrayResult.php

@@ -8,37 +8,37 @@
 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<array<string, mixed>> $data */
-    public function __construct(private array $data)
+    /**
+     * @param list<string>      $columnNames The names of the result columns. Must be non-empty.
+     * @param list<list<mixed>> $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();
 
         if ($row === 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<string, mixed>|false */
+    /** @return list<mixed>|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++];
     }
 }

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);
     }
 
     /**

tests/Cache/ArrayResultTest.php

@@ -0,0 +1,108 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Tests\Cache;
+
+use Doctrine\DBAL\Cache\ArrayResult;
+use Doctrine\DBAL\Exception\InvalidColumnIndex;
+use PHPUnit\Framework\Attributes\TestWith;
+use PHPUnit\Framework\TestCase;
+
+class ArrayResultTest extends TestCase
+{
+    private ArrayResult $result;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        $this->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());
+    }
+}