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

Indexes need to add the db prefix for multiple projects in one db #7889

Closed
lucidlemon opened this issue Mar 5, 2015 · 20 comments
Closed

Indexes need to add the db prefix for multiple projects in one db #7889

lucidlemon opened this issue Mar 5, 2015 · 20 comments
Labels

Comments

@lucidlemon
Copy link

I'm using bican/roles and realized that laravel is not adding the db prefix to db indexes and foreign keys. This leads to the problem that you can't have two indexes with the same name in one mysql database, as it is not allowed to have two in a single database. Even if you use different db prefixes like I do.
So could we prefix the $table->foreign() keys with the db prefix?

@franzliedke
Copy link
Contributor

Agreed.

@di5abled
Copy link

di5abled commented Mar 6, 2015

👍 totally agree, having the same issue.
on my own migrations I can handle this manually but when I want to install package migrations I'm stuck...

and we have a usecase where we have to work on one database :-/

@GrahamCampbell
Copy link
Member

Is anyone able to do anything about this?

@franzliedke
Copy link
Contributor

I think we cannot easily change the index name creation routine, as that would break when removing old indices that were created with the old routine. Any ideas how to do this in a backward-compatible way?

@taylorotwell Would this be okay in the next major release?

@GrahamCampbell
Copy link
Member

Would this be okay in the next major release?

All minor releases in laravel are the equivalent on a major version bump. An actual major version bump isn't planned for a while, and such a version bump will probably mark the end of php 5 support.

@fisharebest
Copy link
Contributor

If we are going to change the logic for generating index names, we should also fix the problem of long identifier names at the same time.

MySQL identifiers can be a maximum of 64 characters. The current algorithm can easily generate names longer than this.

prefix_table_name_first_column_name_second_column_name_third_column_name_unique

One way is to use a hash of this generated value. Doctrine does this. See their implementation here:
https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Schema/AbstractAsset.php#L219

@fisharebest
Copy link
Contributor

Any ideas how to do this in a backward-compatible way?

function dropIndex {
  try {
    drop new_index_name
  } catch {
    drop old_index_name
  }
}

@franzliedke
Copy link
Contributor

@GrahamCampbell Was this taken care of?

@GrahamCampbell
Copy link
Member

No. It was closed due to 0 interest from the community. I can re-open it again if you like.

@di5abled
Copy link

👍 I also think this should be fixed

@leemason
Copy link
Contributor

leemason commented Feb 4, 2016

same here, especially as many people are starting to use multi tenancy packages and different connections for cloud based storage.

i came across this myself again when using the same db with different prefixes.

@EliasZ
Copy link
Contributor

EliasZ commented Feb 19, 2016

If the index names are going to be hashed, should there be a default dependency on doctrine/dbal again? Or copy over the hashing function (MIT)? Use a custom algorithm?

@laravel-internals
Copy link

Sorry, we don't process requests/proposals here. Please see https://github.com/laravel/internals/issues.

@EliasZ
Copy link
Contributor

EliasZ commented Mar 25, 2016

@taylorotwell @GrahamCampbell @laravel-internals This is not a proposal but rather a bug with the table prefix. Either remove the prefix option all together (which has my preference), or fix the naming of the index. Right now, prefix functionality is broken.

@GrahamCampbell
Copy link
Member

Sorry about that.

@jerguslejko
Copy link
Contributor

any updates on this?

@spacemudd
Copy link

I encountered same issue.

Currently manually assigning the index names but doing so over 50+ migrations is cumbersome.

@Eskaaa
Copy link

Eskaaa commented Jun 11, 2018

I run into the same Issue. I could handle the problem with the prefix. But a permanent solution would be great.

@staudenmeir
Copy link
Contributor

What about a config option to enable this functionality? That would be a backward-compatible solution.

@laurencei
Copy link
Contributor

Hi all - I've done a PR for this - we'll see if Taylor accepts or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests