From ad6130b02d5dd2ae37ee5621ed67e5b84e3199cb Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 11 Dec 2011 19:29:36 +0100 Subject: [PATCH] DDC-551 - Cleanup filters branch, especially inheritance related code and yoda conditions and some inconsistencies --- .../ORM/Persisters/BasicEntityPersister.php | 3 ++- .../Persisters/JoinedSubclassPersister.php | 23 ++++++------------- .../ORM/Persisters/ManyToManyPersister.php | 18 +++++++++++---- .../ORM/Persisters/OneToManyPersister.php | 4 +--- .../ORM/Persisters/SingleTablePersister.php | 8 +++---- lib/Doctrine/ORM/Query/Filter/SQLFilter.php | 1 - 6 files changed, 26 insertions(+), 31 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index 4cec1314d..ea9e8cba4 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -1604,6 +1604,7 @@ class BasicEntityPersister } } - return implode(' AND ', $filterClauses); + $sql = implode(' AND ', $filterClauses); + return $sql ? "(" . $sql . ")" : ""; // Wrap again to avoid "X or Y and FilterConditionSQL" } } diff --git a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php index 02d6ab75c..aca4d14b4 100644 --- a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php +++ b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php @@ -328,13 +328,6 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister if ($first) $first = false; else $joinSql .= ' AND '; $joinSql .= $baseTableAlias . '.' . $idColumn . ' = ' . $tableAlias . '.' . $idColumn; - - if ($parentClass->name === $this->_class->rootEntityName) { - // Add filters on the root class - if ('' !== $filterSql = $this->generateFilterConditionSQL($parentClass, $tableAlias)) { - $joinSql .= ' AND ' . $filterSql; - } - } } } @@ -383,14 +376,12 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister $conditionSql = $this->_getSelectConditionSQL($criteria, $assoc); // If the current class in the root entity, add the filters - if ($this->_class->name === $this->_class->rootEntityName) { - if ('' !== $filterSql = $this->generateFilterConditionSQL($this->_class, $baseTableAlias)) { - if ($conditionSql) { - $conditionSql .= ' AND '; - } - - $conditionSql .= $filterSql; + if ($filterSql = $this->generateFilterConditionSQL($this->_em->getClassMetadata($this->_class->rootEntityName), $this->_getSQLTableAlias($this->_class->rootEntityName))) { + if ($conditionSql) { + $conditionSql .= ' AND '; } + + $conditionSql .= $filterSql; } $orderBy = ($assoc !== null && isset($assoc['orderBy'])) ? $assoc['orderBy'] : $orderBy; @@ -420,10 +411,10 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister * * @return string */ - public function getLockTablesSql($alias = null) + public function getLockTablesSql() { $idColumns = $this->_class->getIdentifierColumnNames(); - $baseTableAlias = null !== $alias ? $alias : $this->_getSQLTableAlias($this->_class->name); + $baseTableAlias = $this->_getSQLTableAlias($this->_class->name); // INNER JOIN parent tables $joinSql = ''; diff --git a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php index bfb6ef08e..99391d9fa 100644 --- a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php @@ -219,7 +219,7 @@ class ManyToManyPersister extends AbstractCollectionPersister } list($joinTargetEntitySQL, $filterSql) = $this->getFilterSql($mapping); - if ('' !== $filterSql) { + if ($filterSql) { $whereClauses[] = $filterSql; } @@ -333,7 +333,7 @@ class ManyToManyPersister extends AbstractCollectionPersister if ($addFilters) { list($joinTargetEntitySQL, $filterSql) = $this->getFilterSql($mapping); - if ('' !== $filterSql) { + if ($filterSql) { $quotedJoinTable .= ' t ' . $joinTargetEntitySQL; $whereClauses[] = $filterSql; } @@ -345,6 +345,12 @@ class ManyToManyPersister extends AbstractCollectionPersister /** * Generates the filter SQL for a given mapping. * + * This method is not used for actually grabbing the related entities + * but when the extra-lazy collection methods are called on a filtered + * association. This is why besides the many to many table we also + * have to join in the actual entities table leading to additional + * JOIN. + * * @param array $targetEntity Array containing mapping information. * * @return string The SQL query part to add to a query. @@ -352,10 +358,11 @@ class ManyToManyPersister extends AbstractCollectionPersister public function getFilterSql($mapping) { $targetClass = $this->_em->getClassMetadata($mapping['targetEntity']); + $targetClass = $this->_em->getClassMetadata($targetClass->rootEntityName); // A join is needed if there is filtering on the target entity $joinTargetEntitySQL = ''; - if ('' !== $filterSql = $this->generateFilterConditionSQL($targetClass, 'te')) { + if ($filterSql = $this->generateFilterConditionSQL($targetClass, 'te')) { $joinTargetEntitySQL = ' JOIN ' . $targetClass->getQuotedTableName($this->_conn->getDatabasePlatform()) . ' te' . ' ON'; @@ -384,11 +391,12 @@ class ManyToManyPersister extends AbstractCollectionPersister $filterClauses = array(); foreach ($this->_em->getFilters()->getEnabledFilters() as $filter) { - if ('' !== $filterExpr = $filter->addFilterConstraint($targetEntity, $targetTableAlias)) { + if ($filterExpr = $filter->addFilterConstraint($targetEntity, $targetTableAlias)) { $filterClauses[] = '(' . $filterExpr . ')'; } } - return implode(' AND ', $filterClauses); + $sql = implode(' AND ', $filterClauses); + return $sql ? "(" . $sql . ")" : ""; } } diff --git a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php b/lib/Doctrine/ORM/Persisters/OneToManyPersister.php index 2fe1d5acb..b78d0e561 100644 --- a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/OneToManyPersister.php @@ -1,7 +1,5 @@ _em->getFilters()->getEnabledFilters() as $filter) { - if ('' !== $filterExpr = $filter->addFilterConstraint($targetClass, 't')) { + if ($filterExpr = $filter->addFilterConstraint($targetClass, 't')) { $whereClauses[] = '(' . $filterExpr . ')'; } } diff --git a/lib/Doctrine/ORM/Persisters/SingleTablePersister.php b/lib/Doctrine/ORM/Persisters/SingleTablePersister.php index 39a6b4954..c2687c109 100644 --- a/lib/Doctrine/ORM/Persisters/SingleTablePersister.php +++ b/lib/Doctrine/ORM/Persisters/SingleTablePersister.php @@ -137,11 +137,9 @@ class SingleTablePersister extends AbstractEntityInheritancePersister protected function generateFilterConditionSQL(ClassMetadata $targetEntity, $targetTableAlias) { // Ensure that the filters are applied to the root entity of the inheritance tree - $realTargetEntity = $targetEntity; - if ($targetEntity->name !== $targetEntity->rootEntityName) { - $realTargetEntity = $this->_em->getClassMetadata($targetEntity->rootEntityName); - } + $targetEntity = $this->_em->getClassMetadata($targetEntity->rootEntityName); + // we dont care about the $targetTableAlias, in a STI there is only one table. - return parent::generateFilterConditionSQL($realTargetEntity, $targetTableAlias); + return parent::generateFilterConditionSQL($targetEntity, $targetTableAlias); } } diff --git a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php index 772ef683b..1250b16d7 100644 --- a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php +++ b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php @@ -66,7 +66,6 @@ abstract class SQLFilter */ final public function setParameter($name, $value, $type) { - // @todo: check for a valid type? $this->parameters[$name] = array('value' => $value, 'type' => $type); // Keep the parameters sorted for the hash