Correct DQL INSTANCE OF to filter all possible child classes

lib/Doctrine/ORM/Query.php

@@ -27,6 +27,7 @@
 use Doctrine\ORM\Mapping\ClassMetadata;
 use Doctrine\ORM\Query\ParameterTypeInferer;
 use Doctrine\Common\Collections\ArrayCollection;
+use Doctrine\ORM\Utility\HierarchyDiscriminatorResolver;
 
 /**
  * A Query object represents a DQL query.
@@ -398,6 +399,10 @@ private function processParameterMappings($paramMappings)
                 $value = $value->getMetadataValue($rsm->metadataParameterMapping[$key]);
             }
 
+            if (isset($rsm->discriminatorParameters[$key]) && $value instanceof ClassMetadata) {
+                $value = array_keys(HierarchyDiscriminatorResolver::resolveDiscriminatorsForClass($value, $this->_em));
+            }
+
             $value = $this->processParameterValue($value);
             $type  = ($parameter->getValue() === $value)
                 ? $parameter->getType()

lib/Doctrine/ORM/Query/ResultSetMapping.php

@@ -168,6 +168,13 @@ class ResultSetMapping
      */
     public $metadataParameterMapping = [];
 
+    /**
+     * Contains query parameter names to be resolved as discriminator values
+     *
+     * @var array
+     */
+    public $discriminatorParameters = [];
+
     /**
      * Adds an entity result to this ResultSetMapping.
      *

lib/Doctrine/ORM/Query/SqlWalker.php

@@ -25,6 +25,7 @@
 use Doctrine\ORM\Mapping\ClassMetadataInfo;
 use Doctrine\ORM\OptimisticLockException;
 use Doctrine\ORM\Query;
+use Doctrine\ORM\Utility\HierarchyDiscriminatorResolver;
 use Doctrine\ORM\Utility\PersisterHelper;
 
 /**
@@ -37,7 +38,6 @@
  * @author Alexander <iam.asm89@gmail.com>
  * @author Fabio B. Silva <fabio.bat.silva@gmail.com>
  * @since  2.0
- * @todo Rename: SQLWalker
  */
 class SqlWalker implements TreeWalker
 {
@@ -2017,6 +2017,7 @@ public function walkInExpression($inExpr)
 
     /**
      * {@inheritdoc}
+     * @throws \Doctrine\ORM\Query\QueryException
      */
     public function walkInstanceOfExpression($instanceOfExpr)
     {
@@ -2034,36 +2035,7 @@ public function walkInstanceOfExpression($instanceOfExpr)
         }
 
         $sql .= $class->discriminatorColumn['name'] . ($instanceOfExpr->not ? ' NOT IN ' : ' IN ');
-
-        $sqlParameterList = [];
-
-        foreach ($instanceOfExpr->value as $parameter) {
-            if ($parameter instanceof AST\InputParameter) {
-                $this->rsm->addMetadataParameterMapping($parameter->name, 'discriminatorValue');
-
-                $sqlParameterList[] = $this->walkInputParameter($parameter);
-
-                continue;
-            }
-
-            // Get name from ClassMetadata to resolve aliases.
-            $entityClassName    = $this->em->getClassMetadata($parameter)->name;
-            $discriminatorValue = $class->discriminatorValue;
-
-            if ($entityClassName !== $class->name) {
-                $discrMap = array_flip($class->discriminatorMap);
-
-                if ( ! isset($discrMap[$entityClassName])) {
-                    throw QueryException::instanceOfUnrelatedClass($entityClassName, $class->rootEntityName);
-                }
-
-                $discriminatorValue = $discrMap[$entityClassName];
-            }
-
-            $sqlParameterList[] = $this->conn->quote($discriminatorValue);
-        }
-
-        $sql .= '(' . implode(', ', $sqlParameterList) . ')';
+        $sql .= $this->getChildDiscriminatorsFromClassMetadata($discrClass, $instanceOfExpr);
 
         return $sql;
     }
@@ -2297,4 +2269,37 @@ public function walkResultVariable($resultVariable)
 
         return $resultAlias;
     }
