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

Update collations and make utf8mb4 default #877

Merged
merged 5 commits into from Apr 5, 2019

Conversation

methane
Copy link
Member

@methane methane commented Oct 22, 2018

Description

Update collations table based on MySQL 8.0.12.

  • Change default collation to "utf8mb4_general_ci" which is available from MySQL 5.5.
  • Comment out collations can not be used for connection.

Fixes #839

Checklist

  • Code compiles correctly
  • Created tests which fail without the change (if possible)
  • All tests passing
  • Extended the README / documentation, if necessary
  • Added myself / the copyright holder to the AUTHORS file

@methane methane changed the title Update collations Update collations and make utf8mb4 default Oct 29, 2018
@shopee-jin
Copy link

gofmt please?

@methane
Copy link
Member Author

methane commented Dec 18, 2018

Travis checks gofmt and no error is reported.

Copy link
Member

@julienschmidt julienschmidt left a comment

Choose a reason for hiding this comment

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

Should we maybe make this version dependent instead?

According to https://www.monolune.com/what-is-the-utf8mb4_0900_ai_ci-collation/ the default collation is utf8mb4_0900_ai_ci since MySQL 8.0.1.

We already know the server version at the point where we send the collation.
So we could use utf8_general_ci for server version < 5.5.3, utf8mb4_general_ci for < 8.0.1 and utf8mb4_0900_ai_ci for 8.0.1 and newer.

README.md Outdated Show resolved Hide resolved
@julienschmidt julienschmidt added this to the v1.5.0 milestone Mar 8, 2019
@methane
Copy link
Member Author

methane commented Mar 9, 2019

Should we maybe make this version dependent instead?

Personally speaking, I prefer stable behavior to "automatically changing default".
mysql client uses client's default, not server default.
User can specify collation explicitly.

According to https://www.monolune.com/what-is-the-utf8mb4_0900_ai_ci-collation/ the default collation is utf8mb4_0900_ai_ci since MySQL 8.0.1.

We already know the server version at the point where we send the collation.

I don't want to parse MySQL version string. MySQL version string syntax is not defined.
And MySQL is used by vary software, not only MySQL.
(e.g. MariaDB, PerconaDB, many MySQL proxy implementations)

So we could use utf8_general_ci for server version < 5.5.3, utf8mb4_general_ci for < 8.0.1 and utf8mb4_0900_ai_ci for 8.0.1 and newer.

Later is possible without parsing version string. Initial packet contains server's default collation id.
When it is 255 (utf8mb4_0900_ai_ci), it can be used by default.

But personally speaking, I don't like dynamic default. I prefer stable behavior.
User can choose other collation explicitly.

Copy link
Contributor

@shogo82148 shogo82148 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@methane methane dismissed julienschmidt’s stale review April 5, 2019 02:48

doesn't make sense.

@methane methane merged commit 93c3765 into go-sql-driver:master Apr 5, 2019
@methane methane deleted the collation branch April 5, 2019 02:49
@kaihendry
Copy link

If the collation default is not the same as my DB I'm connecting to, should I be worried? https://stackoverflow.com/questions/55998671/connecting-with-wrong-collation-to-a-mysql-server

@methane
Copy link
Member Author

methane commented May 6, 2019

@kaihendry Your question is not specific to this driver. It's general to any MySQL driver.
Please don't ask such question on issue tracker.

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.

Request to support MySQL 8.0 latest collations: utf8mb4_0900_ai_ic, etc....
5 participants