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

Problem modifying column type from text to mediumText or longText #21847

Closed
antonioribeiro opened this issue Oct 27, 2017 · 12 comments
Closed

Problem modifying column type from text to mediumText or longText #21847

antonioribeiro opened this issue Oct 27, 2017 · 12 comments

Comments

@antonioribeiro
Copy link
Contributor

  • Laravel Version: 5.5.19
  • PHP Version: 7.1.8
  • Database Driver & Version: mysql Ver 14.14 Distrib 5.7.16, for osx10.12 (x86_64)

Description:

Changing columns from text to mediumText or longText changes to the wrong size.

Text is supposed to be 65536 chars long, but the sources are getting information from static::calculateDoctrineTextLength($fluent['type']);:

Which sets text (default) as 255, which is the string length in Laravel:

/**
 * Calculate the proper column length to force the Doctrine text type.
 *
 * @param  string  $type
 * @return int
 */
protected static function calculateDoctrineTextLength($type)
{
    switch ($type) {
        case 'mediumText':
            return 65535 + 1;
        case 'longText':
            return 16777215 + 1;
        default:
            return 255 + 1;
    }
}

Then for doctrine, mediumText is 65536 bytes, which is Text and so on.

So this migration:

    Schema::table('ci_runs', function (Blueprint $table) {
        $table->mediumText('log')->change();
    });

Changes a text column to the very same text length.

Dumping the results from

image

I get

image

Which is also odd, since Doctrine is basically adding one byte to the columns length.

And looking at the table structure in SequelPro, this is before:

image

This is after a migration to longText (went to the biggest to test it):

Schema::table('ci_runs', function (Blueprint $table) {
        $table->longText('log')->change();
});

image

And this is what happens if I manually modify the column:

alter table ci_runs modify column log longtext;

image

@antonioribeiro
Copy link
Contributor Author

Creating it as mediumText:

image

Works fine. So this is how I'm doing it:

public function migrateUp()
{
    Schema::table('ci_runs', function (Blueprint $table) {
        $table->mediumText('log_new')->nullable();
    });

    Database::statement('update ci_runs set log_new = log;');

    Schema::table('ci_runs', function (Blueprint $table) {
        $table->dropColumn('log');
        $table->renameColumn('log_new', 'log');
    });
}

To get

image

To have that column in the correct order, I changed it to

$table->mediumText('log_new')->nullable()->after('was_ok');

@Dimimo
Copy link

Dimimo commented Oct 28, 2017

Very good point. That explains why intervention/imagecache can't hold larger pictures when the cache system is set to database and the cache.value column is set to mediumtext. Me too had to manually intervene in the database to override the Laravel suggested migration change. It's an issue/bug that is there since at least L5.1.

Your solution only works on a test server as log will lose all existing values. A luxury some may not have.

@antonioribeiro
Copy link
Contributor Author

@Dimimo It will loose nothing, it creates the new column, copies old logs to it, then drops the old one and renames it back to log

@sisve
Copy link
Contributor

sisve commented Oct 28, 2017

First off, it isn't Doctrine that is adding the +1 values, that's still in Laravel's code base.

The calculations have been done this way for over three years, since at least Laravel 5.0 / 2014. I tracked it back to 24828f0 but that may not be the first occurrence.

Changing these now will introduce breaking changes to migrations that has worked for a very long time. Every column someone has changed to these types in the last three years will suddenly be changed differently when the migrations are reran in development environments, putting the local environment out-of-sync with production.

My views on any changes to the migration system is as my views to any changes to migrations that have been pushed from the local machine; it should never happen. Migrations need to be stable. Being stable over time and releases is more important, according to me, than being correct.

There are already a lot of red boxes in the migration documentation about weird cases and unsupported things. We could document this behavior in another red box.

@paulofreitas
Copy link
Contributor

paulofreitas commented Nov 11, 2017

I've found that changing any text column type to another text column type does not work at all, even changing Illuminate\Database\Schema\Grammars\RenameColumn::calculateDoctrineTextLength(). It works only when changing from/to a non-text column.

Unfortunately a proper fix to this would require mapping every type not supported by Doctrine. This seems to be the same issue from #8840. I haven't found yet, but maybe there's an easier way to fake those types to force Doctrine to change the columns correctly. 🤔

@cbotsikas
Copy link

This is not an issue of Laravel. dbal has the issue. It ignores the TextType length property when comparing the columns so it doesn't know it actually changed in order to generate the appropriate DDL. You can find more info at doctrine/dbal#2566 (comment). Hopefully it will be sorted out soon, you are more than welcome to jump in the discussion. @antonioribeiro, i like your workaround (#21847 (comment)), looks cleaner that using Alter db statements.

@driesvints
Copy link
Member

Since this seems to be an issue with dbal I'm going to close this here.

@88chico
Copy link

88chico commented Jan 16, 2019

Another workaround that I`ve found.
change the column to string (VARCHAR) type with the length of 70000 (or bigger than 65536)
like this:
$table->string('data', 70000)->change();
since the limit of varchar in doctrine/dbal is 65535, it change it to mediumtext.
on the same way if you need longtext you can put the length as 16777216 or bigger.
(see MySqlPlatform->getClobTypeDeclarationSQL function)

I know it is not elegant or pretty, but it is a working workaround..

@uphlewis
Copy link

@88chico You're my hero

@ryancwalsh
Copy link

@88chico Thank you! I wish "doctrine/dbal" would fix this bug reported >2 years ago, but until they do, your hack seemed to work for my purposes.

@bhulsman
Copy link

bhulsman commented Apr 9, 2019

Check solution proposed here: doctrine/dbal#2566 (comment), very nice one!

@jeromejaglale
Copy link

jeromejaglale commented Jun 12, 2020

The solution mentioned above by @bhulsman is the easiest workaround: making a change to the column's comment triggers the type change:

// up
$table->mediumText('log')->comment(' ')->change();

// down
$table->mediumText('log')->comment('')->change();

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