tpetry / laravel-postgresql-enhanced

Support for many missing PostgreSQL specific features
MIT License
772 stars 31 forks source link

Some errors + CI fails on Laravel 11.15 #85

Closed damiencriado closed 4 months ago

damiencriado commented 4 months ago

I've encountered an issue with the latest version of Laravel (11.15) during migrations.

Schema::table('my_table', static function (Blueprint $table) {
    $table->decimal('my_column', 13, 4)->default(0);
    // ...
});
Duplicate column: 7 ERROR: column "my_column" of relation "my_table" already exists

The CI now fails on the latest version of Laravel: https://github.com/damiencriado/laravel-postgresql-enhanced/actions/runs/9915497290

Maybe it has something to do with this change: https://github.com/laravel/framework/pull/51373

Please let me know if you need any further information to diagnose this issue.

hafezdivandari commented 4 months ago

Overridden compileAdd and compileChange methods must reflect the same changes as PostgresGrammar on PR laravel/framework#51373 as I explained here. Sorry for the trouble.

tpetry commented 4 months ago

Fixing this to work with all versions will again be tricky. I'll work on this on Monday.

tpetry commented 4 months ago

I've fixed the CI issues with the newest Laravel 11.x release in 0.40.1.

But your initial issues of the column already existing should be an issue because of breaking change with Laravel 11.15.x behaviour. The issue should also exist when removing my package. From my understanding you need to reorder the schema changes in your migration.

hafezdivandari commented 4 months ago

Hi @tpetry, the problem is actually on the overriden compileAdd I think. After 11.15, we are adding columns one by one, so $command->column should be used instead of $blueprint->getAddedColumns().

tpetry commented 4 months ago

$blueprint->getAddedColumns() is just an alias to $command->columns with some extra logic. So it shouldnt be the problem. And others reported the same issue with your PR.

@damiencriado Can you check whether the issue remains/disappears when removing my package?

hafezdivandari commented 4 months ago

$blueprint->getAddedColumns() is just an alias to $command->columns

$blueprint->getAddedColumns() is an alias for $blueprint->commands (all added columns on blueprint), that should change to new $command->column (single column on the command).

Reproducing: try to add / change 2 columns:

Schema::table('table', static function (Blueprint $table) {
    $table->string('column_one');
    $table->string('column_two');
});
tpetry commented 4 months ago

The example by hafezdivandari shows that indeed the current version is broken because of behavioural bc break in Laravel.

damiencriado commented 4 months ago

$blueprint->getAddedColumns() is just an alias to $command->columns with some extra logic. So it shouldnt be the problem. And others reported the same issue with your PR.

@damiencriado Can you check whether the issue remains/disappears when removing my package?

Hi @tpetry ! I still have errors in my CI with 0.40.1 :(

SQLSTATE[42701]: Duplicate column: 7 ERROR:   column "my_column" of relation "my_table" already exists

And yes, the error is only present with the package.

tpetry commented 4 months ago

Yeah, I have now a working test case that is failing only in that specific version. Working on a fix based on the PR from @hafezdivandari.

tpetry commented 4 months ago

Its now finally fixed with 0.40.2