Make column and index renaming configurable

[ausi]

Q	A
Type	improvement
Fixed issues	#6299

Summary

Makes column and index rename detection of the Comparator configurable, as suggested in https://github.com/doctrine/dbal/issues/6299#issuecomment-1930097815

[github-actions[bot]]

There hasn't been any activity on this pull request in the past 90 days, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days. If you want to continue working on it, please leave a comment.

[ausi]

I adressed all review comments.

Adressing this issue is quite important for us in order to get compatible with Doctrine 4.

[ausi]

I think this lacks documentation,

Should I add a section for createComparator() to docs/en/reference/schema-manager.rst ? Or should it be a new page about the Comparator itself?

and I'm curious to see what a real-world usage would look like, since "The comparator can be only instantiated by a schema manager" (so, at runtime, rather than at DIC compilation time).

We would use it here: https://github.com/contao/contao/blob/aa54c3df007e4b01b73e08036e35c054a7d1fd84/core-bundle/src/Migration/CommandCompiler.php#L61-L66 like this:

$comparator = $schemaManager->createComparator();
$config = new ComparatorConfig();
$config->setDetectRenamedColumns(false);
$config->setDetectRenamedIndexes(false);
$comparator->setConfig($config);
$diffCommands = $comparator
    ->compareSchemas($fromSchema, $toSchema)
    ->toSql($this->connection->getDatabasePlatform())
;

Are you happy with the setConfig() approach? I think I’d prefer a third parameter to the methods compareTables and compareSchemas instead, see https://github.com/doctrine/dbal/pull/6300#discussion_r1480230124

[greg0ire]

I would drop setConfig, and add a new argument to createComparator, so that the config can be readonly. Having it mutable doesn't feel great.

[ausi]

Should I make the Config object itself also immutable?

[greg0ire]

I think everything should be made immutable by default, yes. If people need to mutate anything, they will send a PR explaining their use case, making it mutable won't be a breaking change.

[greg0ire]

Should I add a section for createComparator() to docs/en/reference/schema-manager.rst ?

A section feels enough

[ausi]

I think this lacks documentation

Done in e117bbbfbd9a6968c5d7466d0f12398a279c79e4

[ausi]

Failing CI seems to be unrelated.

[ausi]

As this didn’t make it into the 4.1.0 release, should I change the base branch to 4.2.x?

[derrabus]

Done. And please rebase your PR and fix the red CI checks.

[ausi]

Done.

[ausi]

…and fix the red CI checks.

Only the “Upload coverage to Codecov” job fails now. I’m not sure how to fix that.

[derrabus]

Only the “Upload coverage to Codecov” job fails now.

You can ignore that one.