+
+    /**
+     * @param ClassMetadataInfo $rootClass
+     * @param AST\InstanceOfExpression $instanceOfExpr
+     * @return string The list in parentheses of valid child discriminators from the given class
+     * @throws QueryException
+     */
+    private function getChildDiscriminatorsFromClassMetadata(ClassMetadataInfo $rootClass, AST\InstanceOfExpression $instanceOfExpr): string
+    {
+        $sqlParameterList = [];
+        $discriminators = [];
+        foreach ($instanceOfExpr->value as $parameter) {
+            if ($parameter instanceof AST\InputParameter) {
+                $this->rsm->discriminatorParameters[$parameter->name] = $parameter->name;
+                $sqlParameterList[] = $this->walkInParameter($parameter);
+                continue;
+            }
+
+            $metadata = $this->em->getClassMetadata($parameter);
+
+            if ($metadata->getName() !== $rootClass->name && ! $metadata->getReflectionClass()->isSubclassOf($rootClass->name)) {
+                throw QueryException::instanceOfUnrelatedClass($parameter, $rootClass->name);
+            }
+
+            $discriminators += HierarchyDiscriminatorResolver::resolveDiscriminatorsForClass($metadata, $this->em);
+        }
+
+        foreach (array_keys($discriminators) as $dis) {
+            $sqlParameterList[] = $this->conn->quote($dis);
+        }
+
+        return '(' . implode(', ', $sqlParameterList) . ')';
+    }
 }

lib/Doctrine/ORM/Utility/HierarchyDiscriminatorResolver.php

@@ -0,0 +1,41 @@
+<?php
+
+namespace Doctrine\ORM\Utility;
+
+use Doctrine\Common\Persistence\Mapping\ClassMetadata;
+use Doctrine\ORM\EntityManagerInterface;
+
+/**
+ * @internal This class exists only to avoid code duplication, do not reuse it externally
+ */
+final class HierarchyDiscriminatorResolver
+{
+    private function __construct()
+    {
+    }
+
+    /**
+     * This method is needed to make INSTANCEOF work correctly with inheritance: if the class at hand has inheritance,
+     * it extracts all the discriminators from the child classes and returns them
+     */
+    public static function resolveDiscriminatorsForClass(
+        ClassMetadata $rootClassMetadata,
+        EntityManagerInterface $entityManager
+    ): array {
+        $hierarchyClasses = $rootClassMetadata->subClasses;
+        $hierarchyClasses[] = $rootClassMetadata->name;
+
+        $discriminators = [];
+
+        foreach ($hierarchyClasses as $class) {
+            $currentMetadata = $entityManager->getClassMetadata($class);
+            $currentDiscriminator = $currentMetadata->discriminatorValue;
+
+            if (null !== $currentDiscriminator) {
+                $discriminators[$currentDiscriminator] = null;
+            }
+        }
+
+        return $discriminators;
+    }
+}

tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1995Test.php

@@ -72,10 +72,9 @@ public function testQueryCache()
                 ->getResult();
 
         $this->assertCount(1, $result1);
-        $this->assertCount(1, $result2);
+        $this->assertCount(2, $result2);
 
         $this->assertInstanceOf(CompanyEmployee::class, $result1[0]);
-        $this->assertInstanceOf(CompanyPerson::class, $result2[0]);
-        $this->assertNotInstanceOf(CompanyEmployee::class, $result2[0]);
+        $this->assertContainsOnlyInstancesOf(CompanyPerson::class, $result2);
     }
 }

tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfAbstractTest.php

@@ -0,0 +1,64 @@
+<?php
+
+namespace Doctrine\Tests\ORM\Functional\Ticket;
+
+use Doctrine\Tests\OrmFunctionalTestCase;
+
+class Ticket4646InstanceOfAbstractTest extends OrmFunctionalTestCase
+{
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        $this->_schemaTool->createSchema([
+            $this->_em->getClassMetadata(PersonTicket4646Abstract::class),
+            $this->_em->getClassMetadata(EmployeeTicket4646Abstract::class),
+        ]);
+    }
+
+    public function testInstanceOf(): void
+    {
+        $this->_em->persist(new EmployeeTicket4646Abstract());
+        $this->_em->flush();
+
+        $dql = 'SELECT p FROM Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646Abstract p
+                WHERE p INSTANCE OF Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646Abstract';
+        $query = $this->_em->createQuery($dql);
+        $result = $query->getResult();
+
+        self::assertCount(1, $result);
+        self::assertContainsOnlyInstancesOf(PersonTicket4646Abstract::class, $result);
+    }
+}
+
+/**
+ * @Entity()
+ * @Table(name="instance_of_abstract_test_person")
+ * @InheritanceType(value="JOINED")
+ * @DiscriminatorColumn(name="kind", type="string")
+ * @DiscriminatorMap(value={
+ *     "employee": EmployeeTicket4646Abstract::class
+ * })
+ */
+abstract class PersonTicket4646Abstract
+{
+    /**
+     * @Id()
+     * @GeneratedValue()
+     * @Column(type="integer")
+     */
+    private $id;
+
+    public function getId(): ?int
+    {
+        return $this->id;
+    }
+}
+
+/**
+ * @Entity()
+ * @Table(name="instance_of_abstract_test_employee")
+ */
+class EmployeeTicket4646Abstract extends PersonTicket4646Abstract
+{
+}

tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfMultiLevelTest.php

