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

"1071 Specified key was too long" will never be resolved with Schema::defaultStringLength(191) #30

Closed
thbighead opened this issue Dec 12, 2019 · 3 comments
Assignees
Labels

Comments

@thbighead
Copy link

thbighead commented Dec 12, 2019

The problem

I'm testing ^2.0 version of this package and found to install exactly the version 2.0.2 after requiring it through Composer.

When I ran the php artisan migration I'm getting the following error:

Illuminate\Database\QueryException :
SQLSTATE[42000]:
Syntax error or access violation:
1071 Specified key was too long; max key length is 1000 bytes
(SQL: alter table `geonames_alternate_names` add index `geonames_alternate_names_alternate_name_index`(`alternate_name`))

Digging through the migration code I found that it was thrown into 2017_02_11_145951_create_geonames_alternate_names_table.php line 70:

$table->index( 'alternate_name' );

If we comment this line the Artisan's migration command will run without any errors. But I went even deeper into the problem:

Some explanation

The error says that the max key length is 1000 bytes and the above highlighted line is trying to create an index key for the alternate_name column. This column is defined in the same migration file at line 37 as a VARCHAR with length 400 (!!!). When we define the length of a column explicitly how it is done at this line, the code Schema::defaultStringLength(191) may be inserted into boot method of AppServiceProvider.php and will just be ignored (at least by this line) resulting on the same error because the basic Laravel collation is utf8mb4 which uses 4 bytes to each char on a VARCHAR column and 400 * 4 = 1600 bytes which is much greater than the maximum size of 1000 bytes to a key (like specified by the error).

What I recommend to solve it

I understand that creating an index to alternate_name column is useful to make filters when querying geonames_alternate_names table. But this table isn't important to everyone using this package (also depending on how and why we are using it there may have more tables which will not be important though). Also, this column stores something like a CSV data, so it could be parsed and stored in another way to avoid creating a VARCHAR(400).

Also, why not let us publish the migrations and commands and change them as we need?

Don't misunderstand me, the package is awesome, it just need a bit of improvement

@thbighead thbighead changed the title 1071 Specified key was too long will never be resolved with Schema::defaultStringLength(191) "1071 Specified key was too long" will never be resolved with Schema::defaultStringLength(191) Dec 12, 2019
@spout
Copy link

spout commented Jan 4, 2020

Same issue, solved migration by commenting :


and

@librafiredev
Copy link

Open file config/database.php and change the strict mode to false under 'mysql'.

https://i.imgur.com/ezinPdn.png

@michaeldrennen michaeldrennen self-assigned this Mar 30, 2020
@michaeldrennen
Copy link
Owner

Thanks @thbighead for posting the issue!

Suggested Improvement to Laravel

Yeah, this could be solved by Laravel allowing us to specify a prefix length for the string indexes.
Reading through the issues posted on the Laravel repository...
Link to GitHub issue: Ability to define index key length while creating schema

It seems like that is being discussed, but no idea when they will get it implemented.

Solution

So for our purposes, I have added some logic around the creation of the two troubled indexes in the migrations.
For example: Link to relevant code: Alternate Names Table Migration

The index will only be created, if the database driver is 'mysql' and it doesn't have an environment variable set indicating that its being run as part of a continuous integration service.

I am using travis-ci.org and it was throwing an error about the syntax I was using. Since these indexes aren't needed as part of the unit tests, I am not creating them under those circumstances.

Regarding indexes you might not need

It was suggested that I could just not create the indexes at all. Some people just won't need them. A fair point. But I need them. ;)
So it would be just as easy to suggest that each user can decide for themselves what indexes they want to delete and which they want to keep. (...as long as creating them doesn't throw an exception :) )

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

4 participants