delight-im / PHP-Auth

Authentication for PHP. Simple, lightweight and secure.
MIT License
1.08k stars 233 forks source link

InnoDB tables for MySQL #137

Open lsantaniello opened 5 years ago

lsantaniello commented 5 years ago

I need to migrate at InnoDB tables but I have this error

1071 - Specified key was too long; max key length is 767 bytes

Is correct if I execute this script?

ALTER TABLEusers ALTERemailDROP DEFAULT, ALTERpasswordDROP DEFAULT; ALTER TABLEusers CHANGE COLUMNemail`email VARCHAR(150) NOT NULL COLLATE 'utf8mb4_unicode_ci' AFTER id, CHANGE COLUMN password password VARCHAR(150) NOT NULL COLLATE 'latin1_general_cs' AFTER email, CHANGE COLUMN username username VARCHAR(150) NULL DEFAULT NULL COLLATE 'utf8mb4_unicode_ci' AFTER password; `

Thanks

ocram commented 5 years ago

Thanks for the feedback on InnoDB compatibility!

I think you may have to change the two columns users.email and users_confirmations.email from varchar(249) to varchar(191).

Preferably, you would change this right in the schema that you copy from this library and use to create the tables.

Does that work for you?

Any other feedback on how this library works with InnoDB is also much appreciated.

darkalchemy commented 5 years ago

You can increase the index length to a max of 767-1024 chars by adding these to my.cnf and restarting:

innodb_file_format = Barracuda # MariaDB deprecated this in 10.2 and removed in 10.3
innodb_large_prefix = 1 # MariaDB deprecated this in 10.2 and removed in 10.3
innodb_file_per_table = 1

At least this is what I use and it works.

ocram commented 5 years ago

@darkalchemy Thank you! This is a more fundamental and intrusive change than changing the length of the two email columns. So if you’re fine with email addresses being limited to 191 characters (instead of 249 characters), that is probably the easier fix (if it does indeed work).

Again, any other feedback on InnoDB support is also appreciated. If the above is the only problem, that would be great, of course.

darkalchemy commented 5 years ago

With those additions to my.cnf, I still limit column lengths to 255. Also noteworthy, changing innodb_file_per_table = 1 might require an export/drop/import to create the storage properly.

eypsilon commented 3 years ago

I've changed my Tables to InnoDb after setting the email columns to varchar(191) without any problems. But that leads to another question, have you ever seen a real email adress with more than 150 chars? Or 240 to 250? And even if there are a few, these should be more an exception, i guess.

/* only from curiosity, here are just around 150 chars and it already looks like anything else than an email address */ {"Lorem.ipsumndolornsitcahmetxconsetetur.sadipscing.elitr.sed.nonumy.eirmod@temporhinviduntaut.laborenetddolorevmagnacaliquyamceratatatacsedcdiam.com"}

Does that work for you?

I've created, for the sake of curiosity, around 15.000 Users (with an additional "user_settings" table containing some extra columns and data for each User) and i've encountered no problems at all, so: yes. With a Pagination-script, i've also not seen any difference when loading 15 from 15 Users or 15 from 15.000. Just blazing fast.

Any other feedback on how this library works with InnoDB is also much appreciated.

It seems the day will come where you will be forced to change it, wanted or not ;)

https://www.percona.com/blog/2016/10/11/mysql-8-0-end-myisam/

PS: Composer will cause problems in the future because of the Exceptions, that are all combined in "src/Exceptions.php". I've created all of them (21) separately, have thrown them in the "src/" folder, outcommented all the content in "Exceptions.php" and that's all. It looks pretty messed up currently, yes, absolutely. If they might be usefull for you, i can upload the files. I've straight copyed the classes from "src/Exceptions.php" in to separate files.

Composer Notice was:

Deprecation Notice: Class Delight\Auth\AuthException located in /vendor/delight-im/auth/src/Exceptions.php does not comply with psr-4 autoloading standard. It will not autoload anymore in Composer v2.0. in phar://composer/composer.phar/src/Composer/Autoload/ClassMapGenerator.php:201

BTW, great work. Really appreciate it

ocram commented 3 years ago

@eypsilon Thanks a lot for sharing your experience with InnoDB instead of MyISAM here. It’s great to have another confirmation that it’s really only a little extra work that is required for the change. The Composer/exception/namespace problem is already tracked in another issue and will be taken care of shortly.

jadeops commented 1 year ago

Hi, any cheatsheet or docs to convert all PHP-Auth tables to InnoDB on an existing app correctly?