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

Token length = 50 #44

Merged

Conversation

CharlyPoppins
Copy link
Contributor

@CharlyPoppins CharlyPoppins commented Aug 24, 2018

Q A
Bug fix? yes
New feature? no
BC breaks? yes
Deprecations? no
Tests pass? yes
License MIT

I had an issue try to create the table on a MySql datatable. There was a problem creating the Unique index : #1071 - Specified key was too long; max key length is xxx bytes

Solve the problem by reducing varchar length.

@CharlyPoppins
Copy link
Contributor Author

Hello, any reason to not merge this PR ?

@vincentchalamon
Copy link
Contributor

Hello @CharlyPoppins,
As tests run on Sqlite, they don't reveal this issue. I've been using this bundle since its creation under Sqlite, MySQL & Postgres without any problem.
Can you give more informations about this issue? Which version of MySQL do you use ?
Thanks

@CharlyPoppins
Copy link
Contributor Author

Got the issue with mysql Ver 14.14 Distrib 5.5.61-38.13, for debian-linux-gnu (x86_64) using readline 5.1

Also while trying to solve the problem I read that I shouldn't be indexing such long varchar columns because the index will be very bulky and inefficient.

According to that, I saw that the generated token was 50 caracters long and I adjusted the varchar lenght.

@vincentchalamon vincentchalamon merged commit f1573c3 into coopTilleuls:master Oct 10, 2018
@vincentchalamon
Copy link
Contributor

Thanks @CharlyPoppins. As it's a BC break, it will be tagged in 1.2.0 soon

@CharlyPoppins CharlyPoppins deleted the feature/token-length branch October 10, 2018 13:00
@CharlyPoppins CharlyPoppins restored the feature/token-length branch October 10, 2018 13:01
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.

2 participants