support doctrine dbal 4

[dbu]

Description

For now, the symfony/orm-pack locks doctrine/dbal to version 3 because Symfony relies on the deprecated schema extension events that have been removed in dbal 4.

The events are used in MessengerTransportDoctrineSchemaListener (in the end to add a postgres notifier for symfony/messenger).

Typo3 replaces the platform driver to use custom extended platform implementations that can add to the schema. I do not yet see where exactly we could do that for the notifier - the typo3 changes e.g. in PostgreSQLSchemaManager are to tweak how columns are defined. for the notifier, we need to add completely custom SQL outside of the table definition.

Before i dig deeper, i wanted to ask for input on this, opinions or pointers where this should happen, and where in symfony we should replace the classes.

Notes:

• MessengerTransportDoctrineSchemaListener (and its test) is the only place in all of symfony/symfony in the 7.1 branch that mentions any DBAL\Event things. fixing this one place will be enough.

[dbu]

thanks for the input on doctrine slack @sbuerk

for symfony, i think we need to extend PostgreSQLPlatform and overwrite getCreateTableSQL with a custom platform that adds the notify sql bit. and provide a CustomPlatformDriverMiddleware and CustomPlatformDriverDecorator similar to typo3.

however, where to put that code? it probably should be in doctrine-bridge. but then, do we change doctrine/doctrine-bundle to load our middleware?

and having the logic about messenger mixed into the doctrine code seems highly specific. should we re-introduce some symfony event that the messenger can listen to? or some alternative mechanism to events to handle this?

[stof]

Maybe Doctrine DBAL misses a way to inject custom logic in this schema management in a composition-based extension point. Having to extend the platform class is OK for projects (assuming they don't need to support multiple platforms) but not really usable for the use case of Symfony.

[derrabus]

for symfony, i think we need to extend PostgreSQLPlatform

Symfony must not override the platform classes unless we use a dedicated connection just for the messenger. Otherwise, the application won't be able to use their own platforms anymore.

[dbu]

Otherwise, the application won't be able to use their own platforms anymore.
thats what i am worying about too.

using a dedicated connection for messenger would lead to very annoying configuration when sharing the same database and having some entities and such, as schema diff would not understand what is going on. and we would duplicate the connection configuration which is not ideal either.

i gave it some more thought but don't see any good solutions.

we could overrite the platforms in the doctrine-bridge to re-add the event system and then use the symfony event system in messenger-doctrine. that feels a bit odd however, and i guess there are some architectural reasons why dbal got rid of the events, so re-adding them does not seem great either.

alternatively we would need to figure out with doctrine dbal if there is a way to add an extension point to schema generation without extending the platform classes.

[greg0ire]

i guess there are some architectural reasons why dbal got rid of the events

Yes, the reasons are detailed here: doctrine/dbal#5784

[Shoplifter]

If I update doctrine/orm to version 3 that causes an update of dbal to v. 4 as well.
When I did that, LoginFormAutherticator did not work anymore. Is that related with this issue?

[derrabus]

If I update doctrine/orm to version 3 that causes an update of dbal to v. 4 as well.

No. It enables the update, but you can choose to remain on DBAL 3.

When I did that, LoginFormAutherticator did not work anymore. Is that related with this issue?

No. Please use our discussions board for further support questions. Thank you.

[thePanz]

On an additional note: after migrating to orm:3.1 and dbal:4.0.1 I noticed that the command
bin/console doctrine:schema:validate
reported that a new migration DROP TABLE doctrine_migration_versions was added.

Probably this is something that should be checked when implementing the support to dbal v4.x

[dbu]

reported that a new migration DROP TABLE doctrine_migration_versions was added.

thanks for reporting this @thePanz

that sounds like the https://github.com/doctrine/DoctrineMigrationsBundle has the same problem with dbal as we see for messenger. i think this report belongs into the migrations bundle repository, afaik the symfony repository does not interact with migrations. can you report it there (and link the discussion here?) - i am still hoping for more input from the dbal maintainers. if i understand correctly, the migrations example shows quite nicely that we need some more tooling as that bundle can not just extend all database drivers to add its logic.

[greg0ire]

@dbu that issue is being tracked at least here.

I'm not sure this is really the exact same issue as with messenger. If it is, the migrations issue seems fixable by leveraging asset filtering.

[beberlei]

I believe one solution would be to introduce new extension points in SchemaTool instead, one each for createSchema, updateSchema, dropSchema right at the end. This could provide enough context for other tools to add their own schema management at this point.

[dbu]

extension points on the SchemaTool sounds like a good idea to me.

the important part is to have events or register something on the SchemaTool and not require to use class inheritance as extension point. do you prefer events or registering handlers on the SchemaTool? if its not very urgent, i can try to propose something later this week once i know which direction i should take.

[greg0ire]

@beberlei @dbu it seems @wmouwen is already proposing something like this: doctrine/orm#11374 albeit just for one event. Would that fit your needs / fit in your proposal?

[dbu]

thanks for the note. we used to interact with the Doctrine\DBAL\Event\SchemaCreateTableEventArgs event. should we add such an event in the SchemaTool of the orm?

if i understand correctly, looking at the full schema would be cumbersome, but i can try to see if that would be doable too.

[stof]

The new event proposed in the ORM won't help here: it does not allow changing the generated SQL, which is what is done for messenger.

[dbu]

@greg0ire would it make sense if i add something to https://github.com/doctrine/orm/blob/3.1.x/src/Tools/Event/GenerateSchemaTableEventArgs.php to allow adding additional sql there? or should we do another event to allow to add sql when the schema for a table is translated into sql statements?

can that be done within orm or do i need to change something in dbal to make that possible?

[greg0ire]

I'm not sure… why is custom SQL needed in the first place? If it's because DBAL doesn't have a proper object representation of what's created by that custom SQL, then there are going to be issues when diffing, right? How is that issue fixed with DBAL 3?

[dbu]

the operation is to add a "trigger":

symfony/src/Symfony/Component/Messenger/Bridge/Doctrine/Transport/PostgreSqlConnection.php

Line 95 in 73f8713

public function getExtraSetupSqlForTable(Table $createdTable): array

i don't know how exactly it worked with dbal3 - i think that the extra sql was also added when doing the diff and made it so there was no diff detected when there were no other changes.

[greg0ire]

The diff that is performed is a diff between schemas, right? Does DBAL really allow introspecting triggers, and exposes object representing them in its API? Or do such things actually never come up when introspecting a database into a Schema object?

[beberlei]

The new event proposed in the ORM won't help here: it does not allow changing the generated SQL, which is what is done for messenger.

The listener is supposed to execute their own sql, an api that adds more SQL strings is not planned anymore

[dbu]

so would we have to listen for migration events and run the extra sql each time a migration happens? (or query the database to decide if we have to run the sql...)

i really liked how this was able to hook into migrations, both for transparency for the user and to avoid database change decisions at runtime.

[dbu]

we are discussing an event in orm here: doctrine/orm#11409