Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update generated diff to match ORM schema diff #1488

Closed

Conversation

cristi-contiu
Copy link
Contributor

@cristi-contiu cristi-contiu commented Feb 9, 2025

Q A
Type improvement
BC Break no/minor
Fixed issues #1487

Summary

Ports doctrine/orm#7875 into Migrations to improve consistency between queries generated by Migrations diff and the ones generated by the ORM schema update/validate commands. In most cases, the queries where already identical, but in some rare cases (like #1487), the migrations generated from diff did not contain the same queries as the ORM schema update and validate commands.

It also removes filtering the schema generated from metadata ($toSchema) using DBAL's schemaAssetsFilter and the doctrine.dbal.schema_filter config, which should only be used to exclude tables in the schema generated from the existing database ($fromSchema - DBAL and DoctrineBundle already cover this). This might be considered a minor BC-break, but if the metadata contains unwanted tables, they shouldn't be included in the mappings in the first place.

The decoration of the existing schemaAssetsFilter from the DBAL configuration is done in doctrine/orm#7875 to whitelist assets present in the $toSchema so that they don't get removed from the $fromSchema by the default schemaAssetsFilter (usually based on the doctrine.dbal.schema_filter config) - a difference which would lead to always generating CREATE TABLE queries.

I think this last part is best tested in a functional/integration test, but I would need some guidance on how to set it up.

If it's a wanted change, I can also add a note to UPGRADE.md, depending on the branch it would be included in and if it's considered a minor BC break.

@cristi-contiu cristi-contiu marked this pull request as ready for review February 9, 2025 20:52
@cristi-contiu cristi-contiu deleted the align-diff-with-orm branch February 14, 2025 10:57
@cristi-contiu cristi-contiu restored the align-diff-with-orm branch February 14, 2025 10:57
@cristi-contiu
Copy link
Contributor Author

cristi-contiu commented Feb 14, 2025

Accidentaly closed this PR, opened a new one here: #1490

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant