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

Add MySQL 5.7 support #705

Merged
merged 5 commits into from
Feb 20, 2023
Merged

Add MySQL 5.7 support #705

merged 5 commits into from
Feb 20, 2023

Conversation

vladsf
Copy link
Contributor

@vladsf vladsf commented Jan 27, 2023

Add fixes to support MySQL 5.7

@theory
Copy link
Collaborator

theory commented Jan 28, 2023

Thanks. I wonder if we need to include instructions for older versions of MySQL, too? Or is 8 old enough now that we can just standardize on it? If the latter, would you mind adding a note near the top that the examples assume MySQL 8? Thanks!

@theory theory self-assigned this Jan 28, 2023
@theory theory self-requested a review January 28, 2023 19:11
@vladsf
Copy link
Contributor Author

vladsf commented Jan 29, 2023 via email

@vladsf
Copy link
Contributor Author

vladsf commented Jan 29, 2023

I found another problem: ENCRYPT() function was deprecated in MySQL 5.7 and removed in MySQL 8.0. SHA2() was recommended to use (https://dev.mysql.com/doc/refman/5.7/en/encryption-functions.html#function_sha2) instead.

@vladsf
Copy link
Contributor Author

vladsf commented Jan 29, 2023

ENCRYPT() call might be replaced with:
SHA2(CONCAT(HEX(RANDOM_BYTES(2)), 'password'), 256)
then the whole code will be 8.0 compatible otherwise it requires 5.[6,7]

@vladsf vladsf changed the title Add MySQL 8 support Add MySQL 5.6+ support Jan 29, 2023
@theory
Copy link
Collaborator

theory commented Jan 29, 2023

ENCRYPT() call might be replaced with:
SHA2(CONCAT(HEX(RANDOM_BYTES(2)), 'password'), 256)
then the whole code will be 8.0 compatible otherwise it requires 5.[6,7]

Sounds good, if you want to add that I'll get it merged. Thank you!

@vladsf
Copy link
Contributor Author

vladsf commented Jan 30, 2023

Examined the rest of the code. Removing ENCRYPT() breaks the consistency of the tutorial. Some filenames have encrypt in the names, etc. I suggest to stay compatible with MySQL 5.7. This version is available on the official MySQL Docker page.
Let this a patch be a step towards MySQL 8.

@vladsf vladsf changed the title Add MySQL 5.6+ support Add MySQL 5.7 support Jan 30, 2023
@vladsf
Copy link
Contributor Author

vladsf commented Feb 9, 2023

@theory any updates?

@theory
Copy link
Collaborator

theory commented Feb 9, 2023

Nope, been completely heads-down on a project at work, plus a mini-project to move the PGXN bot from Twitter to Mastodon. I'll get to it, but it might be a couple weeks.

theory
theory previously approved these changes Feb 18, 2023
Copy link
Collaborator

@theory theory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just one Pod bit that needs fixing.

lib/sqitchtutorial-mysql.pod Outdated Show resolved Hide resolved
@theory theory merged commit 0f0eee0 into sqitchers:develop Feb 20, 2023
theory added a commit that referenced this pull request Feb 20, 2023
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.

None yet

2 participants