Updated DumpSqlCommand to compare against existing database #21

Steveb-p · 2023-07-05T10:04:10Z

Question	Answer
JIRA issue	N/A
Type	improvement
Target Ibexa version	`v4.6`
BC breaks	no

This PR makes it so that schema comparison is executed against current database.

Checklist:

Provided PR description.
Tested the solution manually.
Provided automated test coverage.
Checked that target branch is set correctly (main for features, the oldest supported for bugs).
Ran PHP CS Fixer for new PHP code (use $ composer fix-cs).
Asked for a review (ping @ibexa/engineering).

alongosz

TBH I would prefer some sort of flexibility - being able to both generate dump from the schema file and as a diff.

There's also an issue with Comparator itself - it always shows some changes, even though it shouldn't. For example, on the latest commerce 4.6.x-dev I'm getting the following results when executing the command without file argument: https://gist.github.com/alongosz/e1a3d95a40f819c3c46f7b3781f75622
Moreover when I tried to create custom schema file which defines app_my_table relation, I got the following result: https://gist.github.com/alongosz/8ddceb3d8cf31f16ad6f0d75d97ad493

Those were the reasons I've never introduced schema diff & easy upgrade when working on this package. It's very unstable.

Steveb-p · 2023-07-05T10:54:21Z

@alongosz I would never advocate using this tool to perform an "easy upgrade". The main purpose of it is to generate a list of potential SQL commands, just like doctrine:schema:update --dump-sql command does. An engineer can then use this to check if all his schema changes are included in a proper SQL upgrade script.

The reason why it is unstable are differences in metadata provided by database vendors and the fact that we are already behind in DBAL v3 adoption. When checking their implementation in DBAL I've already noticed a few changes.

Changed in c08028e to allow both current database comparison and straight up schema generation from scratch.

alongosz · 2023-07-05T11:59:07Z

@alongosz I would never advocate using this tool to perform an "easy upgrade". The main purpose of it is to generate a list of potential SQL commands, just like doctrine:schema:update --dump-sql command does. An engineer can then use this to check if all his schema changes are included in a proper SQL upgrade script.

@Steveb-p Do you think then that maybe it's worth to add a warning at the beginning of the execution? Something about making database backup and reviewing the script before applying it. I already can imagine people trying to use it, trusting it's a complete feature with correct output.

I was a bit reluctant to suggest that because without it you could stream the output directly to dbms sql interpreter mysql < $(php ...), but given the nature of the changes, maybe we shouldn't encourage people to do it?

The reason why it is unstable are differences in metadata provided by database vendors and the fact that we are already behind in DBAL v3 adoption. When checking their implementation in DBAL I've already noticed a few changes.

👌