From 9e8a950f2eba86a655b7446735049596b9cf03ac Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Fri, 18 Nov 2011 17:29:31 +0100 Subject: [PATCH] DBAL-171 - Fix bug where params where resorted but types where not in DQL Query --- lib/Doctrine/ORM/Query.php | 89 +++++++++-------- lib/Doctrine/ORM/Query/QueryException.php | 7 +- .../Tests/ORM/Functional/QueryTest.php | 98 +++++++++++++------ 3 files changed, 121 insertions(+), 73 deletions(-) diff --git a/lib/Doctrine/ORM/Query.php b/lib/Doctrine/ORM/Query.php index ac1eb75d7..7fbafce7b 100644 --- a/lib/Doctrine/ORM/Query.php +++ b/lib/Doctrine/ORM/Query.php @@ -44,28 +44,28 @@ final class Query extends AbstractQuery * is called. */ const STATE_DIRTY = 2; - + /* Query HINTS */ /** * The refresh hint turns any query into a refresh query with the result that * any local changes in entities are overridden with the fetched values. - * + * * @var string */ const HINT_REFRESH = 'doctrine.refresh'; - - + + /** * Internal hint: is set to the proxy entity that is currently triggered for loading - * + * * @var string */ const HINT_REFRESH_ENTITY = 'doctrine.refresh.entity'; - + /** * The forcePartialLoad query hint forces a particular query to return * partial objects. - * + * * @var string * @todo Rename: HINT_OPTIMIZE */ @@ -73,9 +73,9 @@ final class Query extends AbstractQuery /** * The includeMetaColumns query hint causes meta columns like foreign keys and * discriminator columns to be selected and returned as part of the query result. - * + * * This hint does only apply to non-object queries. - * + * * @var string */ const HINT_INCLUDE_META_COLUMNS = 'doctrine.includeMetaColumns'; @@ -122,12 +122,12 @@ final class Query extends AbstractQuery * @var Doctrine\ORM\Query\ParserResult The parser result that holds DQL => SQL information. */ private $_parserResult; - + /** * @var integer The first result to return (the "offset"). */ private $_firstResult = null; - + /** * @var integer The maximum number of results to return (the "limit"). */ @@ -147,7 +147,7 @@ final class Query extends AbstractQuery * @var int Query Cache lifetime. */ private $_queryCacheTTL; - + /** * @var boolean Whether to use a query cache, if available. Defaults to TRUE. */ @@ -191,7 +191,7 @@ final class Query extends AbstractQuery /** * Parses the DQL query, if necessary, and stores the parser result. - * + * * Note: Populates $this->_parserResult as a side-effect. * * @return Doctrine\ORM\Query\ParserResult @@ -201,12 +201,12 @@ final class Query extends AbstractQuery if ($this->_state === self::STATE_CLEAN) { return $this->_parserResult; } - + // Check query cache. if ($this->_useQueryCache && ($queryCache = $this->getQueryCacheDriver())) { $hash = $this->_getQueryCacheId(); $cached = $this->_expireQueryCache ? false : $queryCache->fetch($hash); - + if ($cached === false) { // Cache miss. $parser = new Parser($this); @@ -220,9 +220,9 @@ final class Query extends AbstractQuery $parser = new Parser($this); $this->_parserResult = $parser->parse(); } - + $this->_state = self::STATE_CLEAN; - + return $this->_parserResult; } @@ -244,55 +244,62 @@ final class Query extends AbstractQuery } list($sqlParams, $types) = $this->processParameterMappings($paramMappings); - + if ($this->_resultSetMapping === null) { $this->_resultSetMapping = $this->_parserResult->getResultSetMapping(); } return $executor->execute($this->_em->getConnection(), $sqlParams, $types); } - + /** * Processes query parameter mappings - * + * * @param array $paramMappings * @return array */ private function processParameterMappings($paramMappings) { $sqlParams = $types = array(); - + foreach ($this->_params as $key => $value) { if ( ! isset($paramMappings[$key])) { throw QueryException::unknownParameter($key); } - + if (isset($this->_paramTypes[$key])) { foreach ($paramMappings[$key] as $position) { $types[$position] = $this->_paramTypes[$key]; } } - + $sqlPositions = $paramMappings[$key]; $value = array_values($this->processParameterValue($value)); $countValue = count($value); - + for ($i = 0, $l = count($sqlPositions); $i < $l; $i++) { $sqlParams[$sqlPositions[$i]] = $value[($i % $countValue)]; } } - + + if (count($sqlParams) != count($types)) { + throw QueryException::parameterTypeMissmatch(); + } + if ($sqlParams) { ksort($sqlParams); $sqlParams = array_values($sqlParams); + + ksort($types); + $types = array_values($types); } return array($sqlParams, $types); } - + /** * Process an individual parameter value - * + * * @param mixed $value * @return array */ @@ -308,7 +315,7 @@ final class Query extends AbstractQuery } return array($value); - + case is_object($value) && $this->_em->getMetadataFactory()->hasMetadataFor(get_class($value)): if ($this->_em->getUnitOfWork()->getEntityState($value) === UnitOfWork::STATE_MANAGED) { return array_values($this->_em->getUnitOfWork()->getEntityIdentifier($value)); @@ -317,7 +324,7 @@ final class Query extends AbstractQuery $class = $this->_em->getClassMetadata(get_class($value)); return array_values($class->getIdentifierValues($value)); - + default: return array($value); } @@ -334,10 +341,10 @@ final class Query extends AbstractQuery $this->_queryCache = $queryCache; return $this; } - + /** * Defines whether the query should make use of a query cache, if available. - * + * * @param boolean $bool * @return @return Query This query instance. */ @@ -471,7 +478,7 @@ final class Query extends AbstractQuery { return stripos($this->getDQL(), $dql) === false ? false : true; } - + /** * Sets the position of the first result to retrieve (the "offset"). * @@ -484,21 +491,21 @@ final class Query extends AbstractQuery $this->_state = self::STATE_DIRTY; return $this; } - + /** * Gets the position of the first result the query object was set to retrieve (the "offset"). * Returns NULL if {@link setFirstResult} was not applied to this query. - * + * * @return integer The position of the first result. */ public function getFirstResult() { return $this->_firstResult; } - + /** * Sets the maximum number of results to retrieve (the "limit"). - * + * * @param integer $maxResults * @return Query This query object. */ @@ -508,11 +515,11 @@ final class Query extends AbstractQuery $this->_state = self::STATE_DIRTY; return $this; } - + /** * Gets the maximum number of results the query object was set to retrieve (the "limit"). * Returns NULL if {@link setMaxResults} was not applied to this query. - * + * * @return integer Maximum number of results. */ public function getMaxResults() @@ -533,7 +540,7 @@ final class Query extends AbstractQuery $this->setHint(self::HINT_INTERNAL_ITERATION, true); return parent::iterate($params, $hydrationMode); } - + /** * {@inheritdoc} */ @@ -542,7 +549,7 @@ final class Query extends AbstractQuery $this->_state = self::STATE_DIRTY; return parent::setHint($name, $value); } - + /** * {@inheritdoc} */ @@ -597,7 +604,7 @@ final class Query extends AbstractQuery ksort($this->_hints); return md5( - $this->getDql() . var_export($this->_hints, true) . + $this->getDql() . var_export($this->_hints, true) . '&firstResult=' . $this->_firstResult . '&maxResult=' . $this->_maxResults . '&hydrationMode='.$this->_hydrationMode.'DOCTRINE_QUERY_CACHE_SALT' ); diff --git a/lib/Doctrine/ORM/Query/QueryException.php b/lib/Doctrine/ORM/Query/QueryException.php index b9ab2bb07..f581ecc56 100644 --- a/lib/Doctrine/ORM/Query/QueryException.php +++ b/lib/Doctrine/ORM/Query/QueryException.php @@ -77,6 +77,11 @@ class QueryException extends \Doctrine\ORM\ORMException return new self("Invalid parameter: token ".$key." is not defined in the query."); } + public static function parameterTypeMissmatch() + { + return new self("DQL Query parameter and type numbers missmatch, but have to be exactly equal."); + } + public static function invalidPathExpression($pathExpr) { return new self( @@ -140,7 +145,7 @@ class QueryException extends \Doctrine\ORM\ORMException "in the query." ); } - + public static function instanceOfUnrelatedClass($className, $rootClass) { return new self("Cannot check if a child of '" . $rootClass . "' is instanceof '" . $className . "', " . diff --git a/tests/Doctrine/Tests/ORM/Functional/QueryTest.php b/tests/Doctrine/Tests/ORM/Functional/QueryTest.php index 596d929f6..687771e83 100644 --- a/tests/Doctrine/Tests/ORM/Functional/QueryTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/QueryTest.php @@ -34,9 +34,9 @@ class QueryTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->_em->clear(); $query = $this->_em->createQuery("select u, upper(u.name) from Doctrine\Tests\Models\CMS\CmsUser u where u.username = 'gblanco'"); - + $result = $query->getResult(); - + $this->assertEquals(1, count($result)); $this->assertInstanceOf('Doctrine\Tests\Models\CMS\CmsUser', $result[0][0]); $this->assertEquals('Guilherme', $result[0][0]->name); @@ -109,7 +109,7 @@ class QueryTest extends \Doctrine\Tests\OrmFunctionalTestCase $q = $this->_em->createQuery('SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE u.username = ?0'); $q->setParameter(0, 'jwage'); $user = $q->getSingleResult(); - + $this->assertNotNull($user); } @@ -216,7 +216,7 @@ class QueryTest extends \Doctrine\Tests\OrmFunctionalTestCase $identityMap = $this->_em->getUnitOfWork()->getIdentityMap(); $identityMapCount = count($identityMap['Doctrine\Tests\Models\CMS\CmsArticle']); $this->assertTrue($identityMapCount>$iteratedCount); - + $iteratedCount++; } @@ -235,7 +235,7 @@ class QueryTest extends \Doctrine\Tests\OrmFunctionalTestCase $query = $this->_em->createQuery("SELECT u, a FROM Doctrine\Tests\Models\CMS\CmsUser u JOIN u.articles a"); $articles = $query->iterate(); } - + /** * @expectedException Doctrine\ORM\NoResultException */ @@ -366,7 +366,7 @@ class QueryTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertInstanceOf('Doctrine\ORM\Proxy\Proxy', $result[0]->user); $this->assertFalse($result[0]->user->__isInitialized__); } - + /** * @group DDC-952 */ @@ -386,11 +386,11 @@ class QueryTest extends \Doctrine\Tests\OrmFunctionalTestCase } $this->_em->flush(); $this->_em->clear(); - + $articles = $this->_em->createQuery('select a from Doctrine\Tests\Models\CMS\CmsArticle a') ->setFetchMode('Doctrine\Tests\Models\CMS\CmsArticle', 'user', ClassMetadata::FETCH_EAGER) ->getResult(); - + $this->assertEquals(10, count($articles)); foreach ($articles AS $article) { $this->assertNotInstanceOf('Doctrine\ORM\Proxy\Proxy', $article); @@ -456,7 +456,43 @@ class QueryTest extends \Doctrine\Tests\OrmFunctionalTestCase $query = $this->_em->createQuery("select u.username from Doctrine\Tests\Models\CMS\CmsUser u where u.username = 'gblanco'"); $this->assertNull($query->getOneOrNullResult(Query::HYDRATE_SCALAR)); } - + + /** + * @group DBAL-171 + */ + public function testParameterOrder() + { + $user = new CmsUser; + $user->name = 'Benjamin'; + $user->username = 'beberlei'; + $user->status = 'developer'; + $this->_em->persist($user); + + $user = new CmsUser; + $user->name = 'Roman'; + $user->username = 'romanb'; + $user->status = 'developer'; + $this->_em->persist($user); + + $user = new CmsUser; + $user->name = 'Jonathan'; + $user->username = 'jwage'; + $user->status = 'developer'; + $this->_em->persist($user); + + $this->_em->flush(); + $this->_em->clear(); + + $query = $this->_em->createQuery("SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE u.status = :a AND u.id IN (:b)"); + $query->setParameters(array( + 'b' => array(1,2,3), + 'a' => 'developer', + )); + $result = $query->getResult(); + + $this->assertEquals(3, count($result)); + } + public function testDqlWithAutoInferOfParameters() { $user = new CmsUser; @@ -464,30 +500,30 @@ class QueryTest extends \Doctrine\Tests\OrmFunctionalTestCase $user->username = 'beberlei'; $user->status = 'developer'; $this->_em->persist($user); - + $user = new CmsUser; $user->name = 'Roman'; $user->username = 'romanb'; $user->status = 'developer'; $this->_em->persist($user); - + $user = new CmsUser; $user->name = 'Jonathan'; $user->username = 'jwage'; $user->status = 'developer'; $this->_em->persist($user); - + $this->_em->flush(); $this->_em->clear(); - + $query = $this->_em->createQuery("SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE u.username IN (?0)"); $query->setParameter(0, array('beberlei', 'jwage')); - + $users = $query->execute(); - + $this->assertEquals(2, count($users)); } - + public function testQueryBuilderWithStringWhereClauseContainingOrAndConditionalPrimary() { $qb = $this->_em->createQueryBuilder(); @@ -495,13 +531,13 @@ class QueryTest extends \Doctrine\Tests\OrmFunctionalTestCase ->from('Doctrine\Tests\Models\CMS\CmsUser', 'u') ->innerJoin('u.articles', 'a') ->where('(u.id = 0) OR (u.id IS NULL)'); - + $query = $qb->getQuery(); $users = $query->execute(); - + $this->assertEquals(0, count($users)); } - + public function testQueryWithArrayOfEntitiesAsParameter() { $userA = new CmsUser; @@ -509,31 +545,31 @@ class QueryTest extends \Doctrine\Tests\OrmFunctionalTestCase $userA->username = 'beberlei'; $userA->status = 'developer'; $this->_em->persist($userA); - + $userB = new CmsUser; $userB->name = 'Roman'; $userB->username = 'romanb'; $userB->status = 'developer'; $this->_em->persist($userB); - + $userC = new CmsUser; $userC->name = 'Jonathan'; $userC->username = 'jwage'; $userC->status = 'developer'; $this->_em->persist($userC); - + $this->_em->flush(); $this->_em->clear(); - + $query = $this->_em->createQuery("SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE u IN (?0) OR u.username = ?1"); $query->setParameter(0, array($userA, $userC)); $query->setParameter(1, 'beberlei'); - + $users = $query->execute(); - + $this->assertEquals(2, count($users)); } - + public function testQueryWithHiddenAsSelectExpression() { $userA = new CmsUser; @@ -541,25 +577,25 @@ class QueryTest extends \Doctrine\Tests\OrmFunctionalTestCase $userA->username = 'beberlei'; $userA->status = 'developer'; $this->_em->persist($userA); - + $userB = new CmsUser; $userB->name = 'Roman'; $userB->username = 'romanb'; $userB->status = 'developer'; $this->_em->persist($userB); - + $userC = new CmsUser; $userC->name = 'Jonathan'; $userC->username = 'jwage'; $userC->status = 'developer'; $this->_em->persist($userC); - + $this->_em->flush(); $this->_em->clear(); - + $query = $this->_em->createQuery("SELECT u, (SELECT COUNT(u2.id) FROM Doctrine\Tests\Models\CMS\CmsUser u2) AS HIDDEN total FROM Doctrine\Tests\Models\CMS\CmsUser u"); $users = $query->execute(); - + $this->assertEquals(3, count($users)); $this->assertInstanceOf('Doctrine\Tests\Models\CMS\CmsUser', $users[0]); }