Skip to content

change the default utf8 encoding #115

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

Merged
merged 1 commit into from
Apr 7, 2019
Merged

change the default utf8 encoding #115

merged 1 commit into from
Apr 7, 2019

Conversation

oshai
Copy link
Contributor

@oshai oshai commented Apr 4, 2019

according to this post https://mathiasbynens.be/notes/mysql-utf8mb4
the default utf8 in mysql do not exactly match utf8 standard (3 vs 4 bytes)

Since it maps to the same charset in Java
It sounds like the reasonable thing to do is to change the used protocol
from 83 to 224 (utf8mb4_unicode_ci)
I added a flag to allow overriding it -DjasyncMysqlUTF8Collation=83
To put in back in the old legacy behaviour
for example if using mysql server < 5.5.3.

Added also tests for this.

This PR, is a fix for the issue raised here: vert-x3/vertx-mysql-postgresql-client#149

according to this post https://mathiasbynens.be/notes/mysql-utf8mb4
the default utf8 in mysql do not exactly match utf8 standard (3 vs 4 bytes)

Since it maps to the same charset in Java
It sounds like the reasonable thing to do is to change the used protocol
from 83 to 224 (utf8mb4_unicode_ci)
I added a flag to allow overriding it `-DjasyncMysqlUTF8Collation=83`
To put in back in the old legacy behaviour
for example if using mysql server < 5.5.3.

Added also tests for this.
@oshai
Copy link
Contributor Author

oshai commented Apr 4, 2019

@swordfeng - would like to get your opinion about this change. Thanks!

@codecov
Copy link

codecov bot commented Apr 4, 2019

Codecov Report

Merging #115 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #115   +/-   ##
=========================================
  Coverage     78.94%   78.94%           
  Complexity     1011     1011           
=========================================
  Files           258      258           
  Lines          3662     3662           
  Branches        487      487           
=========================================
  Hits           2891     2891           
  Misses          531      531           
  Partials        240      240
Impacted Files Coverage Δ Complexity Δ
...m/github/jasync/sql/db/mysql/util/CharsetMapper.kt 80% <100%> (ø) 4 <0> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9086067...fb3afd2. Read the comment docs.

@swordfeng
Copy link

@oshai Thank you! It looks good to me. Maybe we could add that flag to the document in case someone is using a version of MySQL released 9 years ago.

@oshai oshai merged commit dfb8867 into master Apr 7, 2019
@oshai oshai deleted the utf branch April 7, 2019 08:35
@oshai
Copy link
Contributor Author

oshai commented Apr 7, 2019

released in 0.9.50 and add a wiki here: https://github.com/jasync-sql/jasync-sql/wiki/Supported-Encoding

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