Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 97 additions & 1 deletion core/Command/Db/ConvertType.php
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,8 @@ protected function execute(InputInterface $input, OutputInterface $output): int
}
}
$intersectingTables = array_intersect($toTables, $fromTables);
$intersectingTables = $this->sortTablesByForeignKeys($toDB, $intersectingTables);

$this->convertDB($fromDB, $toDB, $intersectingTables, $input, $output);
return 0;
}
Expand Down Expand Up @@ -265,6 +267,9 @@ protected function clearSchema(Connection $db, InputInterface $input, OutputInte
if (!empty($toTables)) {
$output->writeln('<info>Clearing schema in new database</info>');
}

$toTables = $this->sortTablesByForeignKeys($db, $toTables, true);

foreach ($toTables as $table) {
$db->createSchemaManager()->dropTable($table);
}
Expand Down Expand Up @@ -340,7 +345,7 @@ protected function copyTable(Connection $fromDB, Connection $toDB, Table $table,
for ($chunk = 0; $chunk < $numChunks; $chunk++) {
$query->setFirstResult($chunk * $chunkSize);

$result = $query->executeQuery();
= $query->executeQuery();

try {
$toDB->beginTransaction();
Expand Down Expand Up @@ -400,6 +405,97 @@ protected function getColumnType(Table $table, $columnName) {
return $this->columnTypes[$tableName][$columnName];
}

/**
* Sort tables so that tables referenced by foreign keys are copied
* before the tables containing those foreign keys.
*
* The dependency information is obtained from the target database,
* making this independent of the source/target database vendor.
Comment on lines +412 to +413

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this works because the schema is already applied to the target DB?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, this code runs after the createSchema or when running in the clearSchema. Only problem could be if you use the clearSchema option if there never was a migration database setup before, but in that case you should just not add that option.

*
* @param Connection $connection Target database connection
* @param array<string> $tables Tables to sort
* @param bool $dependenciesFirst Whether to place FK dependencies before dependent tables
* @return array<string> Tables in dependency order
*/
protected function sortTablesByForeignKeys(Connection $connection, array $tables, bool $dependenciesFirst = false): array {
$tableSet = array_fill_keys($tables, true);

// dependencies[table] = tables that must be copied before it
$dependencies = array_fill_keys($tables, []);

// dependents[table] = tables that depend on it
$dependents = array_fill_keys($tables, []);

$schemaManager = $connection->createSchemaManager();

foreach ($tables as $table) {
foreach ($schemaManager->listTableForeignKeys($table) as $foreignKey) {
$foreignTable = $foreignKey->getForeignTableName();

// Ignore references to tables which aren't being converted.
if (!isset($tableSet[$foreignTable])) {
continue;
}
Comment on lines +435 to +438

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should never happen, because then something is seriously wrong.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even thought other things might be seriously wrong isn't it best to catch this in this code?


// Ignore self-references. They don't impose an ordering
// requirement on the table itself.
Comment on lines +440 to +441

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how we copy tables, but could this mean that rows also need to be copied in the correct order by dependency?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the case that a table self-references itself yes. But that seemed out of scope for this fix. In my case I didn't have self-referencing tables and I'm not sure if there are apps using those.

if ($foreignTable === $table) {
continue;
}

$dependencies[$table][$foreignTable] = true;
$dependents[$foreignTable][$table] = true;
}
}

/*
* Kahn's topological sort.
*
* Tables without dependencies can be copied immediately.
*/
$readyTables = [];

foreach ($tables as $table) {
if ($dependencies[$table] === []) {
$readyTables[] = $table;
}
}

$sortedTables = [];

while ($readyTables !== []) {
$table = array_shift($ready);
$sortedTables[] = $table;

foreach (array_keys($dependents[$table]) as $dependent) {
unset($dependencies[$dependent][$table]);

if ($dependencies[$dependent] === []) {
$readyTables[] = $dependent;
}
}
}

/*
* A cycle means there is no valid topological ordering.
*
* Don't silently produce an invalid ordering. Keep the original
* order for the remaining tables; PostgreSQL may still reject
* the conversion, but the failure will accurately expose the
* cyclic dependency rather than being hidden by this sorter.
*/
if (count($sortedTables) !== count($tables)) {
$remaining = array_diff($tables, $sortedTables);
$sortedTables = array_merge($sortedTables, $remaining);
}

if ($dependenciesFirst) {
$sortedTables = array_reverse($sortedTables);
}

return $sortedTables;
}

protected function convertDB(Connection $fromDB, Connection $toDB, array $tables, InputInterface $input, OutputInterface $output) {
$this->config->setSystemValue('maintenance', true);
$schema = $fromDB->createSchema();
Expand Down