Preventing incorrect column renames no longer possible in 4.0 about dbal

[ausi]

why do you want to avoid detecting column renames?

We (Contao) use the doctrine schema manager to update the database schema after updates or after installing an extension. In this case the user can not alter the result of the comparator (like it would be possible using doctrine migrations) so if an incorrect rename was detected there would be no way to intervene. (Most users would probably also not recognise the wrong rename). We had many issues with this in the past (see also #3661) but we since used the workaround as outlined in #6299 (comment) and this worked flawlessly ever since.

And would the problem you have disappear with #6280 ?

No, as the rename detection is still in place and there is no option to disable it.
But I think #6280 is a very good feature as it makes intentional renames possible 👍

[derrabus]

Is there a better way to do this? I basically just want to disable Comparator::detectRenamedColumns() so that it never detects a rename of a column.

We could make that configurable. Detecting renames is fuzzy, so the requirement to disable that feature seems reasonable to me. Up for a PR?

[derrabus]

Yes. However, we could think about backporting the API to 3.x if that makes the upgrade from 3.x to 4.x easier. But let's target 4.1.x first.

[ausi]

… I'm thinking of an alternative, which would be to tag dropped columns, when doing dropColumn, as excluded from detection with a flag on Column …

The problem is we never do a manual dropColumn. We simply compare the live schema with the desired one like $comparator->compareSchemas($schemaManager->introspectSchema(), $schemaTool->getSchemaFromMetadata(…)) and need to make sure this never results in any renames.
(in our case, intentional renames would be done “manually” before the schema update)

[ausi]

Up for a PR?

Sure. That would be for version 4.1.0 I guess, right?

[ausi]

Done: #6300

[greg0ire]

@ausi why do you want to avoid detecting column renames? And would the problem you have disappear with #6280 ?

[Tofandel]

My PR does not remove the rename detection even if an explicit renaming has been provided (maybe it should?) but if a column is explicitly renamed then those 2 particular columns shouldn't be taken into account during the detection

[Tofandel]

Since the detection is made from dropped columns and added columns of the same type, I'm thinking of an alternative, which would be to tag dropped columns, when doing dropColumn, as excluded from detection with a flag on Column (which already has a list of options available)

[Tofandel]

You could still iterate over the tables and columns in the old schema to set the flag

Or an in between, we add a configuration in the SchemaConfig of Table so the iteration is only on the tables

Otherwise I don't have anything against the comparator config but I do think it should indeed be as a state to limit the signature changes, the comparator class is not final and only the constructor is internal, so it's still allowed to be extended it seems which means a signature change of public or protected methods would indeed be a breaking change

[ausi]

Up for a PR?

Done: #6300

Addressed all review comments there, so this should be ready. 🎉