SchemaManager::createSchema wrongly quotes default values on MariaDB(?)

[flack]

Bug Report

Q	A
BC Break	no
Version	2.13.1

Summary

I have a field in the database that looks like this in the mysqldump:

`metadata_exported` datetime NOT NULL DEFAULT '0001-01-01 00:00:00'

I read the database structure like this:

$em->getConnection()->getSchemaManager()->createSchema();

If I dump the field, it looks like this:

(notice the double quotes for the _default value)

When I build the schema with SchemaTool::getSchemaFromMetadata, I get the default without double quotes. so when I feed both of them to Doctrine\DBAL\Schema\Comparator, I get a difference in each date field in the entire database looking like this:

Current behaviour

DBAL thinks that "0001-01-01 00:00:00" is different from "'0001-01-01 00:00:00'" and runs a bunch of pointless updates every time I run comparator

How to reproduce

I can try to come up with some way to reproduce this, but I noticed it only happens on some machines. So far my theory is that the problem exists on MariaDB but not on MySQL, but I'm not a 100% sure

Expected behaviour

createSchema should not produce double quotes when reading the default values form the database

[morozov]

This must be related to how MariaDB represents the default value during schema introspection. See "MariaDB 10.2.7 information changes schema for default values" in #2825.

In theory, this case should be handled by

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

Lines 230 to 241 in 628e846

	private function getMariaDb1027ColumnDefault(MariaDb1027Platform $platform, ?string $columnDefault) : ?string
	{
	if ($columnDefault === 'NULL' || $columnDefault === null) {
	return null;
	}
	if ($columnDefault[0] === "'") {
	return stripslashes(
	str_replace("''", "'",
	preg_replace('/^\'(.*)\'$/', '$1', $columnDefault)
	)
	);
	}

[flack]

@morozov maybe I'm reading the code wrong, but shouldn't the snippet you quoted only replace '' with '? So it would turn ''0001-01-01 00:00:00'' into '0001-01-01 00:00:00', but not '0001-01-01 00:00:00' into 0001-01-01 00:00:00

[morozov]

It's the preg_replace() call that removes the surrounding quotes. There's the DefaultValueTest::testEscapedDefaultValueCanBeIntrospected() that covers this code. I ran it with your example, and it passes on MariaDB.
Here's the DDL being reported back by MariaDB:

create table default_value(
	id int not null,
	metadata_exported datetime default '0001-01-01 00:00:00' null
) collate=utf8_unicode_ci;

[flack]

I've debugged this some more today and noticed that _getPortableTableColumnDefinition (and thus getMariaDb1027ColumnDefault) never gets called for datetime fields.
This is because I have a onSchemaColumnDefinition function that performs some other operations on datetime fields and then calls $args->preventDefault(). So I guess there is no other way than duplicating the normalization code into my event handler function, right?

[morozov]

So I guess there is no other way than duplicating the normalization code into my event handler function, right?

I don't know what exactly your code does and why it disables the default event handler but it sounds right.

[flack]

@morozov well, the thing is if you want to use the callback to set some info for the column, you have to call preventDefault, otherwise your changes are 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);
	}

It would be nice if $column from the event callback would be passed into _getPortableTableColumnDefinition somehow, but I guess that's hard to do in practice, not to mention a BC break

[morozov]

That's indeed a clumsy extension API. If you need to do some customization of the column definitions before they get created, it might be easier to register a custom platform and just override _getPortableTableColumnDefinition() (assuming your application supports only one platform).

Passing the column to the event handler would likely solve your problem but I don't think it's worth an API break. If you feel you want to participate in improving the API, please file another issue and describe your problem there.

[flack]

One idea might be to have _getPortableTableColumnDefinition run before the event gets called and then pass $column as a parameter to SchemaColumnDefinitionEventArgs. Then you could modify (or replace) $column in your callback. If there's interest in something like that, I can put together a ticket for it

[morozov]

I don't really like the idea of events here. The caller of an event handler doesn't usually expect anything to be returned. Using events to implement certain API is a bit weird. It would make more sense to define an interface (e.g. ColumnProvider), create a default implementation, and let users use their own. This way, there will be a single extensible API instead of two APIs that don't achieve the same level of flexibility.

If there's interest in something like that, I can put together a ticket for it.

Please file a case if you believe it's important.

[flack]

alright, I've submitted #4676

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.