DiffGenerator: fix applying regex pattern filter to mappings schema
| 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.
@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.
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.
I don't know why there is a filter at the migrations level. @goetas , maybe you know?