@@ -0,0 +1,77 @@
+<?php
+
+namespace Doctrine\Tests\ORM\Functional\Ticket;
+
+use Doctrine\Tests\OrmFunctionalTestCase;
+
+class Ticket4646InstanceOfMultiLevelTest extends OrmFunctionalTestCase
+{
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        $this->_schemaTool->createSchema([
+            $this->_em->getClassMetadata(PersonTicket4646MultiLevel::class),
+            $this->_em->getClassMetadata(EmployeeTicket4646MultiLevel::class),
+            $this->_em->getClassMetadata(EngineerTicket4646MultiLevel::class),
+        ]);
+    }
+
+    public function testInstanceOf(): void
+    {
+        $this->_em->persist(new PersonTicket4646MultiLevel());
+        $this->_em->persist(new EmployeeTicket4646MultiLevel());
+        $this->_em->persist(new EngineerTicket4646MultiLevel());
+        $this->_em->flush();
+
+        $dql = 'SELECT p FROM Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646MultiLevel p
+                WHERE p INSTANCE OF Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646MultiLevel';
+        $query = $this->_em->createQuery($dql);
+        $result = $query->getResult();
+
+        self::assertCount(3, $result);
+        self::assertContainsOnlyInstancesOf(PersonTicket4646MultiLevel::class, $result);
+    }
+}
+
+/**
+ * @Entity()
+ * @Table(name="instance_of_multi_level_test_person")
+ * @InheritanceType(value="JOINED")
+ * @DiscriminatorColumn(name="kind", type="string")
+ * @DiscriminatorMap(value={
+ *     "person": "Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646MultiLevel",
+ *     "employee": "Doctrine\Tests\ORM\Functional\Ticket\EmployeeTicket4646MultiLevel",
+ *     "engineer": "Doctrine\Tests\ORM\Functional\Ticket\EngineerTicket4646MultiLevel",
+ * })
+ */
+class PersonTicket4646MultiLevel
+{
+    /**
+     * @Id()
+     * @GeneratedValue()
+     * @Column(type="integer")
+     */
+    private $id;
+
+    public function getId(): ?int
+    {
+        return $this->id;
+    }
+}
+
+/**
+ * @Entity()
+ * @Table(name="instance_of_multi_level_employee")
+ */
+class EmployeeTicket4646MultiLevel extends PersonTicket4646MultiLevel
+{
+}
+
+/**
+ * @Entity()
+ * @Table(name="instance_of_multi_level_engineer")
+ */
+class EngineerTicket4646MultiLevel extends EmployeeTicket4646MultiLevel
+{
+}

tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfParametricTest.php

@@ -0,0 +1,67 @@
+<?php
+
+namespace Doctrine\Tests\ORM\Functional\Ticket;
+
+use Doctrine\Tests\OrmFunctionalTestCase;
+
+class Ticket4646InstanceOfParametricTest extends OrmFunctionalTestCase
+{
+    protected function setUp(): void
+    {
+        parent::setUp();
+        $this->_schemaTool->createSchema([
+            $this->_em->getClassMetadata(PersonTicket4646Parametric::class),
+            $this->_em->getClassMetadata(EmployeeTicket4646Parametric::class),
+        ]);
+    }
+
+    public function testInstanceOf(): void
+    {
+        $this->_em->persist(new PersonTicket4646Parametric());
+        $this->_em->persist(new EmployeeTicket4646Parametric());
+        $this->_em->flush();
+        $dql = 'SELECT p FROM Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646Parametric p
+                WHERE p INSTANCE OF :parameter';
+        $query = $this->_em->createQuery($dql);
+        $query->setParameter(
+            'parameter',
+            $this->_em->getClassMetadata(PersonTicket4646Parametric::class)
+        );
+        $result = $query->getResult();
+        self::assertCount(2, $result);
+        self::assertContainsOnlyInstancesOf(PersonTicket4646Parametric::class, $result);
+    }
+}
+
+/**
+ * @Entity()
+ * @Table(name="instance_of_parametric_person")
+ * @InheritanceType(value="JOINED")
+ * @DiscriminatorColumn(name="kind", type="string")
+ * @DiscriminatorMap(value={
+ *     "person": "Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646Parametric",
+ *     "employee": "Doctrine\Tests\ORM\Functional\Ticket\EmployeeTicket4646Parametric"
+ * })
+ */
+class PersonTicket4646Parametric
+{
+    /**
+     * @Id()
+     * @GeneratedValue()
+     * @Column(type="integer")
+     */
+    private $id;
+
+    public function getId(): ?int
+    {
+        return $this->id;
+    }
+}
+
+/**
+ * @Entity()
+ * @Table(name="instance_of_parametric_employee")
+ */
+class EmployeeTicket4646Parametric extends PersonTicket4646Parametric
+{
+}