Orm: Comparator detects unnecessary change in UNSIGNED field

[vicdelfant]

Bug Report

| Q | A
|------------ | ------
| BC Break | no
| Version | 2.8.0

Summary

I have implemented a custom type that requires a BIGINT UNSIGNED for storage in MySQL. In order to achieve this, I override Type::getSQLDeclaration() in our custom type class as follows:

public function getSQLDeclaration(array $fieldDeclaration, AbstractPlatform $platform)
{
    $fieldDeclaration['unsigned'] = true;

    return $platform->getBigIntTypeDeclarationSQL($fieldDeclaration);
}

This indeed generates the proper SQL query:

fancy_id BIGINT UNSIGNED NOT NULL COMMENT '(DC2Type:fancy_id)'

However, after applying this query, subsequent schema diffs keep generating the same ALTER query for the same field, with no actual modifications being done.

Current behaviour

Running the schema diff keeps generating ALTER queries for the field for no apparent reason. At first I thought this was related to #2596, but the comments are left intact. This could also be related to #3021, but I figured the hardcoded "UNSIGNED" string in that issue could actually be part of the cause there.

Upon digging further, I found out that Comparator::diffColumn() is getting false intel:

// $column1
Doctrine\DBAL\Schema\Column {#733
  #_type: Foo\FancyIdType {#344}
  #_length: null
  #_precision: 10
  #_scale: 0
  #_unsigned: true
  #_fixed: false
  #_notnull: true
  #_default: null
  #_autoincrement: false
  #_platformOptions: []
  #_columnDefinition: null
  #_comment: null
  #_customSchemaOptions: []
  #_name: "fancy_id"
  #_namespace: null
  #_quoted: false
}

// $column2
Doctrine\DBAL\Schema\Column {#702
  #_type: Foo\FancyIdType {#344}
  #_length: null
  #_precision: 0
  #_scale: 0
  #_unsigned: false
  #_fixed: false
  #_notnull: true
  #_default: null
  #_autoincrement: false
  #_platformOptions: array:1 [
    "version" => false
  ]
  #_columnDefinition: null
  #_comment: null
  #_customSchemaOptions: []
  #_name: "fancy_id"
  #_namespace: null
  #_quoted: false
}

It seems that the unsigned property isn't being set correctly in what's referenced to as $properties2 here. This obviously results in a difference while there actually isn't. I then turned to MySqlSchemaManager but it seems that it's detecting the unsigned flag properly.

How to reproduce

See above - implement a custom type that inherits from the IntegerType and set the $fieldDeclaration['unsigned'] property to true.

The schema diff works correctly if the unsigned flag isn't set (same type, same everything), so it's definitely related to the unsigned property.

Expected behaviour

Both columns get their unsigned property set to true in the Comparator, resulting in an empty $changedProperties and thus not resulting in any unnecessary ALTER queries in the schema diff.

[morozov]

@vicdelfant thank you for the report. Given that you already have all the needed code in place, could you put together a test case which we could run and reproduce the issue? Otherwise, we'll have to spend time building code from your specification which is not productive. Eventually, this code will turn into a test which covers the fix.

[vicdelfant]

Sorry for not getting back about this sooner @morozov!

I have prepared a simple test case: https://github.com/vicdelfant/doctrine-unsigned-type-issue. Since I'm not entirely sure whether it's the ORM or DBAL (or the combination of the two) that's causing issues, I have prepared it as a simple CLI test. Further instructions are in the README.

[vicdelfant]

Right. After debugging some more, I think I managed to pinpoint the cause. It's not the DBAL but the ORM that's causing the issue.

The Comparator called here gets its information from the ORM's SchemaTool::gatherColumn(), which doesn't include the unsigned property simply because it doesn't know that there should be one for the specific entity property we're working with. As a result, internally, there's a difference between the local entity (which has unsigned set to false according to the ORM) and the table (which has unsigned set to true). Upon schema generation, the custom DBAL type takes over and forcefully adds the unsigned flag, but the difference continues to exist.

So it's not the schema comparison that's causing the issue, it's the ORM side of things. If I change the entity's @ORM\Column annotation to include options={"unsigned"=true} then everything works as expected. Ideally, however, there'd be a way for the ORM to check with the custom DBAL type so it knows about the unsigned flag, or to have the comparison ignore this change.

The above means that this issue actually is a duplicate of #3021.

[vicdelfant]

The following could be a fix for both this issue and #3021: https://github.com/vicdelfant/doctrine2/commit/79d620064722f3e6dc8b6ce85b5689f9d0c3048a. The default Type in the DBAL would only need the following placeholder function:

/**
 * Gets the default field options of this type.
 *
 * @param AbstractPlatform $platform
 *
 * @return array
 */
public function getDefaultOptions(AbstractPlatform $platform)
{
    return [];
}

The custom type, in turn, could then return:

public function getDefaultOptions(AbstractPlatform $platform)
{
    return [
        'unsigned' => true,
    ];
}

The above solution has been tested in my PoC repo and it's working. The question is: does this fit with the Doctrine project's vision? It does create a (stronger) dependency between the ORM and DBAL components.

[morozov]

Right. After debugging some more, I think I managed to pinpoint the cause. It's not the DBAL but the ORM that's causing the issue.

Thank you for the info, @vicdelfant. I tried debugging your code briefly, and what I see is when the $toSchema is built in ShemaTool::getUpdateSchemaSql(), the CustomIdType::getSQLDeclaration() method is not invoked (however it probably should). At that moment, the comparator is given wrong structures to compare.

I'll transfer this issue to the ORM project.

[beberlei]

This can be fixed on your end by doing @Column(options={'unsigned': true}, ...).