Call to a member function removeQueryCacheProfile() on null

[cmodijk]

Bug Report

Q	A
BC Break	Not sure
Version	2.16.2

Summary

I have create a this example repository to reproduce the error. When i grap this project using PHP 7.4 and al the dependencies in it I get this error.

The first call to the method tries to to connect to a database. When i ignore this and then refresh the page i get this error. It seems to happen after the cache is stored on disk and then it is tried to be reloaded.

On the same error i already found doctrine/DoctrineBundle#1654 and #10684 but this pull request does not fixes it. After debugging when i remove the __unserialize method from lib/Doctrine/ORM/Query/ParserResult.php it seems to work not sure what is going on here.

Current behavior

Results in a error

Call to a member function removeQueryCacheProfile() on null

How to reproduce

git clone git@github.com:JCID/removeQueryCacheProfile.git
cd removeQueryCacheProfile
composer install
symfony serve 

Expected behavior

To not have a error.

[greg0ire]

Reproduced with PHP 8 (you can drop the php 7 constraint for your reproducer).

Here is the full stack trace after I add a return type to getSQLExecutor():

TypeError:
Doctrine\ORM\Query\ParserResult::getSqlExecutor(): Return value must be of type Doctrine\ORM\Query\Exec\AbstractSqlExecutor, null returned

  at vendor/doctrine/orm/lib/Doctrine/ORM/Query/ParserResult.php:94
  at Doctrine\ORM\Query\ParserResult->getSqlExecutor()
     (vendor/doctrine/orm/lib/Doctrine/ORM/Query.php:286)
  at Doctrine\ORM\Query->_doExecute()
     (vendor/doctrine/orm/lib/Doctrine/ORM/AbstractQuery.php:1212)
  at Doctrine\ORM\AbstractQuery->executeIgnoreQueryCache()
     (vendor/doctrine/orm/lib/Doctrine/ORM/AbstractQuery.php:1166)
  at Doctrine\ORM\AbstractQuery->execute()
     (vendor/doctrine/orm/lib/Doctrine/ORM/AbstractQuery.php:901)
  at Doctrine\ORM\AbstractQuery->getResult()
     (src/Controller/TestController.php:29)
  at App\Controller\TestController->__invoke()
     (vendor/symfony/symfony/src/Symfony/Component/HttpKernel/HttpKernel.php:169)
  at Symfony\Component\HttpKernel\HttpKernel->handleRaw()
     (vendor/symfony/symfony/src/Symfony/Component/HttpKernel/HttpKernel.php:81)
  at Symfony\Component\HttpKernel\HttpKernel->handle()
     (vendor/symfony/symfony/src/Symfony/Component/HttpKernel/Kernel.php:201)
  at Symfony\Component\HttpKernel\Kernel->handle()
     (public/index.php:18)    

[cmodijk]

I removed the 7.4 part, i also just noticed that it does not seem to happen when using symfony/symfony^5.4

[cmodijk]

It is also weird that the __unserialize method in \Doctrine\ORM\Query\ParserResult is called with this input. This keys don't align with what the method is getting out of it.

^ array:1 [▼
  "Doctrine\ORM\Query\ParserResult" => array:2 [▼
    "sqlExecutor" => Doctrine\ORM\Query\Exec\SingleSelectExecutor {#253 ▼
      #_sqlStatements: "SELECT t0_.id AS id_0 FROM testEntity t0_ LIMIT 1"
      #queryCacheProfile: null
    }
    "resultSetMapping" => Doctrine\ORM\Query\ResultSetMapping {#250 ▼
      +isMixed: false
      +isSelect: true
      +aliasMap: array:1 [▼
        "testentity" => "App\Entity\TestEntity"
      ]
      +relationMap: []
      +parentAliasMap: []
      +fieldMappings: array:1 [▼
        "id_0" => "id"
      ]
      +scalarMappings: []
      +enumMappings: []
      +typeMappings: []
      +entityMappings: array:1 [▼
        "testentity" => null
      ]
      +metaMappings: []
      +columnOwnerMap: array:1 [▼
        "id_0" => "testentity"
      ]
      +discriminatorColumns: []
      +indexByMap: []
      +declaringClasses: array:1 [▼
        "id_0" => "App\Entity\TestEntity"
      ]
      +isIdentifierColumn: []
      +newObjectMappings: []
      +metadataParameterMapping: []
      +discriminatorParameters: []
    }
  ]
]

[greg0ire]

Here is what it looks like with symfony 5.4 (contains non-printable chars):

