SchemaTool: Better way to manipulate Column definitions

[flack]

Feature Request

Q	A
New Feature	yes
RFC	don't know
BC Break	depends :-)

Summary

I am maintaining a Doctrine based emulation layer for a different ORM API. For various backwards compatibility reasons, among other things I replace DBAL's DateTimeType with my own implementation. This gets set in my MappingDriver, and to allow for schema updates, it also needs to be set in the return value of SchemaTool::getSchemaFromMetadata. To my knowledge, the only way to do this is to register a listener for onSchemaColumnDefinition which instantiates the column with the needed type. It looks more or less like this:

    public function onSchemaColumnDefinition(SchemaColumnDefinitionEventArgs $args)
    {
        $column = array_change_key_case($args->getTableColumn(), CASE_LOWER);
        $type = strtok($column['type'], '()');

        if ($type == 'datetime') {
            $options = [
                'default' => $column['default'] ?? null,
                'notnull' => $column['null'] != 'YES',
            ];

            $args->preventDefault();
            $args->setColumn(new Column($column['field'], Type::getType(MyVerySpecialDateTime::TYPE), $options));
        }
    }

If you use such a listener, you will have to call $args->preventDefault(), because otherwise your changes will get immediately overwritten:

dbal/lib/Doctrine/DBAL/Schema/AbstractSchemaManager.php

Lines 811 to 821 in 628e846

	if (null !== $eventManager && $eventManager->hasListeners(Events::onSchemaColumnDefinition)) {
	$eventArgs = new SchemaColumnDefinitionEventArgs($tableColumn, $table, $database, $this->_conn);
	$eventManager->dispatchEvent(Events::onSchemaColumnDefinition, $eventArgs);
	$defaultPrevented = $eventArgs->isDefaultPrevented();
	$column = $eventArgs->getColumn();
	}
	if ( ! $defaultPrevented) {
	$column = $this->_getPortableTableColumnDefinition($tableColumn);
	}

This means that AbstractSchemaManager::_getPortableTableColumnDefinition never gets called on your custom column, so none of the regular normalization code runs and you will instead have to copy whatever you need for the platforms you support into your onSchemaColumnDefinition listener.

It would be a lot more maintainable if there was a way to use all the work DBAL is doing and then only tweak the resulting $column (or maybe you could manipulate _getPortableTableColumnDefinition's $tableColumn parameter before it gets called?). One idea that came up in #4671 was to maybe pass $column as an argument to the onSchemaColumnDefinition callback, another was to have a ColumnProvider interface that users could implement.

[morozov]

For various backwards compatibility reasons, among other things I replace DBAL's DateTimeType with my own implementation.

Is it the primary goal of your listener? In this case, you can try using Type::overrideType('datetime', MyVerySpecialDateTime::class) instead.

[flack]

So I did a bit of code archaeology, it seems I added the datetime code in 2014 to an already existing event listener which translates ENUM fields from legacy databases. I guess I didn't notice Type::overrideType back then (I did have some code with Type::addType, but that only works if there isn't a datetime type already. I've switched to Type::overrideType now and it seems to work fine.

I guess I will still need the listener for the ENUM stuff, because DBAL probably can't help me there, right? But also, ENUM is probably something where you actually don't want the normalization to run, so maybe all is well then?

[morozov]

I guess I will still need the listener for the ENUM stuff, because DBAL probably can't help me there, right?

You'll have to be more specific. What translation does the listener perform?

[flack]

nothing fancy, really. I sets the type to string and saves the original type information in the comment field:

    public function onSchemaColumnDefinition(SchemaColumnDefinitionEventArgs $args)
    {
        $column = array_change_key_case($args->getTableColumn(), CASE_LOWER);
        $type = strtok($column['type'], '()');

        if ($type == 'enum') {
            $options = [
                'length' => 255,
                'default' => $column['default'] ?? null,
                'notnull' => $column['null'] != 'YES',
                'comment' => $column['type']
            ];

            $args->preventDefault();
            $args->setColumn(new Column($column['field'], Type::getType(Types::STRING), $options));
        }
    }

[morozov]

Thanks, @flack. This looks like a valid, generic enough case where you'd just like to override the default behavior. IMO, it deserves a BC break and an API change since the current API doesn't allow it to do it reasonably.

[UncertaintyP]

Just stumbled upon this in my own project also because of a custom DateTime implementation.
A non BC break would be to change the visibility of _getPortableTableColumnDefinition to public in which case one could call this method from the event listener through $eventArgs->getConnection()->createSchemaManager()->_getPortableTableColumnDefinition and mimic the default behavior + applying changes afterwards.