Skip to content

Loading…

DDC-966: SchemaTool does not check for inherited fields in STI sub-classes and overwrites their column definitions to DEFAULT NULL #5501

Closed
doctrinebot opened this Issue · 3 comments

2 participants

@doctrinebot

Jira issue originally created by user ayhan:

/****
 * @Entity
 * @Table(
 *     name="eav_attribute",
 *     uniqueConstraints={
 *         @UniqueConstraint(columns={"core*project*id", "name"})
 *     }
 * )
 * @InheritanceType(
 *     "SINGLE_TABLE"
 * )
 * @DiscriminatorColumn(
 *     name="datatype",
 *     type="string"
 * )
 * @DiscriminatorMap({
 *     "datetime"="AttributeDatetime",
 *     "decimal"="AttributeDecimal",
 *     "int"="AttributeInt",
 *     "string"="AttributeString",
 *     "text"="AttributeText"
 * })
 */
abstract class AbstractAttribute
{
    // ...

    /****
     * @Column(type="string")
     */
    protected $name;

    /****
     * @Column(type="boolean")
     */
    protected $is_unique;

    // ...
}

/****
 * @Entity
 */
class AttributeDatetime extends AbstractAttribute
{
}

Expected SQL-dump:

name VARCHAR(255) NOT NULL,
is_unique TINYINT(1) NOT NULL,

SQL-dump created by SchemaTool:

name VARCHAR(255) DEFAULT NULL,
is_unique TINYINT(1) DEFAULT NULL,

This behaviour is problematic, especially for columns which are part of a unique constraint.

Reason:

SchemaTool doesn't really check for inherited fields in STI sub classes in method getSchemaFromMetadata() and passes all fields (incl. the inherited) to method _gatherColumn() where finally the column definition of the parent class will be overwritten.

private function _gatherColumn($class, array $mapping, $table)
{
    // Lines 309 - 311
    if ($class->isInheritanceTypeSingleTable() && count($class->parentClasses) > 0) {
        $options['notnull'] = false;
    }
}

A quick fix:

Change that if-clause above to

if ($class->isInheritanceTypeSingleTable() && count($class->parentClasses) > 0 && ! isset($mapping['inherited'])) {

Better fix:

Only pass not inherited fields to the _gatherColumn() method, as done with CTI sub classes.

@doctrinebot

Comment created by @beberlei:

Fixed

@doctrinebot

Issue was closed with resolution "Fixed"

@doctrinebot

Comment created by ayhan:

Wow, that was fast!

@beberlei beberlei was assigned by doctrinebot
@doctrinebot doctrinebot added this to the 2.0.1 milestone
@doctrinebot doctrinebot closed this
@doctrinebot doctrinebot added the Bug label
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.