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

[6.x] Float database types fix #30891

Merged
merged 1 commit into from
Dec 23, 2019

Conversation

mnabialek
Copy link
Contributor

At the moment it's impossible to change unsigned float/decimal columns to signed using Laravel Blueprint methods. This change fixes the problem

This is alternative implementation to #30890 where signature of methods was updated.

For consitency same change was done for double however it's impossible to update double colum out of the box using doctribe/dbal.

Proof of problem:

1st migration:

Schema::create('users', function (Blueprint $table) {
    $table->bigIncrements('id');
    $table->integer('a')->unsigned();
    $table->float('b')->unsigned();
    $table->decimal('d')->unsigned();
});

2nd migration:

Schema::table('users', function (Blueprint $table) {
    $table->integer('a')->change();
    $table->float('b')->change();
    $table->decimal('d')->change();
});

Run first migration and then run migrate --pretend as a result you get:

ALTER TABLE users CHANGE a a INT NOT NULL, CHANGE b b DOUBLE PRECISION UNSIGNED NOT NULL, CHANGE d d NUMERIC(8, 2) UNSIGNED NOT NULL

As you see integer column was changed to signed, however float and decimal weren't. After fix result of this is:

ALTER TABLE users CHANGE a a INT NOT NULL, CHANGE b b DOUBLE PRECISION NOT NULL, CHANGE d d NUMERIC(8, 2) NOT NULL

as expected.

@GrahamCampbell
Copy link
Member

Thanks for the PR. I think this would probably have to go to 7.x, instead of 6.x, since it's a breaking change.

@mnabialek
Copy link
Contributor Author

Is it not a bug fix? When we have columns in database and don't use unsigned when altering them they should not be unsigned anymore.

@taylorotwell taylorotwell merged commit 7924520 into laravel:6.x Dec 23, 2019
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

Successfully merging this pull request may close these issues.

3 participants