bastinald / laravel-automatic-migrations

Automatic Laravel model migrations.
42 stars 11 forks source link

Duplicate Forign Key Constraint Name issue #5

Open AlexJames-Dev opened 3 years ago

AlexJames-Dev commented 3 years ago

Heya, just started out a fresh project using your livewire-ui package, however i am encountering an issue with the automatic migrations. It doesnt seem to be handling a foreign key constraint properly when checking for differences in the table. I can migrate initially, however even with no changes, i run the migrate:auto command and it seems to try to re-add the foreignId as i get an error that the constraint already exists. Not sure if im just missing something obvious or if this is a bug!

Video of issue

Using Sail Docker dev environment using MySQL 8.0.

Cheers!

bastinald commented 2 years ago

hey, for edge cases like this you could use traditional migrations.

can you post the code you're using in your migration method with the foreign keys?

it might be the way it creates a temp table which is causing the dupe.

AlexJames-Dev commented 2 years ago

Yeah, i've done that, as a stop gap, just had to swap the order around in the vendor file, to run the traditional migrations after the automatic migrations (could potentially be a case for an extra flag on the migrate:auto command?).

Sure thing, the code for the Courses table is:

public $migrationOrder = 3;

    public function migration(Blueprint $table)
    {
        $table->id();
        $table->string('name');
        $table->foreignId('college_id')->constrained()->cascadeOnDelete();
        $table->date('start_date');
        $table->date('end_date');
        $table->longText('notes');
        $table->foreignId('user_id')->constrained();
        $table->timestamp('created_at')->nullable();
        $table->timestamp('updated_at')->nullable();
        $table->softDeletes();
    }

and the Colleges table it is referencing:

public $migrationOrder = 2;

    public function migration(Blueprint $table)
    {
        $table->id();
        $table->string('name');
        $table->string('location');
        $table->timestamp('created_at')->nullable();
        $table->timestamp('updated_at')->nullable();
        $table->softDeletes();
    }

Let me know if you need anything more! Many thanks.

marstell-sw commented 2 years ago

Hi, I have this issue, too. It's a problem similar to Issue #3

I noted two problems:

1) when you add a foreign key to a table, it also creates an index on that field which has a random name. This makes problems in the comparison on subsequent migrations. --> You have to add ->index() on the field. Rewrite your code such as:

$table->foreignId('college_id')->constrained()->cascadeOnDelete();
$table->index('college_id')   // <--------

or

$table->foreignId('college_id')->index();  // <--------
$table->foreign('college_id)->references('colleges')->on('id')->cascadeOnDelete();

2) the first time you run the migration, you create the temp table "table_X", e.g. "table_colleges" and then it's renamed to "colleges". But it has created the "table_colleges_Y_index", not "colleges_Y_index" and then you have again errors the next migrations (see Issue #3 linked up above). The solution is to rewrite the code in MigrateAutoCommand.php to create the table directly if not exists, without passing by the temp_table.

        $modelTable = $model->getTable();
        $tempTable = 'table_' . $modelTable;
        $tableExists = Schema::hasTable($modelTable);  // <--------

        Schema::dropIfExists($tempTable);
        Schema::create($tableExists?$tempTable:$modelTable,  // <--------
                    function (Blueprint $table) use ($model) {
            $model->migration($table);
        });

        if ($tableExists) {    // <--------
            $schemaManager = $model->getConnection()->getDoctrineSchemaManager();
            $modelTableDetails = $schemaManager->listTableDetails($modelTable);
            $tempTableDetails = $schemaManager->listTableDetails($tempTable);

          //see Issue #3  // <--------
            foreach ($tempTableDetails->getIndexes() as $indexName=>$indexInfo){

                if ($indexInfo->isPrimary()) continue;
                $correctIndexName=str_replace('table_','',$indexName);
                $tempTableDetails->renameIndex($indexName, $correctIndexName);

            }  //see Issue #3  // <--------
            $tableDiff = (new Comparator)->diffTable($modelTableDetails, $tempTableDetails);

           [ ... ]

        } else {
            //Schema::rename($tempTable, $modelTable);      // <--------

            $this->line('<info>Table created:</info> ' . $modelTable);
        }

It works for me

Bye, Marco

salmanhijazi commented 2 years ago

Thank you, Marco! Your code did the trick. I place the file, below, under Console > Commands and it automatically got picked up and worked! Will remove this file once a fix is added to the repo. I'm using the one under legodion/lucid, though.

MigrateCommand.php

<?php

namespace App\Console\Commands;

use Legodion\Lucid\Commands\MigrateCommand as MigrateCommandOriginal;
use Doctrine\DBAL\Schema\Comparator;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;

class MigrateCommand extends MigrateCommandOriginal
{

    protected function migrateModel(Model $model)
    {
        $modelTable = $model->getTable();
        $tempTable = 'table_' . $modelTable;
        $tableExists = Schema::hasTable($modelTable);

        Schema::dropIfExists($tempTable);

        Schema::create($tableExists ? $tempTable : $modelTable, function (Blueprint $table) use ($model) {
            $model->migration($table);
        });

        if ($tableExists) {
            $manager = $model->getConnection()->getDoctrineSchemaManager();
            $modelTableDetails = $manager->listTableDetails($modelTable);
            $tempTableDetails = $manager->listTableDetails($tempTable);

            foreach ($tempTableDetails->getIndexes() as $indexName => $indexInfo) {
                $correctIndexName = str_replace('table_','',$indexName);
                $tempTableDetails->renameIndex($indexName, $correctIndexName);
            }

            $diff = (new Comparator)->diffTable($modelTableDetails, $tempTableDetails);

            if ($diff) {
                $manager->alterTable($diff);
                $this->line('<info>Table updated:</info> ' . $modelTable);
            }

            Schema::drop($tempTable);
        } else {
            $this->line('<info>Table created:</info> ' . $modelTable);
        }
    }
}