Allow association mappings as IDs for joined-table inherited entity

SchemaTool has custom logic for creating the primary key of a
joined-table inherited entity. This logic overlooked association maps
as a possible source for identity columns, resulting in a fatal error
when fetching the primary key list for child entities.

Removed any custom logic for generating primary keys for root entities
in joined-table inheritance, deferring to the common logic used for
other entities.

Also adjusted the child entity logic, scanning association maps for
identity columns, and including the column as appropriate. It also
ensures that the primary key columns are in the correct order.

lib/Doctrine/ORM/Tools/SchemaTool.php

@@ -183,19 +183,9 @@ class SchemaTool
                 }
             } elseif ($class->isInheritanceTypeJoined()) {
                 // Add all non-inherited fields as columns
-                $pkColumns = [];
                 foreach ($class->fieldMappings as $fieldName => $mapping) {
                     if ( ! isset($mapping['inherited'])) {
-                        $columnName = $this->quoteStrategy->getColumnName(
-                            $mapping['fieldName'],
-                            $class,
-                            $this->platform
-                        );
                         $this->gatherColumn($class, $mapping, $table);
-
-                        if ($class->isIdentifier($fieldName)) {
-                            $pkColumns[] = $columnName;
-                        }
                     }
                 }
 
@@ -206,10 +196,12 @@ class SchemaTool
                     $this->addDiscriminatorColumnDefinition($class, $table);
                 } else {
                     // Add an ID FK column to child tables
+                    $pkColumns           = [];
                     $inheritedKeyColumns = [];
+
                     foreach ($class->identifier as $identifierField) {
-                        $idMapping = $class->fieldMappings[$identifierField];
+                        if (isset($class->fieldMappings[$identifierField]['inherited'])) {
-                        if (isset($idMapping['inherited'])) {
+                            $idMapping = $class->fieldMappings[$identifierField];
                             $this->gatherColumn($class, $idMapping, $table);
                             $columnName = $this->quoteStrategy->getColumnName(
                                 $identifierField,
@@ -221,9 +213,39 @@ class SchemaTool
 
                             $pkColumns[] = $columnName;
                             $inheritedKeyColumns[] = $columnName;
+
+                            continue;
+                        }
+
+                        if (isset($class->associationMappings[$identifierField]['inherited'])) {
+                            $idMapping = $class->associationMappings[$identifierField];
+
+                            $targetEntity = current(
+                                array_filter(
+                                    $classes,
+                                    function (ClassMetadata $class) use ($idMapping) : bool {
+                                        return $class->name === $idMapping['targetEntity'];
+                                    }
+                                )
+                            );
+
+                            foreach ($idMapping['joinColumns'] as $joinColumn) {
+                                if (isset($targetEntity->fieldMappings[$joinColumn['referencedColumnName']])) {
+                                    $idMapping = $targetEntity->fieldMappings[$joinColumn['referencedColumnName']];
+                                    $columnName = $this->quoteStrategy->getJoinColumnName(
+                                        $joinColumn,
+                                        $class,
+                                        $this->platform
+                                    );
+
+                                    $pkColumns[]           = $columnName;
+                                    $inheritedKeyColumns[] = $columnName;
+                                }
+                            }
                         }
                     }
-                    if (!empty($inheritedKeyColumns)) {
+
+                    if ( ! empty($inheritedKeyColumns)) {
                         // Add a FK constraint on the ID column
                         $table->addForeignKeyConstraint(
                             $this->quoteStrategy->getTableName(
@@ -236,10 +258,10 @@ class SchemaTool
                         );
                     }
 
+                    if ( ! empty($pkColumns)) {
+                        $table->setPrimaryKey($pkColumns);
+                    }
                 }
-
-                $table->setPrimaryKey($pkColumns);
-
             } elseif ($class->isInheritanceTypeTablePerClass()) {
                 throw ORMException::notSupported();
             } else {
@@ -330,7 +352,7 @@ class SchemaTool
             }
         }
 
-        if ( ! $this->platform->supportsSchemas() && ! $this->platform->canEmulateSchemas() ) {
+        if ( ! $this->platform->supportsSchemas() && ! $this->platform->canEmulateSchemas()) {
             $schema->visit(new RemoveNamespacedAssets());
         }
 
@@ -496,8 +518,8 @@ class SchemaTool
      */
     private function gatherRelationsSql($class, $table, $schema, &$addedFks, &$blacklistedFks)
     {
-        foreach ($class->associationMappings as $mapping) {
+        foreach ($class->associationMappings as $id => $mapping) {
-            if (isset($mapping['inherited'])) {
+            if (isset($mapping['inherited']) && array_search($id, $class->identifier) === false) {
                 continue;
             }
 
@@ -633,21 +655,21 @@ class SchemaTool
 
             if ( ! $definingClass) {
                 throw new \Doctrine\ORM\ORMException(
-                    "Column name `".$joinColumn['referencedColumnName']."` referenced for relation from ".
+                    'Column name `' . $joinColumn['referencedColumnName'] . '` referenced for relation from '
-                    $mapping['sourceEntity'] . " towards ". $mapping['targetEntity'] . " does not exist."
+                    . $mapping['sourceEntity'] . ' towards ' . $mapping['targetEntity'] . ' does not exist.'
                 );
             }
 
-            $quotedColumnName       = $this->quoteStrategy->getJoinColumnName($joinColumn, $class, $this->platform);
+            $quotedColumnName    = $this->quoteStrategy->getJoinColumnName($joinColumn, $class, $this->platform);
-            $quotedRefColumnName    = $this->quoteStrategy->getReferencedJoinColumnName(
+            $quotedRefColumnName = $this->quoteStrategy->getReferencedJoinColumnName(
                 $joinColumn,
                 $class,
                 $this->platform
             );
 
-            $primaryKeyColumns[]    = $quotedColumnName;
+            $primaryKeyColumns[] = $quotedColumnName;
-            $localColumns[]         = $quotedColumnName;
+            $localColumns[]      = $quotedColumnName;
-            $foreignColumns[]       = $quotedRefColumnName;
+            $foreignColumns[]    = $quotedRefColumnName;
 
             if ( ! $theJoinTable->hasColumn($quotedColumnName)) {
                 // Only add the column to the table if it does not exist already.
@@ -666,7 +688,7 @@ class SchemaTool
                 $columnOptions = ['notnull' => false, 'columnDefinition' => $columnDef];
 
                 if (isset($joinColumn['nullable'])) {
-                    $columnOptions['notnull'] = !$joinColumn['nullable'];
+                    $columnOptions['notnull'] = ! $joinColumn['nullable'];
                 }
 
                 if (isset($fieldMapping['options'])) {
@@ -713,7 +735,7 @@ class SchemaTool
                 }
             }
             $blacklistedFks[$compositeName] = true;
-        } elseif (!isset($blacklistedFks[$compositeName])) {
+        } elseif ( ! isset($blacklistedFks[$compositeName])) {
             $addedFks[$compositeName] = ['foreignTableName' => $foreignTableName, 'foreignColumns' => $foreignColumns];
             $theJoinTable->addUnnamedForeignKeyConstraint(
                 $foreignTableName,
@@ -820,7 +842,7 @@ class SchemaTool
                 if ($table->hasPrimaryKey()) {
                     $columns = $table->getPrimaryKey()->getColumns();
                     if (count($columns) == 1) {
-                        $checkSequence = $table->getName() . "_" . $columns[0] . "_seq";
+                        $checkSequence = $table->getName() . '_' . $columns[0] . '_seq';
                         if ($fullSchema->hasSequence($checkSequence)) {
                             $visitor->acceptSequence($fullSchema->getSequence($checkSequence));
                         }

tests/Doctrine/Tests/Models/CompositeKeyInheritance/JoinedDerivedChildClass.php

@@ -0,0 +1,22 @@
+<?php
+namespace Doctrine\Tests\Models\CompositeKeyInheritance;
+
+/**
+ * @Entity
+ * @Table(name = "joined_derived_child")
+ */
+class JoinedDerivedChildClass extends JoinedDerivedRootClass
+{
+    /**
+     * @var string
+     * @Column(type="string")
+     */
+    public $extension = 'ext';
+
+    /**
+     * @var string
+     * @Column(type="string")
+     * @Id
+     */
+    private $additionalId = 'additional';
+}

tests/Doctrine/Tests/Models/CompositeKeyInheritance/JoinedDerivedIdentityClass.php

@@ -0,0 +1,25 @@
+<?php
+namespace Doctrine\Tests\Models\CompositeKeyInheritance;
+
+/**
+ * @Entity
+ * @Table(name = "joined_derived_identity")
+ */
+class JoinedDerivedIdentityClass
+{
+    /**
+     * @var string
+     * @Column(type="string")
+     * @Id
+     */
+    protected $id = 'part-0';
+
+    /**
+     * @var JoinedDerivedRootClass[]
+     * @OneToMany(
+     *     targetEntity="JoinedDerivedRootClass",
+     *     mappedBy="keyPart1"
+     * )
+     */
+    protected $children;
+}

tests/Doctrine/Tests/Models/CompositeKeyInheritance/JoinedDerivedRootClass.php

@@ -0,0 +1,29 @@
+<?php
+namespace Doctrine\Tests\Models\CompositeKeyInheritance;
+
+/**
+ * @Entity
+ * @Table(name = "joined_derived_root")
+ * @InheritanceType("JOINED")
+ * @DiscriminatorColumn(name="discr", type="string")
+ * @DiscriminatorMap({"child" = "JoinedDerivedChildClass", "root" = "JoinedDerivedRootClass"})
+ */
+class JoinedDerivedRootClass
+{
+    /**
+     * @var JoinedDerivedIdentityClass
+     * @ManyToOne(
+     *     targetEntity="JoinedDerivedIdentityClass",
+     *     inversedBy="children"
+     * )
+     * @Id
+     */
+    protected $keyPart1 = 'part-1';
+
+    /**
+     * @var string
+     * @Column(type="string")
+     * @Id
+     */
+    protected $keyPart2 = 'part-2';
+}

tests/Doctrine/Tests/ORM/Tools/SchemaToolTest.php

@@ -14,6 +14,9 @@ use Doctrine\Tests\Models\CMS\CmsEmployee;
 use Doctrine\Tests\Models\CMS\CmsGroup;
 use Doctrine\Tests\Models\CMS\CmsPhonenumber;
 use Doctrine\Tests\Models\CMS\CmsUser;
+use Doctrine\Tests\Models\CompositeKeyInheritance\JoinedDerivedChildClass;
+use Doctrine\Tests\Models\CompositeKeyInheritance\JoinedDerivedIdentityClass;
+use Doctrine\Tests\Models\CompositeKeyInheritance\JoinedDerivedRootClass;
 use Doctrine\Tests\Models\Forum\ForumAvatar;
 use Doctrine\Tests\Models\Forum\ForumUser;
 use Doctrine\Tests\Models\NullDefault\NullDefaultColumn;
@@ -184,6 +187,50 @@ class SchemaToolTest extends OrmTestCase
 
         $this->assertEquals(255, $column->getLength());
     }
+
+    public function testDerivedCompositeKey() : void
+    {
+        $em         = $this->_getTestEntityManager();
+        $schemaTool = new SchemaTool($em);
+
+        $schema = $schemaTool->getSchemaFromMetadata(
+            [
+                $em->getClassMetadata(JoinedDerivedIdentityClass::class),
+                $em->getClassMetadata(JoinedDerivedRootClass::class),
+                $em->getClassMetadata(JoinedDerivedChildClass::class),
+            ]
+        );
+
+        self::assertTrue($schema->hasTable('joined_derived_identity'));
+        self::assertTrue($schema->hasTable('joined_derived_root'));
+        self::assertTrue($schema->hasTable('joined_derived_child'));
+
+        $rootTable = $schema->getTable('joined_derived_root');
+        self::assertNotNull($rootTable->getPrimaryKey());
+        self::assertSame(['keyPart1_id', 'keyPart2'], $rootTable->getPrimaryKey()->getColumns());
+
+        $childTable = $schema->getTable('joined_derived_child');
+        self::assertNotNull($childTable->getPrimaryKey());
+        self::assertSame(['keyPart1_id', 'keyPart2'], $childTable->getPrimaryKey()->getColumns());
+
+        $childTableForeignKeys = $childTable->getForeignKeys();
+
+        self::assertCount(2, $childTableForeignKeys);
+
+        $expectedColumns = [
+            'joined_derived_identity' => [['keyPart1_id'], ['id']],
+            'joined_derived_root'     => [['keyPart1_id', 'keyPart2'], ['keyPart1_id', 'keyPart2']],
+        ];
+
+        foreach ($childTableForeignKeys as $foreignKey) {
+            self::assertArrayHasKey($foreignKey->getForeignTableName(), $expectedColumns);
+
+            [$localColumns, $foreignColumns] = $expectedColumns[$foreignKey->getForeignTableName()];
+
+            self::assertSame($localColumns, $foreignKey->getLocalColumns());
+            self::assertSame($foreignColumns, $foreignKey->getForeignColumns());
+        }
+    }
 }
 
 /**