Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate removal of namespaced assets from schema #5432

Merged
merged 1 commit into from
Jun 6, 2022

Conversation

morozov
Copy link
Member

@morozov morozov commented Jun 4, 2022

Q A
Type deprecation

Silently dropping database objects from the schema looks like a disservice. If the application declares certain schema objects, it means that it expects them to exist in the resulting schema, so it won't work properly if these objects don't exist.

It will be easier to spot such issues if the visitor drops a table (all the queries against it will fail) but the problems may be way harder to diagnose if a foreign key is dropped. This way, the application will rely on the referential integrity being preserved by the database but in fact it the database won't have the expected constraints in place.

According to the GH6823Test in the ORM, there is a flaw in this logic:

// The table may already be deleted in a previous
// RemoveNamespacedAssets#acceptTable call. Removing Foreign keys that
// point to nowhere.
if (! $this->schema->hasTable($fkConstraint->getForeignTableName())) {
$localTable->removeForeignKey($fkConstraint->getName());

The entities being deployed in the test don't declare a namespaced object name but the visitor drops a valid foreign key because the referenced entity (GH6823Tag) isn't being deployed as part of the schema.

The following statement is missing as a result of processing schema by the visitor:

ALTER TABLE gh6823_user_tags ADD FOREIGN KEY (tag_id) REFERENCES gh6823_tag (id)

@morozov morozov marked this pull request as ready for review June 4, 2022 15:56
@morozov morozov requested review from greg0ire and derrabus June 4, 2022 15:56
@morozov morozov added this to the 3.4.0 milestone Jun 4, 2022
@morozov morozov merged commit 91c0aa7 into doctrine:3.4.x Jun 6, 2022
@morozov morozov deleted the deprecate-remove-namespaced-assets branch June 6, 2022 18:45
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants