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

Make default character set consistent #233

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

olafz
Copy link

@olafz olafz commented Aug 30, 2018

In MariaEx, the default character set defaults to utf8. However, the initial client negotiation (the first client response after the server greeting) is currently hardcoded at character set 8 (which is latin1 COLLATE latin1_swedish_ci):

screen shot 2018-08-30 at 19 22 15

To make the behaviour of MariaEx consistent, this PR changes the initial client negotiation to character set 33 (which is utf8 COLLATE utf8_general_ci).

@olafz olafz changed the title Make default character set consequent Make default character set consistent Aug 30, 2018
@coveralls
Copy link

coveralls commented Aug 30, 2018

Coverage Status

Coverage decreased (-2.1%) to 82.895% when pulling caa1ec6 on olafz:master into 78cd8a7 on xerions:master.

@olafz
Copy link
Author

olafz commented Aug 30, 2018

The behaviour of this change has no effect on everything issued after the SET CHARACTER SET ..., which is usually the first thing that happens after the handshake:

defp handle_handshake(packet(msg: ok_resp(affected_rows: _affected_rows, last_insert_id: _last_insert_id) = _packet), nil, state) do
statement = "SET CHARACTER SET " <> (state.opts[:charset] || "utf8")
query = %Query{type: :text, statement: statement}

@foeken
Copy link

foeken commented Aug 31, 2018

Setting the actual proper default charset is quite a challenge since you need the internal charset nr (which is some rune from a generated IndexFile on the server side and is version specific):

Character set and collation information are specific to a server version and installation, and are generated automatically from the sql/share/charsets/Index.xml file in the source distribution.

https://github.com/mysql/mysql-server/blob/4f1d7cf5fcb11a3f84cff27e37100d7295e7d5ca/mysys/charset.cc#L629

Setting it to UTF-8 is not the true solution, perhaps this just needs to be 'known' by users of MariaEX: you need to specify an explicit charset else you will get the hard coded default not the database default. cc @michalmuskala @josevalim

Perhaps a warning/exception would be in order in those places.

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.

3 participants