Skip to content

[5.7] Support index length on MySQL#25200

Closed
staudenmeir wants to merge 1 commit intolaravel:5.7from
staudenmeir:index-length
Closed

[5.7] Support index length on MySQL#25200
staudenmeir wants to merge 1 commit intolaravel:5.7from
staudenmeir:index-length

Conversation

@staudenmeir
Copy link
Copy Markdown
Contributor

@staudenmeir staudenmeir commented Aug 14, 2018

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

taylorotwell commented Aug 14, 2018

No plans to add this at this time.

@staudenmeir staudenmeir deleted the index-length branch August 14, 2018 15:05
@arborrow arborrow mentioned this pull request Sep 11, 2018
Closed
@driesvints
Copy link
Copy Markdown
Member

driesvints commented Jan 21, 2020

@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.

@driesvints driesvints mentioned this pull request Jan 21, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@staudenmeir @sisve @mfn @taylorotwell @driesvints