dbal icon indicating copy to clipboard operation
dbal copied to clipboard

Changing precision/scale value has no effect in schema comparison when not extendend from DecimalType

Open gregor-tb opened this issue 3 years ago • 2 comments

Bug Report

Q A
BC Break no
Version 3.1.3

Summary

We have a custom "money_value" type, to set decimal with predefined values for precision + scale like numeric(10,4). This works on first migration, but changes (e.g. to 11,4) are not recognized.

The type class looks like this:

class MoneyValueType extends ApplicationDoctrineType
{
    public const NAME = 'money_value';

    /**
     * {@inheritdoc}
     */
    public function getSQLDeclaration(array $column, AbstractPlatform $platform)
    {
        $column['precision'] = 11;
        $column['scale'] = 4;

        return $platform->getDecimalTypeDeclarationSQL($column);
    }

    /**
     * {@inheritdoc}
     */
    public function getName()
    {
        return self::NAME;
    }
}

This should do it, SQL declaration returns numeric(11,4)... But DBAL has a trap in Comparator.php:

https://github.com/doctrine/dbal/blob/96b0053775a544b4a6ab47654dac0621be8b4cf8/src/Schema/Comparator.php#L482-L490

The documentation does not require to extend specific Type classes (https://www.doctrine-project.org/projects/doctrine-dbal/en/latest/reference/types.html#custom-mapping-types).

If precision and scale are irrelevant for non-decimal types, they should not have default values at all and the magic "10" should be set in DecimalType.

There is a similar problem with fixed as well.

gregor-tb avatar Nov 10 '21 17:11 gregor-tb

@gregor-tb please try reproducing your scenario on 3.2.x-dev. The issue may have been fixed by https://github.com/doctrine/dbal/pull/4746.

morozov avatar Nov 13 '21 20:11 morozov

Sorry for late answer, no the issue is still present in 3.x branch.

We made a patch on our side to replace instanceof Types\DecimalType with an interface (DecimalTypeInterface), which we think is a good solution similar to PhpDateMappingType.

gregor-tb avatar Feb 15 '24 09:02 gregor-tb