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

[9.x] Enhance column modifying #44101

Merged
merged 3 commits into from
Sep 13, 2022
Merged

[9.x] Enhance column modifying #44101

merged 3 commits into from
Sep 13, 2022

Conversation

hafezdivandari
Copy link
Contributor

@hafezdivandari hafezdivandari commented Sep 12, 2022

This PR does:

  • Add support for changing column type to double
  • Add support for changing column type to tinyText.
  • Fix a issue when changing column type to timestamp:
    • The SQL contained charset/collate: TIMESTAMP SET utf8mb4 COLLATE utf8mb4_unicode_ci
  • Fix a issue when changing column type to binary:
    • on MySQL, the type was LONGBLOB instead of expected BLOB

PS: I added tests for all adds and changes.
PS: changes to binary reverted.

@driesvints
Copy link
Member

Won't this change the behavior for apps who have already run their migrations in production?

@hafezdivandari
Copy link
Contributor Author

@driesvints if they have already run their migrations, how this change the behavior of those apps?

Currently, when adding a binary column it is always a BLOB equivalent column. but when changing to binary column it depends on the previous column's length and default to LONGBLOB if it has no length. for example:

Schema::create('tests', function (Blueprint $table) {
    $table->string('string_to_binary');
    $table->integer('integer_to_binary');
    $table->binary('binary_column'); // is always `BLOB`
});

Schema::table('tests', function (Blueprint $table) {
    $table->binary('string_to_binary')->change();  // changes to `TINYBLOB` because it already has the length of 255.
    $table->binary('integer_to_binary')->change(); // changes to `LONGBLOB` because it already has no length.
});

Please check this function on Doctrine DBAL;

I think it is inconsistent behavior but if you think this is the expected behavior or is a breaking change, I can revert changes to binary column modifying. Please let me know. Other changes are not breaking at all.

@driesvints
Copy link
Member

Their migrations which are part of their app will still be run in development. If they run them in development after this PR is merged, their development databases will be out if sync with production.

@hafezdivandari
Copy link
Contributor Author

OK, I reverted changes on binary I'll send that to master later.

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