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

[5.7] Support index length on MySQL #25200

Closed
wants to merge 1 commit into from
Closed

[5.7] Support index length on MySQL #25200

wants to merge 1 commit into from

Conversation

staudenmeir
Copy link
Contributor

This PR allows MySQL migrations to specify the index length:

Schema::create('table', function(Blueprint $table) {
    $table->string('a');
    $table->string('b');
    $table->text('c');
    $table->primary(['a', 'b'], null, null, [10, 20]);
    $table->index('c', null, null, 50);
});

This is especially useful for TEXT columns, as they don't allow indexes without a length.

PostgreSQL, SQLite and SQL Server don't support index lengths. There is a workaround for PostgreSQL, but it only supports non-unique indexes and is cumbersome to use in queries. So I don't think it's worth the effort.

I didn't add exceptions for the other databases because I don't think they are necessary for a such a minor feature.

Resolves #9293.

@sisve
Copy link
Contributor

sisve commented Aug 14, 2018

PostgreSQL, SQLite and SQL Server don't support index lengths.

I think SQLite supports it via expression indexes, but I havn't looked into it.

I didn't add exceptions for the other databases because I don't think they are necessary for a such a minor feature.

I disagree with this part.

@mfn
Copy link
Contributor

mfn commented Aug 14, 2018

I didn't add exceptions for the other databases because I don't think they are necessary for a such a minor feature.

This is one of the major pain points with the current migrations and the Fluent approach: unsupported stuff totally goes unnnoticed until you realize later it didn't work.

Great example from practice: you switch from MySQL to Postgres.

You may say "edge case", I say unnecessary pain point because of "laziness" (note: this isn't meant a diss of your work 👍 , just how it might turn out in the end).

@taylorotwell
Copy link
Member

No plans to add this at this time.

@driesvints
Copy link
Member

@staudenmeir @sisve @mfn we're reconsidering this for Laravel 7. Would any of you be up to create a PR for the master branch? I think support for other DB engines is wanted as well.

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.

None yet

5 participants