Doctrine 2 Doctrine\ORM\Persisters\Entity\BasicEntityPersister::getSelectSQL() method issue

[kbora504]

There seems to be a bug in method public function getSelectSQL($criteria, $assoc = null, $lockMode = null, $limit = null, $offset = null, array $orderBy = null) As per documentation of Doctrine\ORM\Persisters\Entity\EntityPersister the $criteria parameter should be of type array; so, the below code

$conditionSql = ($criteria instanceof Criteria)
            ? $this->getSelectConditionCriteriaSQL($criteria)
            : $this->getSelectConditionSQL($criteria, $assoc);

will never execute $this->getSelectConditionCriteriaSQL($criteria) statement, because it is always of type array. Please fix this, so that it accepts single criteria object too.

++ Exception

#0 C:\projects\data\vendor\doctrine\orm\lib\Doctrine\ORM\Persisters\Entity\BasicEntityPersister.php(1704): Doctrine\ORM\ORMException::unrecognizedField(0)
#1 C:\projects\data\vendor\doctrine\orm\lib\Doctrine\ORM\Persisters\Entity\BasicEntityPersister.php(1582): Doctrine\ORM\Persisters\Entity\BasicEntityPersister->getSelectConditionStatementColumnSQL(0, NULL)
#2 C:\projects\data\vendor\doctrine\orm\lib\Doctrine\ORM\Persisters\Entity\BasicEntityPersister.php(1724): Doctrine\ORM\Persisters\Entity\BasicEntityPersister->getSelectConditionStatementSQL(0, Object(Doctrine\Common\Collections\Criteria), NULL)
#3 C:\projects\data\vendor\doctrine\orm\lib\Doctrine\ORM\Persisters\Entity\BasicEntityPersister.php(1058): Doctrine\ORM\Persisters\Entity\BasicEntityPersister->getSelectConditionSQL(Array, NULL)
#4 C:\projects\data\vendor\doctrine\orm\lib\Doctrine\ORM\Persisters\Entity\BasicEntityPersister.php(882): Doctrine\ORM\Persisters\Entity\BasicEntityPersister->getSelectSQL(Array, NULL, NULL, 20, 0, NULL)
#5 C:\projects\data\vendor\doctrine\orm\lib\Doctrine\ORM\EntityRepository.php(181): Doctrine\ORM\Persisters\Entity\BasicEntityPersister->loadAll(Array, NULL, 20, 0)
#6 C:\projects\data\module\MyModule\src\DAO\DataDao.php(231): Doctrine\ORM\EntityRepository->findBy(Array, NULL, 20, 0)
#7 C:\projects\data\module\MyModule\src\Service\DataService.php(47): MyModule\DAO\DataDao->findData(Object(MyModule\Form\Data\PaginationOptions))
#8 C:\projects\data\module\MyModule\src\Controller\DataController.php(54): MyModule\Service\DataService->findData(Object(MyModule\Form\Data\PaginationOptions))
#9 C:\projects\data\vendor\zendframework\zend-mvc\src\Controller\AbstractActionController.php(78): MyModule\Controller\DataController->dataAction()
#10 C:\projects\data\vendor\zendframework\zend-eventmanager\src\EventManager.php(322): Zend\Mvc\Controller\AbstractActionController->onDispatch(Object(Zend\Mvc\MvcEvent))
#11 C:\projects\data\vendor\zendframework\zend-eventmanager\src\EventManager.php(179): Zend\EventManager\EventManager->triggerListeners(Object(Zend\Mvc\MvcEvent), Object(Closure))
#12 C:\projects\data\vendor\zendframework\zend-mvc\src\Controller\AbstractController.php(106): Zend\EventManager\EventManager->triggerEventUntil(Object(Closure), Object(Zend\Mvc\MvcEvent))
#13 C:\projects\data\vendor\zendframework\zend-mvc\src\DispatchListener.php(138): Zend\Mvc\Controller\AbstractController->dispatch(Object(Zend\Http\PhpEnvironment\Request), Object(Zend\Http\PhpEnvironment\Response))
#14 C:\projects\data\vendor\zendframework\zend-eventmanager\src\EventManager.php(322): Zend\Mvc\DispatchListener->onDispatch(Object(Zend\Mvc\MvcEvent))
#15 C:\projects\data\vendor\zendframework\zend-eventmanager\src\EventManager.php(179): Zend\EventManager\EventManager->triggerListeners(Object(Zend\Mvc\MvcEvent), Object(Closure))
#16 C:\projects\data\vendor\zendframework\zend-mvc\src\Application.php(332): Zend\EventManager\EventManager->triggerEventUntil(Object(Closure), Object(Zend\Mvc\MvcEvent))
#17 C:\projects\data\public\index.php(40): Zend\Mvc\Application->run()
#18 {main}

[Ocramius]

@kbora504 could you please write a failing test case that demonstrates the issue?

[kbora504]

It is very obvious from code written. Just check the passed parameter type is array and we are checking for instanceof Criteria.

[Ocramius]

It doesn't really matter from our perspective: tests verify the behavior and prevent regressions. No tests = no fix/merge ;-)

[kbora504]

@Ocramius I understand your point. But how can I write test case for a satement inside function? :(

[Ocramius]

@kbora504 you got a stack trace there: how did you get to that failure? Can you isolate it and make an example that reproduces the problem? See https://github.com/doctrine/doctrine2/tree/388afb46d0cb3ed0c51332e8df0de9e942c2690b/tests/Doctrine/Tests/ORM/Functional/Ticket for examples