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

Clean up utf8 parameter usage #6868

Merged

Conversation

sbulen
Copy link
Contributor

@sbulen sbulen commented Jul 22, 2021

Fixes #6163
Closes #6494

The upgrader & installer already had mechanisms to set proper collation (utf8_general_ci), but they weren't working under certain circumstances. Tidying up this existing logic is cleaner than trying to implement partial utf8mb4 support. Utf8mb4 support should wait for #6409 .

Upgrader:
If $db_character_set were not utf8, and, you were upgrading into a DB with a utf8mb4 default charset/collation, you would get key too long errors as reported in #6163 . Note for this to happen you must not be utf8 already, and you must not migrate to a new Settings file (because doing so sets $db_character_set to utf8....).

This change prevents you from falling back to the DB default, which might use utf8mb4 (for new tables only, like qanda).

Installer:
utf8_support is now a function, no longer a variable, so any empty check was erroneous (function exists vs what the function returned...). Fixing them still yielded complex if statements that were trying to determine if we were using utf8.

But... we're using utf8 in 2.1... So the if statements could go.

The bottom line is that in 2.1, the installer and the upgrader should NEVER fall back to using the db-level default collation/charset.

@sbulen sbulen added this to the Final milestone Jul 22, 2021
@pr-triage pr-triage bot added the PR: draft label Jul 22, 2021
@sbulen sbulen marked this pull request as ready for review July 23, 2021 01:59
@sbulen
Copy link
Contributor Author

sbulen commented Jul 23, 2021

Testing completed. Installs & upgrades tested in mysql 8. Also did installs & upgrades in Windows as well as linux, under circumstances that produced the initial bug report. All OK.

@Sesquipedalian Sesquipedalian merged commit 02ae3b1 into SimpleMachines:release-2.1 Aug 11, 2021
@pr-triage pr-triage bot added the PR: merged label Aug 11, 2021
@sbulen sbulen deleted the no_more_utf8_checkbox branch August 11, 2021 05:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

quanda table lngfile key too long
2 participants