DiffGenerator: fix applying regex pattern filter to mappings schema#1490
DiffGenerator: fix applying regex pattern filter to mappings schema#1490cristi-contiu wants to merge 1 commit intodoctrine:3.8.xfrom
Conversation
src/Generator/DiffGenerator.php
Outdated
|
|
||
| $schemaAssetsFilter = $this->dbalConfiguration->getSchemaAssetsFilter(); | ||
| if ($previousFilter === null) { | ||
| return $this->schemaManager->introspectSchema(); |
There was a problem hiding this comment.
This is kept for compatibility with DBAL v3. In DBAL v4, it can no longer be null. Might trigger a codecov warning
3936acf to
cd81404
Compare
| } | ||
|
|
||
| return preg_match($filterExpression, $assetName); | ||
| return (bool) preg_match($filterExpression, $assetName); |
There was a problem hiding this comment.
Casting to bool for docs and DoctrineBundle alignment
0ae9dc5 to
92b5f6c
Compare
92b5f6c to
7a187c9
Compare
greg0ire
left a comment
There was a problem hiding this comment.
Can you please add a failing test?
|
@greg0ire My use case is the DiffCommand being called without an explicit 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 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 |
|
Then there is the broader issue (addressed in this PR): if the DiffGenerator should manipulate/filter the mapped 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 |
|
I don't know why there is a filter at the migrations level. @goetas , maybe you know? |
Summary
DiffGenerator applies regex pattern filtering to the metadata-based schema (
$toSchema) using DBAL'sschemaAssetsFilter(and DoctrineBundle'sdoctrine.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$toSchemaand$fromSchema(if it exists), regardless ofdoctrine.dbal.schema_filterconfig.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 doctrine/orm#7875 and prevents adding unwanted "CREATE TABLE" queries.