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

Dependence in order of calling change() and nullable() in Laravel 11 for modified columns (database migrations) #50705

Closed
Jhnbrn90 opened this issue Mar 21, 2024 · 3 comments

Comments

@Jhnbrn90
Copy link

Jhnbrn90 commented Mar 21, 2024

Laravel Version

11.0.8

PHP Version

8.2.13

Database Driver & Version

MySQL 8.0

Description

In upgrading to Laravel 11, I've discovered an interesting artifact regarding the section on Modifying Columns. The order of calling change() and nullable() seem to matter and $table->string('column_a')->nullable()->change() seems not equivalent to $table->string('column_a')->change()->nullable().

In my project I have these two migrations:

  • add_some_nullable_column_to_table:
      $table->string('my_column')->after('some_other_column')->nullable();
    
  • change_datatype_of_column
      $table->longText('my_column')->change();
    

The first migration simply adds a column of type string to the table, the following migration changes this datatype from string to longtext. From the earlier mentioned upgrade guide, it is expected that the nullable property is dropped.

Now, the interesting part: when updating the second migration to include the nullable() property as mentioned in the documented example, the column remains the type of NOT NULL in the database upon inspection.

What I did find is that changing the order of calling nullable() and change() does work! So using the following migration does actually work to obtain the correct datatype column which is nullable:

     $table->longText('my_column')->change()->nullable();

Disclaimer

If this is actually expected/desired behavior, just the example on the documentation page might need an update.

Steps To Reproduce

Reproducing the error

Using the provided Laravel version, create two simple migrations that 1) add a new nullable column to an existing table; 2) update the type of the column and try to make it nullable by using the example as mentioned on the Laravel 11 upgrade page:

Schema::table('users', function (Blueprint $table) {
    $table->integer('votes')
        ->unsigned()
        ->default(1)
        ->comment('The vote count')
        ->nullable()
        ->change();
});

When running the migrations, notice that the column does not have a nullable type.

Reproducing the happy path

Now, switch ->nullable()->change(); to ->change()->nullable();.
The column now is actually nullable.

@DanteB918
Copy link

DanteB918 commented Mar 21, 2024

I was unable to repeat this behavior unfortunately. Below are screenshots based on my tests.

->nullable()->change();

image_720

->change()->nullable();

image_720

Just ->change();

image_720

notice that it is only not nullable when i use ->change() alone

This on Laravel version v11.0.8, PHP v8.2.15, MySQL v8.0

Here are what my migrations look like.

Migration 1

    public function up(): void
    {
        Schema::create('test_two', function (Blueprint $table) {
            $table->string('my_column')->nullable();
        });
    }

Migration 2

    public function up(): void
    {
        Schema::table('test_two', function (Blueprint $table) {
            $table->longText('my_column')->nullable()->change();
        });
    }

Let me know if perhaps I tested something inaccurately

@hafezdivandari
Copy link
Contributor

@Jhnbrn90 Just added 2 integration tests on PR #50708 based on your scenario, all tests are passing. What am I missing here?

@Jhnbrn90
Copy link
Author

I have just double checked by swapping the calls in my local test setup and now they do appear to both yield the same result. I'm really sorry for the confusion. This must have been a me-problem.

I'm really sorry for the confusion and want to thank you for your time @hafezdivandari

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

3 participants