array(3) {
  ["
Doctrine\ORM\Query\ParserResult
sqlExecutor"]=>
  object(Doctrine\ORM\Query\Exec\SingleSelectExecutor)#258 (2) {
    ["_sqlStatements":protected]=>
    string(49) "SELECT t0_.id AS id_0 FROM testEntity t0_ LIMIT 1"
    ["queryCacheProfile":protected]=>
    NULL
  }
  ["
Doctrine\ORM\Query\ParserResult
resultSetMapping"]=>
  object(Doctrine\ORM\Query\ResultSetMapping)#261 (19) {
    ["isMixed"]=>
    bool(false)
    ["isSelect"]=>
    bool(true)
    ["aliasMap"]=>
    array(1) {
      ["testentity"]=>
      string(21) "App\Entity\TestEntity"
    }
    ["relationMap"]=>
    array(0) {
    }
    ["parentAliasMap"]=>
    array(0) {
    }
    ["fieldMappings"]=>
    array(1) {
      ["id_0"]=>
      string(2) "id"
    }
    ["scalarMappings"]=>
    array(0) {
    }
    ["enumMappings"]=>
    array(0) {
    }
    ["typeMappings"]=>
    array(0) {
    }
    ["entityMappings"]=>
    array(1) {
      ["testentity"]=>
      NULL
    }
    ["metaMappings"]=>
    array(0) {
    }
    ["columnOwnerMap"]=>
    array(1) {
      ["id_0"]=>
      string(10) "testentity"
    }
    ["discriminatorColumns"]=>
    array(0) {
    }
    ["indexByMap"]=>
    array(0) {
    }
    ["declaringClasses"]=>
    array(1) {
      ["id_0"]=>
      string(21) "App\Entity\TestEntity"
    }
    ["isIdentifierColumn"]=>
    array(0) {
    }
    ["newObjectMappings"]=>
    array(0) {
    }
    ["metadataParameterMapping"]=>
    array(0) {
    }
    ["discriminatorParameters"]=>
    array(0) {
    }
  }
  ["
Doctrine\ORM\Query\ParserResult
parameterMappings"]=>
  array(0) {
  }
}

The method seems to handle 2 different serialization formats, and this would be a 3rd one.

[andrew-demb]

Adding here

orm/lib/Doctrine/ORM/Query/ParserResult.php

Lines 142 to 148 in 858b01f

	foreach (self::LEGACY_PROPERTY_MAPPING as $property => $legacyProperty) {
	$this->$property = $data[sprintf("\0%s\0%s", self::class, $legacyProperty)]
	?? $data[sprintf("\0%s\0%s", self::class, $property)]
	?? $this->$property
	?? null;
	}
	}

an additional key check - ?? $data[self::class][$property] fixes the issue for me (Symfony 4.4, PHP 7.4.29, doctrine/orm 2.16.2)

[greg0ire]

That makes sense… would you mind contributing that? Also, I don't fully understand why this has to do with Symfony… does Symfony decide what serialization format is in use? 🤔

Cc @derrabus , I think we should either revert the addition of the method, or apply @andrew-demb's suggestion.

[cmodijk]

does Symfony decide what serialization format is in use?

I don't believe so it should alway be the same. But this location call's the __unserialize directly which gives the wrong format if you ask me. Maybe it is a bug in the Symfony unserializer?

When i update to Symfony 5.4 the input seems to be correct.

^ array:3 [▼
  "\x00Doctrine\ORM\Query\ParserResult\x00sqlExecutor" => Doctrine\ORM\Query\Exec\SingleSelectExecutor {#258 ▶}
  "\x00Doctrine\ORM\Query\ParserResult\x00resultSetMapping" => Doctrine\ORM\Query\ResultSetMapping {#261 ▶}
  "\x00Doctrine\ORM\Query\ParserResult\x00parameterMappings" => []
]

[cmodijk]

Inside the dumped cache file i just found this diff Symfony seemed to have fixed the dumper. Seems that this is the changed we are looking for symfony/symfony@6d8a363 or symfony/symfony@6ccb85e

     [
         [
-            'Doctrine\\ORM\\Query\\ParserResult' => [
-                'sqlExecutor' => $o[1],
-                'resultSetMapping' => $o[2],
-            ],
+            "\0".'Doctrine\\ORM\\Query\\ParserResult'."\0".'sqlExecutor' => $o[1],
+            "\0".'Doctrine\\ORM\\Query\\ParserResult'."\0".'resultSetMapping' => $o[2],
+            "\0".'Doctrine\\ORM\\Query\\ParserResult'."\0".'parameterMappings' => [],

         ],
     ]

[greg0ire]

@cmodijk thanks for the investigation. Can you try @andrew-demb solution? If it works for you, can you please contribute it? Another "solution" would be to drop support for Symfony 4, which is in security fixes only mode, but that would leave you in a bad situation.

[cmodijk]

I just created #10948. It would leave us in a bad situation if Symfony 4 support is dropped but we are in the process of upgrading all of our old Symfony projects. At this time we disabled the query caching to allow us to upgrade everything else. But the issue here for us is we are not sure how bug this impact is on preformance.

[cmodijk]

Fixed by #10948