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

Unexpected type change when renaming a column #45254

Closed
hafezdivandari opened this issue Dec 10, 2022 · 1 comment
Closed

Unexpected type change when renaming a column #45254

hafezdivandari opened this issue Dec 10, 2022 · 1 comment

Comments

@hafezdivandari
Copy link
Contributor

hafezdivandari commented Dec 10, 2022

  • Laravel Version: 9.43.0
  • PHP Version: 8.0
  • Database Driver & Version: MySQL 8.0

Description:

Some unexpected type changes occurs when trying to rename a column with one of the following types:

Schema::create('bars', function (Blueprint $table) {
    $table->mediumInteger('foo');
});

Schema::table('bars', function (Blueprint $table) {
    $table->renameColumn('foo', 'bar'); // changes the name but also changes its type to INT
});
Column type Expected Actual Notes
timestamp('foo', 2) TIMESTAMP(2) DATETIME Changes type and removes precision
mediumInteger('foo') MEDIUMINT INT Changes type
tinyInteger('foo') TINYINT TINYINT(1) Changes type to Boolean
dateTime('foo', 2) DATETIME(2) DATETIME Removes precision
time('foo', 2) TIME(2) TIME Removes precision
year('foo') YEAR DATE Changes type #38506
double('foo') DOUBLE(8, 2) DOUBLE PRECISION Removes total and places
float('foo') DOUBLE(8, 2) DOUBLE PRECISION Removes total and places
set('foo', ['bar', 'qux']) SET('bar', 'qux') LONGTEXT COMMENT '(DC2Type:simple_array)' Changes the type and adds a comment
enum('foo', ['bar', 'qux']) ENUM('bar', 'qux') VARCHAR(255) Documented on Laravel docs as not supported

Additional notes:

  1. I just tested MySQL, but I believe there are similar issues on other databases.
  2. Some modifiers are also get removed when renaming a column like useCurrentOnUpdate and invisible. I didn't check all of them.
  3. The nullable modifier compiles to default null when renaming. I'm not sure if there is a difference between INT NULL and INT DEFAULT NULL on MySQL. Also when using nullable()->default(4) modifiers together, we expect INT NULL DEFAULT 4 but we get INT DEFAULT 4 instead.
  4. Renaming all spatial column types (geometry, point, lineString, polygon, geometryCollection, multiPoint, multiLineString and multiPolygon) throws an exception.
  5. Obviously all dependent column types like unsignedMediumInteger, unsignedTinyInteger, mediumIncrements, tinyIncrements etc. have the same problem.

Conclusion

I tried to list all issues of renameColumn, but some of these are also apply to modifying columns using change and some other unexpected behavior occurs when using change. I tried to fix some of them (#44101, #41320 and #43541) but still have issues like #44912. I wish we could get rid of doctrine/dbal and do column renaming and modifying natively (on Laravel 10 maybe?) or via a new Laravel first-party package, specially with many deprecations on upcoming doctrine/dbal v4: https://github.com/doctrine/dbal/blob/4.0.x/UPGRADE.md

Steps To Reproduce:

  1. Rename a column with one of types listed above.

You may use the following test to reproduce the issue;

use Illuminate\Database\Schema\Blueprint;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Illuminate\Support\Facades\DB;
use Tests\TestCase;

class RenameColumnTest extends TestCase
{
    use RefreshDatabase;

    public function test_rename_column()
    {
        $connection = DB::connection('mysql');

        $connection->getSchemaBuilder()->create('bars', function (Blueprint $table) {
            $table->timestamp('foo', 2)->nullable()->useCurrentOnUpdate();
            $table->mediumInteger('baz')->nullable()->default(4);
        });

        $blueprint = new Blueprint('bars', function (Blueprint $table) {
            $table->renameColumn('foo', 'bar');
            $table->renameColumn('baz', 'qux');
        });

        $queries = $blueprint->toSql($connection, $connection->getSchemaGrammar());

        $expected = [
            'ALTER TABLE bars CHANGE foo bar TIMESTAMP(2) ON UPDATE CURRENT_TIMESTAMP(2) NULL',
            'ALTER TABLE bars CHANGE baz qux MEDIUMINT NULL DEFAULT 4',
        ];

        $this->assertEquals($expected, $queries);
    }
}
@driesvints
Copy link
Member

I feel your pain but I don't feel we're responsible here as DBAL is the package we use to handle these. Like you said it would be better if we could rely on native renaming but as said on the PR that would lead to breaking changes? I'm sorry but I don't think we can do anything ourselves here specifically.

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

No branches or pull requests

2 participants