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

Schema comparison logic is lossy by design #6653

Open
morozov opened this issue Dec 19, 2024 · 0 comments
Open

Schema comparison logic is lossy by design #6653

morozov opened this issue Dec 19, 2024 · 0 comments

Comments

@morozov
Copy link
Member

morozov commented Dec 19, 2024

Q A
Version 4.2.x

Even though the Schema object has its tables and sequences indexed by their qualified names, the schema comparison uses unqualified names for the objects in the default schemas.

dbal/src/Schema/Schema.php

Lines 186 to 195 in bb16de1

private function normalizeName(AbstractAsset $asset): string
{
$name = $asset->getName();
if ($asset->getNamespaceName() === null) {
$name = $this->getName() . '.' . $name;
}
return strtolower($name);
}
foreach ($newSchema->getTables() as $newTable) {
$newTableName = $newTable->getShortestName($newSchema->getName());
if (! $oldSchema->hasTable($newTableName)) {

This is logic is lossy by definition and therefore leads to conflicts. Here's a reproducer on PostgreSQL (a similar scenario could be used to reproduce the issue on SQLite and SQL Server as well):

$schemaConfig = new SchemaConfig();
$schemaConfig->setName('test');

$schema = new Schema([], [], $schemaConfig);

$table1 = $schema->createTable('t');
$table1->addColumn('id', Types::INTEGER);

$table2 = $schema->createTable('public.t');
$table2->addColumn('id', Types::INTEGER);

$this->connection->createSchemaManager()
    ->migrateSchema($schema);

// SQLSTATE[42P07]: Duplicate table: 7 ERROR:  relation "t" already exists

From the source schema's perspective where the default namespace is test, the t and public.t names are different because t resolves to test.t, public.t is already qualified, and "test.t" ≠ "public.t" .

By default, the default PostgreSQL schema is public. When the comparator compares the schemas, it applies Table::getShortestName() to table names and drops the default names namespace from the name. Thus, it turns test.t back into t. Eventually, the schema manager attempts to create both tables t and public.t and fails because they both resolve to the same qualified name public.t in the destination database.

The library should use unqualified names only in its consumer-facing API and only as a convenience feature (similar to SQL). Once an unqualified name has been resolved to a qualified one, this information should not be discarded. From the lossiness perspective, the current logic is as absurd as dropping units before comparing lengths and concluding that 5 feet is greater than 3 meters.

If the destination platform supports namespaces, the comparator should use qualified names for comparing schema objects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant