Make serialized SQL executors forward compatible

[greg0ire]

The idea here is that instead of having a backward compatibility layer
in the next major branch, we can have a forward compatibility layer in
this branch.

Closes #11026

[greg0ire]

This weirdness can be understood by reading https://www.phpinternalsbook.com/php5/classes_objects/serialization.html

[SenseException]

I did once some PHP shenanigans by creating private/protected stdClass properties just for fun. 😂

[SenseException]

Next to the review I did also a check of the occurrences of used _sqlStatements in case if some were forgotten, but you get anyone of those rascals. 😁

[SenseException]

Even though _sqlStatements is referencing sqlStatements this test would be more understandable if you compare the contents of both properties.

Suggested change

	$this->assertIsString($r->getValue($unserialized->getSqlExecutor()));
	$this->assertSame($r->getValue($unserialized->getSqlExecutor()), $unserialized->getSqlExecutor()->getSqlStatements());

Maybe you want to create a separated test method for your changes instead of re-using testSerializeParserResult(). One tests the serialized ParserResult, the other your forward compatiility.

[derrabus]

Can you add a the new format to ParserResultSerializationTest::provideSerializedSingleSelectResults()?

[PhilETaylor]

upgraded from doctrine/orm: '2.17.x-dev#16028e4fd3202e21cecbd87071e46b40f7eda8bf'
to doctrine/orm: '2.17.x-dev#f08159eb87ec2feb1aaf3a25bf138704a052edd7'

and now code like this:

return $this->createQueryBuilder('a')
            ->where('a.lastVisitDate > :d')
            ->andWhere('a.username != :un')
            ->setParameter('d', (new DateTime('now'))->modify($period))
            ->setParameter('un', "rabbit")
            ->orderBy('a.lastVisitDate', 'desc')
            ->setMaxResults($limit)
            ->getQuery()
            ->getResult();

results in Exception

Doctrine\DBAL\Connection::executeQuery(): Argument #1 ($sql) must be of type string, null given, called in /var/www/current/vendor/doctrine/orm/lib/Doctrine/ORM/Query/Exec/SingleSelectExecutor.php on line 33

[greg0ire]

Thanks for reporting this, I will take a look.

[greg0ire]

@PhilETaylor fixed

[PhilETaylor]

Awesome - thanks.

[alcohol]

@greg0ire you sure it was fixed? Our production environment just broke while rolling out with 2.17.0. See #11063 -- from the comments it looks like I'm not the only one who ran into this also.

[greg0ire]

@alcohol yes, I'm sure, see #11048, I added a test that covers what's reported here. What you see in #11063 must be slightly different I suppose.