Make caching layer not rely on closeCursor()

[morozov]

Q	A
Type	improvement
BC Break	no

The current behavior, although it's documented in the form of a test, is counterintuitive:

dbal/tests/Doctrine/Tests/DBAL/Functional/ResultCacheTest.php

Line 136 in c08d3bc

public function testDontCloseNoCache() : void

This test and the fact that all other tests call $stmt->closeCursor() imply that the statement result will be only cached if the statement cursor is closed.

The meaning of closeCursor() has nothing to deal with caching:

dbal/lib/Doctrine/DBAL/Driver/ResultStatement.php

Lines 13 to 18 in c08d3bc

	/**
	* Closes the cursor, enabling the statement to be executed again.
	*
	* @return bool TRUE on success or FALSE on failure.
	*/
	public function closeCursor();

It should be used in certain environments when not the entire result set is fetched in order to prevent errors like this:

SQLSTATE[HY000]: General error: 2014 Cannot execute queries while other unbuffered queries are active

In the case when the full result set is fetched, nothing should prevent the caching layer from caching the result. It happens at the end of fetchAll() or iterative fetch() operations.

[greg0ire]

Great idea! I used the caching layer a few weeks ago and had a hard time making it work, until I read the docs more carefully. I think they should be changed, namely https://github.com/doctrine/dbal/blob/2.11.x/docs/en/reference/caching.rst

[beberlei]

I haven't seen this code for a long while, but does this ever read from the cache? fetchAll immediately delegates to the nested statement for example.

[morozov]

I haven't seen this code for a long while, but does this ever read from the cache? fetchAll immediately delegates to the nested statement for example.

This class is responsible only for reading from the underlying statement and storing the result in the cache if there was no hit. Reading from cache is implemented in Connection::executeCacheQuery():

dbal/lib/Doctrine/DBAL/Connection.php

Lines 1204 to 1207 in acf84c3

	[$cacheKey, $realKey] = $qcp->generateCacheKeys($query, $params, $types, $connectionParams);
	// fetch the row pointers entry
	$data = $resultCache->fetch($cacheKey);

[morozov]

[…] I read the docs more carefully. I think they should be changed […]

Thanks for pointing out, @greg0ire. Actually, the following bit can be also removed from the documentation as of #3070:

dbal/docs/en/reference/caching.rst

Lines 48 to 50 in 96f8cab

	.. warning::
	When using the cache layer not all fetch modes are supported. See the code of the `ResultCacheStatement <https://github.com/doctrine/dbal/blob/master/src/Cache/ResultCacheStatement.php>`_ for details.

[greg0ire]

Please squash your commits unless the squash button is enabled.

[morozov]

This had to be rebased due to a conflict with #4050.