ResultSet: a row taken by fetch() is not handed out again by the iterator

fetch() stored the row in $lastRow, which valid() reads as "the iterator already
has a current row" - so fetch() followed by fetchAll() or foreach replayed the row
the caller had just consumed. The other fetch methods never touched $lastRow, so
after them the iteration carried on correctly, making the behaviour inconsistent
as well as wrong.

$lastRow now belongs to the iterator alone and is filled in by valid(), the only
place that fetches on the iterator's behalf.

Author: dg

src/Database/ResultSet.php

@@ -187,7 +187,15 @@ public function valid(): bool
 			return true;
 		}
 
-		return $this->fetch() !== null;
+		// only a row fetched here belongs to the iterator, one taken by the caller's fetch() is gone;
+		// assigning the result directly would turn the false left by next() into null and lose the rewind guard
+		$row = $this->fetch();
+		if ($row === null) {
+			return false;
+		}
+
+		$this->lastRow = $row;
+		return true;
 	}
 
 
@@ -233,7 +241,7 @@ public function fetch(): ?Row
 		}
 
 		$this->lastRowKey++;
-		return $this->lastRow = Arrays::toObject($data, new Row);
+		return Arrays::toObject($data, new Row);
 	}
 
 

tests/Database/ResultSet.fetch().phpt

@@ -73,3 +73,39 @@ test('returns null for empty result set', function () use ($connection, $driverN
 
 	Assert::null($res->fetch());
 });
+
+
+test('a row taken by fetch() is not handed out again by the iterator', function () use ($connection) {
+	$query = 'SELECT id FROM author ORDER BY id';
+	$ids = fn(array $rows) => array_map(fn($row) => $row->id, $rows);
+
+	$res = $connection->query($query);
+	Assert::same(11, $res->fetch()->id);
+	Assert::same([12, 13], $ids($res->fetchAll()));
+
+	$res = $connection->query($query);
+	$res->fetch();
+	$collected = [];
+	foreach ($res as $row) {
+		$collected[] = $row->id;
+	}
+
+	Assert::same([12, 13], $collected);
+
+	$res = $connection->query($query);
+	while ($res->fetch());
+	Assert::same([], iterator_to_array($res));
+});
+
+
+test('the iterator carries on after the other fetch methods too', function () use ($connection) {
+	$query = 'SELECT id FROM author ORDER BY id';
+
+	$res = $connection->query($query);
+	$res->fetchAssoc();
+	Assert::same([12, 13], array_map(fn($row) => $row->id, $res->fetchAll()));
+
+	$res = $connection->query($query);
+	$res->fetchField();
+	Assert::same([12, 13], array_map(fn($row) => $row->id, $res->fetchAll()));
+});