From 0e6121e8f508097226ead3aace9ccc41ced2ca70 Mon Sep 17 00:00:00 2001 From: kwiateusz Date: Thu, 21 Jul 2011 12:40:43 +0200 Subject: [PATCH 1/5] Now findByOne really retrieve only one entity adding limit to query --- lib/Doctrine/ORM/EntityRepository.php | 2 +- lib/Doctrine/ORM/Persisters/BasicEntityPersister.php | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/Doctrine/ORM/EntityRepository.php b/lib/Doctrine/ORM/EntityRepository.php index de2689db2..2b2bee7ac 100644 --- a/lib/Doctrine/ORM/EntityRepository.php +++ b/lib/Doctrine/ORM/EntityRepository.php @@ -178,7 +178,7 @@ class EntityRepository implements ObjectRepository */ public function findOneBy(array $criteria) { - return $this->_em->getUnitOfWork()->getEntityPersister($this->_entityName)->load($criteria); + return $this->_em->getUnitOfWork()->getEntityPersister($this->_entityName)->load($criteria, null, null, array(), 0, 1); } /** diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index 19da2e200..c1436ffa2 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -558,13 +558,13 @@ class BasicEntityPersister * a new entity is created. * @param $assoc The association that connects the entity to load to another entity, if any. * @param array $hints Hints for entity creation. - * @param int $lockMode + * @param int $limit Limit number of results * @return object The loaded and managed entity instance or NULL if the entity can not be found. * @todo Check identity map? loadById method? Try to guess whether $criteria is the id? */ - public function load(array $criteria, $entity = null, $assoc = null, array $hints = array(), $lockMode = 0) + public function load(array $criteria, $entity = null, $assoc = null, array $hints = array(), $lockMode = 0, $limit = 0) { - $sql = $this->_getSelectEntitiesSQL($criteria, $assoc, $lockMode); + $sql = $this->_getSelectEntitiesSQL($criteria, $assoc, $lockMode, $limit); list($params, $types) = $this->expandParameters($criteria); $stmt = $this->_conn->executeQuery($sql, $params, $types); From 570799b48db72a762226897ace966a7f71f893dd Mon Sep 17 00:00:00 2001 From: kwiateusz Date: Thu, 21 Jul 2011 04:13:15 -0700 Subject: [PATCH 2/5] Restoring the missing comment --- lib/Doctrine/ORM/Persisters/BasicEntityPersister.php | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index c1436ffa2..1938ad160 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -558,6 +558,7 @@ class BasicEntityPersister * a new entity is created. * @param $assoc The association that connects the entity to load to another entity, if any. * @param array $hints Hints for entity creation. + * @param int $lockMode * @param int $limit Limit number of results * @return object The loaded and managed entity instance or NULL if the entity can not be found. * @todo Check identity map? loadById method? Try to guess whether $criteria is the id? From 4b85d7a68302ca78648ecb8a7e3acc8e6a14821f Mon Sep 17 00:00:00 2001 From: Guilherme Blanco Date: Fri, 22 Jul 2011 11:38:20 -0300 Subject: [PATCH 3/5] Reverted PR #86, which broke our suite. --- lib/Doctrine/ORM/EntityRepository.php | 2 +- lib/Doctrine/ORM/Persisters/BasicEntityPersister.php | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/Doctrine/ORM/EntityRepository.php b/lib/Doctrine/ORM/EntityRepository.php index 2b2bee7ac..de2689db2 100644 --- a/lib/Doctrine/ORM/EntityRepository.php +++ b/lib/Doctrine/ORM/EntityRepository.php @@ -178,7 +178,7 @@ class EntityRepository implements ObjectRepository */ public function findOneBy(array $criteria) { - return $this->_em->getUnitOfWork()->getEntityPersister($this->_entityName)->load($criteria, null, null, array(), 0, 1); + return $this->_em->getUnitOfWork()->getEntityPersister($this->_entityName)->load($criteria); } /** diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index 1938ad160..19da2e200 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -559,13 +559,12 @@ class BasicEntityPersister * @param $assoc The association that connects the entity to load to another entity, if any. * @param array $hints Hints for entity creation. * @param int $lockMode - * @param int $limit Limit number of results * @return object The loaded and managed entity instance or NULL if the entity can not be found. * @todo Check identity map? loadById method? Try to guess whether $criteria is the id? */ - public function load(array $criteria, $entity = null, $assoc = null, array $hints = array(), $lockMode = 0, $limit = 0) + public function load(array $criteria, $entity = null, $assoc = null, array $hints = array(), $lockMode = 0) { - $sql = $this->_getSelectEntitiesSQL($criteria, $assoc, $lockMode, $limit); + $sql = $this->_getSelectEntitiesSQL($criteria, $assoc, $lockMode); list($params, $types) = $this->expandParameters($criteria); $stmt = $this->_conn->executeQuery($sql, $params, $types); From 65f7e897b579549b9ef75465edcad4d32d54f727 Mon Sep 17 00:00:00 2001 From: Alexander Date: Tue, 26 Jul 2011 00:19:26 +0200 Subject: [PATCH 4/5] [DDC-1294] Add discriminator information to subselects --- lib/Doctrine/ORM/Query/SqlWalker.php | 41 +++++++++---------- .../ORM/Query/SelectSqlGenerationTest.php | 11 +++++ 2 files changed, 30 insertions(+), 22 deletions(-) diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 2a9f58faf..7393ba893 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -359,13 +359,7 @@ class SqlWalker implements TreeWalker { $sql = $this->walkSelectClause($AST->selectClause); $sql .= $this->walkFromClause($AST->fromClause); - - if (($whereClause = $AST->whereClause) !== null) { - $sql .= $this->walkWhereClause($whereClause); - } else if (($discSql = $this->_generateDiscriminatorColumnConditionSQL($this->_rootAliases)) !== '') { - $sql .= ' WHERE ' . $discSql; - } - + $sql .= $this->walkWhereClause($AST->whereClause); $sql .= $AST->groupByClause ? $this->walkGroupByClause($AST->groupByClause) : ''; $sql .= $AST->havingClause ? $this->walkHavingClause($AST->havingClause) : ''; @@ -407,12 +401,7 @@ class SqlWalker implements TreeWalker { $this->_useSqlTableAliases = false; $sql = $this->walkUpdateClause($AST->updateClause); - - if (($whereClause = $AST->whereClause) !== null) { - $sql .= $this->walkWhereClause($whereClause); - } else if (($discSql = $this->_generateDiscriminatorColumnConditionSQL($this->_rootAliases)) !== '') { - $sql .= ' WHERE ' . $discSql; - } + $sql .= $this->walkWhereClause($AST->whereClause); return $sql; } @@ -427,12 +416,7 @@ class SqlWalker implements TreeWalker { $this->_useSqlTableAliases = false; $sql = $this->walkDeleteClause($AST->deleteClause); - - if (($whereClause = $AST->whereClause) !== null) { - $sql .= $this->walkWhereClause($whereClause); - } else if (($discSql = $this->_generateDiscriminatorColumnConditionSQL($this->_rootAliases)) !== '') { - $sql .= ' WHERE ' . $discSql; - } + $sql .= $this->walkWhereClause($AST->whereClause); return $sql; } @@ -1158,15 +1142,19 @@ class SqlWalker implements TreeWalker public function walkSubselect($subselect) { $useAliasesBefore = $this->_useSqlTableAliases; + $rootAliasesBefore = $this->_rootAliases; + + $this->_rootAliases = array(); // reset the rootAliases for the subselect $this->_useSqlTableAliases = true; $sql = $this->walkSimpleSelectClause($subselect->simpleSelectClause); $sql .= $this->walkSubselectFromClause($subselect->subselectFromClause); - $sql .= $subselect->whereClause ? $this->walkWhereClause($subselect->whereClause) : ''; + $sql .= $this->walkWhereClause($subselect->whereClause); $sql .= $subselect->groupByClause ? $this->walkGroupByClause($subselect->groupByClause) : ''; $sql .= $subselect->havingClause ? $this->walkHavingClause($subselect->havingClause) : ''; $sql .= $subselect->orderByClause ? $this->walkOrderByClause($subselect->orderByClause) : ''; + $this->_rootAliases = $rootAliasesBefore; // put the main aliases back $this->_useSqlTableAliases = $useAliasesBefore; return $sql; @@ -1193,6 +1181,8 @@ class SqlWalker implements TreeWalker $sql .= $class->getQuotedTableName($this->_platform) . ' ' . $this->getSQLTableAlias($class->table['name'], $dqlAlias); + $this->_rootAliases[] = $dqlAlias; + if ($class->isInheritanceTypeJoined()) { $sql .= $this->_generateClassTableInheritanceJoins($class, $dqlAlias); } @@ -1414,16 +1404,23 @@ class SqlWalker implements TreeWalker /** * Walks down a WhereClause AST node, thereby generating the appropriate SQL. + * WhereClause or not, the appropriate discriminator sql is added. * * @param WhereClause * @return string The SQL. */ public function walkWhereClause($whereClause) { + $condSql = null !== $whereClause ? $this->walkConditionalExpression($whereClause->conditionalExpression) : ''; $discrSql = $this->_generateDiscriminatorColumnConditionSql($this->_rootAliases); - $condSql = $this->walkConditionalExpression($whereClause->conditionalExpression); - return ' WHERE ' . (( ! $discrSql) ? $condSql : '(' . $condSql . ') AND ' . $discrSql); + if ($condSql) { + return ' WHERE ' . (( ! $discrSql) ? $condSql : '(' . $condSql . ') AND ' . $discrSql); + } else if ($discrSql) { + return ' WHERE ' . $discrSql; + } + + return ''; } /** diff --git a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php index deffb9232..6c21abfcf 100644 --- a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php +++ b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php @@ -922,6 +922,17 @@ class SelectSqlGenerationTest extends \Doctrine\Tests\OrmTestCase "SELECT COALESCE(NULLIF(c0_.name, ''), c0_.username) AS sclr0 FROM cms_users c0_" ); } + + /** + * Test that the right discriminator data is inserted in a subquery. + */ + public function testSubSelectDiscriminator() + { + $this->assertSqlGeneration( + "SELECT u.name, (SELECT COUNT(cfc.id) total FROM Doctrine\Tests\Models\Company\CompanyFixContract cfc) as cfc_count FROM Doctrine\Tests\Models\CMS\CmsUser u", + "SELECT c0_.name AS name0, (SELECT COUNT(c1_.id) AS dctrn__total FROM company_contracts c1_ WHERE c1_.discr IN ('fix')) AS sclr1 FROM cms_users c0_" + ); + } } From 97a4dbf2cd7d199af8a96eb5240eb5e94a022337 Mon Sep 17 00:00:00 2001 From: Alexander Date: Tue, 26 Jul 2011 17:15:21 +0200 Subject: [PATCH 5/5] [DDC-1294] Added more tests for subselects in subselects --- .../ORM/Query/SelectSqlGenerationTest.php | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php index 6c21abfcf..ad5ce1329 100644 --- a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php +++ b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php @@ -933,6 +933,38 @@ class SelectSqlGenerationTest extends \Doctrine\Tests\OrmTestCase "SELECT c0_.name AS name0, (SELECT COUNT(c1_.id) AS dctrn__total FROM company_contracts c1_ WHERE c1_.discr IN ('fix')) AS sclr1 FROM cms_users c0_" ); } + + public function testIdVariableResultVariableReuse() + { + $exceptionThrown = false; + try { + $query = $this->_em->createQuery("SELECT u.name FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE u.name IN (SELECT u.name FROM Doctrine\Tests\Models\CMS\CmsUser u)"); + + $query->getSql(); + $query->free(); + } catch (\Exception $e) { + $exceptionThrown = true; + } + + $this->assertTrue($exceptionThrown); + + } + + public function testSubSelectAliasesFromOuterQuery() + { + $this->assertSqlGeneration( + "SELECT uo, (SELECT ui.name FROM Doctrine\Tests\Models\CMS\CmsUser ui WHERE ui.id = uo.id) AS bar FROM Doctrine\Tests\Models\CMS\CmsUser uo", + "SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3, (SELECT c1_.name FROM cms_users c1_ WHERE c1_.id = c0_.id) AS sclr4 FROM cms_users c0_" + ); + } + + public function testSubSelectAliasesFromOuterQueryWithSubquery() + { + $this->assertSqlGeneration( + "SELECT uo, (SELECT ui.name FROM Doctrine\Tests\Models\CMS\CmsUser ui WHERE ui.id = uo.id AND ui.name IN (SELECT uii.name FROM Doctrine\Tests\Models\CMS\CmsUser uii)) AS bar FROM Doctrine\Tests\Models\CMS\CmsUser uo", + "SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3, (SELECT c1_.name FROM cms_users c1_ WHERE c1_.id = c0_.id AND c1_.name IN (SELECT c2_.name FROM cms_users c2_)) AS sclr4 FROM cms_users c0_" + ); + } }