Connection::executeQuery return type

[pauliusrum]

BC Break Report

Q	A
BC Break	yes
Version	2.8

Previous behavior

In 2.7 Doctrine\DBAL\Connection::executeQuery return type was Doctrine\DBAL\Driver\Statement. https://github.com/doctrine/dbal/blob/2.7/lib/Doctrine/DBAL/Connection.php#L906

Current behavior

In 2.8 Doctrine\DBAL\Connection::executeQuery return type is Doctrine\DBAL\Driver\ResultStatement
https://github.com/doctrine/dbal/blob/2.8/lib/Doctrine/DBAL/Connection.php#L911

[morozov]

@pauliusrum is there a practical impact introduced by this change?

[pauliusrum]

When you upgrade to 2.8 and you use ResultStatement interface method after running executeQuery PHPStan analysis fail.

[morozov]

@pauliusrum given that Doctrine\DBAL\Driver\Statement is a subtype of Doctrine\DBAL\Driver\ResultStatement:

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

Line 35 in df43437

interface Statement extends ResultStatement

The former can be transparently used instead of the latter according to the LSP. What kind of failures are you seeing?

[pauliusrum]

@pauliusrum given that Doctrine\DBAL\Driver\Statement is a subtype of Doctrine\DBAL\Driver\ResultStatement:
dbal/lib/Doctrine/DBAL/Driver/Statement.php

Line 35 in df43437

interface Statement extends ResultStatement
The former can be transparently used instead of the latter according to the LSP. What kind of failures are you seeing?

It's the other way around. In 2.8 executeQuery returns the base interface ResultStatement not the subtype Statement. So if one was using rowCount in 2.7 there was no error since Statement interface has that method. Now in 2.8 it will show an error because ResultStatement interface does not have a method rowCount.

[Majkl578]

Hi,
first, this isn't a BC break, the behavior hasn't changed, only the documented return type which was incorrect (done as part of #3025).
The reason is simple: Doctrine\DBAL\Connection::execute(Cache)Query() supports caching, and for cached results, Doctrine\DBAL\Cache\ResultCacheStatement is returned. This class is a subclass of Doctrine\DBAL\Driver\ResultStatement, but not Doctrine\DBAL\Driver\Statement, therefore Doctrine\DBAL\Connection::executeCacheQuery() could not return Doctrine\DBAL\Driver\Statement.
This indicates that your code will likely break if you use caching.

You can verify this by the following patch and running the test suite:

diff --git a/lib/Doctrine/DBAL/Connection.php b/lib/Doctrine/DBAL/Connection.php
index c1cb8acf5..6a9457f55 100644
--- a/lib/Doctrine/DBAL/Connection.php
+++ b/lib/Doctrine/DBAL/Connection.php
@@ -915,7 +915,8 @@ class Connection implements DriverConnection
     public function executeQuery($query, array $params = [], $types = [], QueryCacheProfile $qcp = null)
     {
         if ($qcp !== null) {
-            return $this->executeCacheQuery($query, $params, $types, $qcp);
+            $stmt = $this->executeCacheQuery($query, $params, $types, $qcp);
+            goto end;
         }
 
         $this->connect();
@@ -948,7 +949,9 @@ class Connection implements DriverConnection
         if ($logger) {
             $logger->stopQuery();
         }
+end:
 
+        assert($stmt instanceof DriverStatement);
         return $stmt;
     }

Will cause assertion error:

Stack trace:
#0 lib/Doctrine/DBAL/Connection.php(954): assert(false, 'assert($stmt in...')
#1 tests/Doctrine/Tests/DBAL/Functional/ResultCacheTest.php(172): Doctrine\DBAL\Connection->executeQuery('SELECT * FROM c...', Array, Array, Object(Doctrine\DBAL\Cache\QueryCacheProfile))
#2 tests/Doctrine/Tests/DBAL/Functional/ResultCacheTest.php(64): Doctrine\Tests\DBAL\Functional\ResultCacheTest->assertCacheNonCacheSelectSameFetchModeAreEqual(Array, 2)

Note that now we have a type issue here:

dbal/lib/Doctrine/DBAL/Connection.php

Line 987 in 5140a64

$stmt = new ResultCacheStatement($this->executeQuery($query, $params, $types), $resultCache, $cacheKey, $realKey, $qcp->getLifetime());

As ResultCacheStatement expects Driver\Statement instead of Driver\ResultStatement, but that can't happen as Connection::executeQuery() is invoked without cache profile.

Closing as invalid.

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.