laravel / framework

The Laravel Framework.
https://laravel.com
MIT License
32.56k stars 11.03k forks source link

Migrating a varchar to a bigint causes invalid charset instructions to appear #30539

Closed James34Shaw100 closed 4 years ago

James34Shaw100 commented 5 years ago

Description:

While writing a migration to fix a bad database that had stored a foreign key as a varchar, I wrote the following migration:

Schema::table('jobseeker_qualifications', function (Blueprint $table) {
        $table->bigIncrements('id')->change();
        $table->unsignedBigInteger('user_id')->change();
        $table->unsignedBigInteger('jobseekers_education_id')->change();
        $table->foreign('user_id')->references('id')->on('users');
        $table->foreign('jobseekers_education_id')->references('id')->on('jobseeker_educations');
});

However, this above migration generated the following SQL:

ALTER TABLE jobseeker_qualifications
CHANGE id id BIGINT UNSIGNED AUTO_INCREMENT NOT NULL,
CHANGE jobseekers_education_id jobseekers_education_id BIGINT UNSIGNED CHARACTER SET utf8 NOT NULL COLLATE `utf8_unicode_ci`,
CHANGE user_id user_id BIGINT UNSIGNED NOT NULL

Work Around: Removing the instructions related to the character set and running this query manually was successful.

Steps To Reproduce:

  1. Create a table containing a VARCHAR field that contains valid id values (only numbers).
  2. Run a migration similar to the above, requesting a change the VARCHAR to unsigned BIGINT.
rimace commented 5 years ago

What was the error message?

James34Shaw100 commented 5 years ago

What was the error message?

SQL Error (1064): You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'CHARACTER SET utf8 NOT NULL COLLATE `utf8_unicode_ci`, CHANGE user_id user_id B' at line 3

Those character set and collate instructions should not have been there.

staudenmeir commented 5 years ago

What version of the doctrine/dbal package are you using (run composer show)?

James34Shaw100 commented 5 years ago

What version of the doctrine/dbal package are you using (run composer show)?

Here: doctrine/dbal v2.10.0

staudenmeir commented 5 years ago

Can you try downgrading to 2.9.3?

It looks like this is the same bug: https://github.com/doctrine/dbal/issues/3714

James34Shaw100 commented 5 years ago

Can you try downgrading to 2.9.3? It looks like this is the same bug: doctrine/dbal#3714

It does indeed look like the same bug, I'll subscribe to that bug tracker to see when it's been fixed and I could update this ticket in turn.

I don't have the time at the moment to try the downgrade due to work pressures and the manual script work around is functional for the moment, but if I get a chance to I shall look in to it.

driesvints commented 5 years ago

Seems like this is related to DBAL and not Laravel so going to close this one.

Justin991q commented 4 years ago

DBAL just closed the issue: see https://github.com/doctrine/dbal/issues/3714#issuecomment-558573089

JohnyProkie commented 4 years ago

Can please folks from Laravel and folks from Doctrine make an agreement and push this issue a bit further? https://github.com/doctrine/dbal/issues/3714#issuecomment-559585932

Laravel says it's issue on Doctrine's side. Doctrine says it's issue on Laravel side. Meanwhile we are locked on older version of Doctrine when we want to keep our projects up-to-date. Thank you in advance.

Please note this happened to us when changing varchar column to integer.

lcobucci commented 4 years ago

Hello everyone, I'm one of the core members of the Doctrine team 👋

As I mentioned on the issue in DBAL, I completely understand your frustration and am sorry for this inconvenience.

We really believe that the issue is related to how Laravel configures the DBAL objects to perform the comparisons and think that it was working previously due to a bug that got fixed.

I don't know Laravel so much to solve this but feel free to ping me in a PR or on Doctrine's slack channel and I'll do my best to support the resolution of this issue.

@JohnyProkie thanks for pointing us here and @driesvints and other Laravel maintainers for their hard work on building bridges between the two projects 🤟

JohnyProkie commented 4 years ago

@lcobucci Thank you a lot for stepping in!

driesvints commented 4 years ago

Let's figure this out. If anyone could help pinpoint where we can implement a fix that'd be great.

@lcobucci thanks for your hard work on Doctrine as well :)

lcobucci commented 4 years ago

@driesvints I haven't debugged anything but https://github.com/laravel/framework/blob/1bbe5528568555d597582fdbec73e31f8a818dbc/src/Illuminate/Database/Schema/Grammars/ChangeColumn.php#L125-L127 might give us a clue.

Laravel should make sure to not set the charset/collation info on the objects while performing the changes.

driesvints commented 4 years ago

@lcobucci it does seem that only applies to json and binary columns while the issue here is changing a column type from varchar to bigint so I'm not sure if that's the actual culprit?

https://github.com/laravel/framework/blob/1bbe5528568555d597582fdbec73e31f8a818dbc/src/Illuminate/Database/Schema/Grammars/ChangeColumn.php#L124-L128

lcobucci commented 4 years ago

@driesvints that's the problem. As @AlterTable explained it:

But when it's original type was varchar/text, there is a implicit change: the charset definition should be removed. Neither Laravel nor Doctrine handled this. I think there are two ways to fix this, one is add extra checks in \Doctrine\DBAL\Schema\Table::changeColumn, another is modify Laravel migrate, add checks after changed attributes are set. I prefer the second one.

Laravel should define characterset and collation as null to remove that info.

Does this help?

driesvints commented 4 years ago

@lcobucci heya, it does. I think this is indeed the correct way to go forward. I currently don't have time to work on this so if anyone is experiencing this issue and wants to move this forward, feel free to send in a PR.

yahya09 commented 4 years ago

Hi all, if anyone on Laravel 6.x & doctrine/dbal 2.10 experiencing this issue, the quick fix is to set charset and collation to empty string on column change statement. For example:

$table->integer('resource_type')->default(0)->charset('')->collation('')->change();

It does work on my case where previous column type is string (varchar). Looks like someone has already sent PR to fix this, but I hope solution above still help before the PR merged.

squatto commented 4 years ago

Hi all, if anyone on Laravel 6.x & doctrine/dbal 2.10 experiencing this issue, the quick fix is to set charset and collation to empty string on column change statement. For example:

$table->integer('resource_type')->default(0)->charset('')->collation('')->change();

It does work on my case where previous column type is string (varchar). Looks like someone has already sent PR to fix this, but I hope solution above still help before the PR merged.

This unfortunately isn't working for me for some reason. I've tried null and '' for charset and collation and I still get the same error. I'm going from string to bigInteger:

$table->bigInteger('number')
      ->charset('')
      ->collation('')
      ->change();

I finally ended up just running the raw query:

\DB::statement('alter table orders modify number bigint not null');
taylorotwell commented 4 years ago

Fixed by https://github.com/laravel/framework/commit/fccdf7c42d5ceb50985b3e8243d7ba650de996d6

strtz commented 4 years ago

still present in: "php": "^7.2", "laravel/framework": "^6.0", "doctrine/dbal": "^2.10",

error while trying to change column from text to blob

workaround for me was to make a intermediary migration that runs above the alter and drop the column and recreate it for both fwd and reverse migration

Doctrine\DBAL\Driver\PDOException::("SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'CHARACTER SET utf8mb4 DEFAULT NULL' at line 1") /var/www/****/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/PDOConnection.php:66

osbre commented 4 years ago

@strtz try to update dependencies

vannakdev commented 4 years ago

Fixed by fccdf7c

It worked thanks @taylorotwell