Closed thbighead closed 4 years ago
Same issue, solved migration by commenting : https://github.com/michaeldrennen/Geonames/blob/ca4c481d14db3dabf40fbbb9b71df64940b58e6d/src/Migrations/2017_02_11_145951_create_geonames_alternate_names_table.php#L70 and https://github.com/michaeldrennen/Geonames/blob/ca4c481d14db3dabf40fbbb9b71df64940b58e6d/src/Migrations/2017_03_25_141711_create_geonames_admin_1_codes_table.php#L32
Open file config/database.php and change the strict mode to false under 'mysql'.
Thanks @thbighead for posting the issue!
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.
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.
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 :) )
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:Digging through the migration code I found that it was thrown into 2017_02_11_145951_create_geonames_alternate_names_table.php line 70:
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 codeSchema::defaultStringLength(191)
may be inserted intoboot
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 queryinggeonames_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