Merge pull request #137 from Unlink/columns-cache-fix

Columns cache fix

Author: dg

src/Database/Table/ActiveRow.php

@@ -323,7 +323,7 @@ public function accessColumn($key, $selectColumn = TRUE)
 	{
 		if ($this->table->accessColumn($key, $selectColumn) && !$this->dataRefreshed) {
 			if (!isset($this->table[$this->getSignature()])) {
-				throw new Nette\InvalidStateException('Database refetch failed; row does not exist!');
+				throw new Nette\InvalidStateException("Database refetch failed; row with signature '{$this->getSignature()}' does not exist!");
 			}
 			$this->data = $this->table[$this->getSignature()]->data;
 			$this->dataRefreshed = TRUE;

src/Database/Table/Selection.php

@@ -59,9 +59,6 @@ class Selection implements \Iterator, IRowContainer, \ArrayAccess, \Countable
 	/** @var string */
 	protected $generalCacheKey;
 
-	/** @var array */
-	protected $generalCacheTraceKey;
-
 	/** @var string */
 	protected $specificCacheKey;
 
@@ -618,20 +615,20 @@ protected function query($query)
 	}
 
 
-	protected function emptyResultSet($saveCache = TRUE, $deleteRererencedCache = TRUE)
+	protected function emptyResultSet($clearCache = TRUE, $deleteRererencedCache = TRUE)
 	{
-		if ($this->rows !== NULL && $saveCache) {
+		if ($this->rows !== NULL && $clearCache) {
 			$this->saveCacheState();
 		}
 
-		if ($saveCache) {
-			// null only if missing some column
-			$this->generalCacheTraceKey = NULL;
+		if ($clearCache) {
+			// not null in case of missing some column
+			$this->previousAccessedColumns = NULL;
+			$this->generalCacheKey = NULL;
 		}
 
 		$this->rows = NULL;
 		$this->specificCacheKey = NULL;
-		$this->generalCacheKey = NULL;
 		$this->refCache['referencingPrototype'] = [];
 		if ($deleteRererencedCache) {
 			$this->refCache['referenced'] = [];
@@ -687,15 +684,12 @@ protected function getGeneralCacheKey()
 		}
 
 		$key = [__CLASS__, $this->name, $this->sqlBuilder->getConditions()];
-		if (!$this->generalCacheTraceKey) {
-			$trace = [];
-			foreach (debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS) as $item) {
-				$trace[] = isset($item['file'], $item['line']) ? $item['file'] . $item['line'] : NULL;
-			};
-			$this->generalCacheTraceKey = $trace;
-		}
+		$trace = [];
+		foreach (debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS) as $item) {
+			$trace[] = isset($item['file'], $item['line']) ? $item['file'] . $item['line'] : NULL;
+		};
 
-		$key[] = $this->generalCacheTraceKey;
+		$key[] = $trace;
 		return $this->generalCacheKey = md5(serialize($key));
 	}
 
@@ -735,8 +729,6 @@ public function accessColumn($key, $selectColumn = TRUE)
 		}
 
 		if ($selectColumn && $this->previousAccessedColumns && ($key === NULL || !isset($this->previousAccessedColumns[$key])) && !$this->sqlBuilder->getSelect()) {
-			$this->previousAccessedColumns = [];
-
 			if ($this->sqlBuilder->getLimit()) {
 				$generalCacheKey = $this->generalCacheKey;
 				$sqlBuilder = $this->sqlBuilder;
@@ -753,10 +745,12 @@ public function accessColumn($key, $selectColumn = TRUE)
 				$this->wherePrimary($primaryValues);
 
 				$this->generalCacheKey = $generalCacheKey;
+				$this->previousAccessedColumns = [];
 				$this->execute();
 				$this->sqlBuilder = $sqlBuilder;
 			} else {
 				$this->emptyResultSet(FALSE);
+				$this->previousAccessedColumns = [];
 				$this->execute();
 			}
 
@@ -1018,7 +1012,9 @@ public function key()
 
 	public function next()
 	{
-		next($this->keys);
+		do {
+			next($this->keys);
+		} while (($key = current($this->keys)) !== FALSE && !isset($this->data[$key]));
 	}
 
 

tests/Database/Table/Table.cache.phpt

@@ -185,3 +185,35 @@ test(function () use ($context) { // Test saving the union of needed cols, the s
 		['id', 'author_id', 'translator_id', 'title'],
 	], $cols);
 });
+
+
+test(function () use ($context) { // Test multiple use of same selection
+	$sql = [];
+	$context->getConnection()->onQuery[] = function($_, $result) use (& $sql) {
+		$sql[] = $result->getQueryString();
+	};
+
+	for ($i = 0; $i < 3; $i += 1) {
+		$bookSelection = $context->table('book');
+		count($bookSelection);
+
+		foreach ($bookSelection->where('author_id = ?', 11) as $book) {
+			$book->title;
+			if ($i>=1) {
+				$book->translator_id;
+			}
+		}
+		$bookSelection->__destruct();
+	}
+
+	Assert::same([
+		reformat('SELECT * FROM [book]'), //First round
+		reformat('SELECT * FROM [book] WHERE ([author_id] = 11)'),
+		reformat('SELECT [id] FROM [book]'), //Second round
+		reformat('SELECT [id], [title] FROM [book] WHERE ([author_id] = 11)'),
+		reformat('SELECT * FROM [book] WHERE ([author_id] = 11)'), //Missing translator_id
+		reformat('SELECT [id] FROM [book]'), //Third round
+		reformat('SELECT [id], [title], [translator_id] FROM [book] WHERE ([author_id] = 11)'),
+
+	], $sql);
+});

tests/Database/Table/bugs/deleteCacheBug.phpt

@@ -21,7 +21,7 @@ test(function () use ($context) {
 			$book->delete();
 			Assert::exception(function () use ($book) {
 				$book->toArray();
-			}, Nette\InvalidStateException::class, 'Database refetch failed; row does not exist!');
+			}, Nette\InvalidStateException::class, "Database refetch failed; row with signature '1' does not exist!");
 		}
 
 		$booksSelection->__destruct();
@@ -42,13 +42,35 @@ test(function () use ($context) {
 
 			Assert::exception(function () use ($book) {
 				$book->toArray();
-			}, Nette\InvalidStateException::class, 'Database refetch failed; row does not exist!');
+			}, Nette\InvalidStateException::class, "Database refetch failed; row with signature '2' does not exist!");
 		}
 
 		$booksSelection->__destruct();
 	}
 });
 
+test(function () use ($context) {
+	$books = [];
+	for ($i=0; $i<2; $i++) {
+		$booksSelection = $context->table('book')->where('id IN ?', [3,4]);
+		foreach ($booksSelection as $book) {
+			$books[] = $book->id;
+
+			if ($i === 1) {
+				$context->query('DELETE FROM book WHERE id = 4'); //After refetch second row is skipped
+				$book->title; // cause refetch
+			}
+
+			$booksSelection->__destruct();
+		}
+	}
+	Assert::same([
+		3,
+		4,
+		3,
+	], $books);
+});
+
 test(function () use ($context) {
 
 	for ($i=0; $i<2; $i++) {
@@ -60,7 +82,7 @@ test(function () use ($context) {
 			$context->query('DELETE FROM book WHERE id = 3');
 			Assert::exception(function () use ($book) {
 				$book->title;
-			}, Nette\InvalidStateException::class, 'Database refetch failed; row does not exist!');
+			}, Nette\InvalidStateException::class, "Database refetch failed; row with signature '3' does not exist!");
 		}
 
 		$booksSelection->__destruct();