migrations icon indicating copy to clipboard operation
migrations copied to clipboard

DiffGenerator: fix applying regex pattern filter to mappings schema

Open cristi-contiu opened this issue 1 year ago • 3 comments

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

Summary

DiffGenerator applies regex pattern filtering to the metadata-based schema ($toSchema) using DBAL's schemaAssetsFilter (and DoctrineBundle's doctrine.dbal.schema_filter, if applicable), which was meant for filtering the introspected schema ($fromSchema ) to prevent dropping existing tables from the database (DBAL and DoctrineBundle already cover this). This is also explained in the docs : "custom tables which are not managed by Doctrine" = not in the Doctrine mappings, so if an asset is in the metadata ("managed by Doctrine"), it should be included in both $toSchema and $fromSchema (if it exists), regardless of doctrine.dbal.schema_filter config.

This extra filtering also generates differences in the diff created by Migrations vs the ones created by ORM's SchemaTool, as explained in #1487 .

The logic not to exclude tables in the introspected schema that exist in the mappings was ported from https://github.com/doctrine/orm/pull/7875 and prevents adding unwanted "CREATE TABLE" queries.

cristi-contiu avatar Feb 14 '25 12:02 cristi-contiu

@greg0ire My use case is the DiffCommand being called without an explicit --filter-expression , but the DBAL already having a SchemaAssetsFilter callback configured (by DoctrineBundle and the doctrine.dbal.schema_filter config).

Failing test and pipeline

There are differences in the way Migrations and DBAL apply the filter: DBAL applies it during schema introspection on what it thinks it's the most accurate table name using "portableTableDefinition" which has platform-specific checks like schemas support or current / default search-path schemas (for example some_schema.my_table on PostgreSQL), while Migrations removes the schema and applies the filter only on the unqualified table name (for the same table as earlier, it would be only my_table).

The Migrations library should at least apply the regex filtering on the exact table name provided by the user in the mappings, it should not assume that some_schema.my_table and my_table reference the same table . I suggested a fix that keeps the schema name for platforms that support schemas.

cristi-contiu avatar Feb 27 '25 21:02 cristi-contiu

Then there is the broader issue (addressed in this PR): if the DiffGenerator should manipulate/filter the mapped $toSchema at all (the ORM SchemaTool does not).

The metadata schema does not have the advanced platform-specific checks that the introspected schema has, and any apparent differences are addressed by the Comparator during diff.

For example, a mapped table with name public.my_table OR a table with both name my_table and explicit schema/namespace public OR a table just with name my_table can reference the same table in PostgreSQL, but filtering just by the provided name as string without platform-specific knowledge would produce mixed results. A safer approach would be to remove that table from the mappings instead on relying on Migrations library to filter it out.

cristi-contiu avatar Feb 28 '25 07:02 cristi-contiu

I don't know why there is a filter at the migrations level. @goetas , maybe you know?

greg0ire avatar Feb 28 '25 17:02 greg0ire