DDC-551 - Cleanup filters branch, especially inheritance related code and yoda conditions and some inconsistencies

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"
     }
 }

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,15 +376,13 @@ 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->_em->getClassMetadata($this->_class->rootEntityName), $this->_getSQLTableAlias($this->_class->rootEntityName))) {
-            if ('' !== $filterSql = $this->generateFilterConditionSQL($this->_class, $baseTableAlias)) {
             if ($conditionSql) {
                 $conditionSql .= ' AND ';
             }
 
             $conditionSql .= $filterSql;
         }
-        }
 
         $orderBy = ($assoc !== null && isset($assoc['orderBy'])) ? $assoc['orderBy'] : $orderBy;
         $orderBySql = $orderBy ? $this->_getOrderBySQL($orderBy, $baseTableAlias) : '';
@@ -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 = '';

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 . ")" : "";
     }
 }

lib/Doctrine/ORM/Persisters/OneToManyPersister.php

@@ -1,7 +1,5 @@
 <?php
 /*
- *  $Id$
- *
  * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
  * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
  * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
@@ -122,7 +120,7 @@ class OneToManyPersister extends AbstractCollectionPersister
         }
 
         foreach ($this->_em->getFilters()->getEnabledFilters() as $filter) {
-            if ('' !== $filterExpr = $filter->addFilterConstraint($targetClass, 't')) {
+            if ($filterExpr = $filter->addFilterConstraint($targetClass, 't')) {
                 $whereClauses[] = '(' . $filterExpr . ')';
             }
         }

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;
+        $targetEntity = $this->_em->getClassMetadata($targetEntity->rootEntityName);
-        if ($targetEntity->name !== $targetEntity->rootEntityName) {
+        // we dont care about the $targetTableAlias, in a STI there is only one table.
-            $realTargetEntity = $this->_em->getClassMetadata($targetEntity->rootEntityName);
-        }
 
-        return parent::generateFilterConditionSQL($realTargetEntity, $targetTableAlias);
+        return parent::generateFilterConditionSQL($targetEntity, $targetTableAlias);
     }
 }

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