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

support for TiDB (a MySQL-compatible database)? #1997

Closed
ian-p-cooke opened this issue Feb 22, 2019 · 6 comments
Closed

support for TiDB (a MySQL-compatible database)? #1997

ian-p-cooke opened this issue Feb 22, 2019 · 6 comments

Comments

@ian-p-cooke
Copy link

I've been working on getting Diesel/MySQL working with TiDB. Do you have any interest in folding in some of the work I've done? I'm doing development on Windows and hit a few snags along the way related to that and others due to TiDB not being 100% compatible.

libmysql 8.0.4-3

  • diesel_cli/mysqlclient-sys won't build
    ** master off of github works ok (they updated vcpkg so that it finds dependencies now)
  • diesel_cli returns Unknown charset id 255
    ** Support for utf8mb4_0900_ai_ci in syntax. pingcap/tidb#9426
    ** if I change the mysql options in the raw connection to use 'utf8' instead of 'utf8mb4' we can get past this
    ** I think that's an ok change to make for everyone because later on you set character_set_client and friends and that works fine. The end result should be the same whether you specify utf8 or utf8mb4 as the default.
  • diesel_cli returns Unknown MySQL error
    ** Support for caching_sha2_password pingcap/tidb#9411
    ** this was really tricky to track down but the solution is simple. The libmysql I was using wants to use a newer authentication scheme than TiDB supports. If I set another mysql option in the raw connection to change the default auth back to mysql_native_password then it works.
    ** I don't know if this is good or bad. 5.x users are all using native authentication but MySQL has some explanation of why they changed things: https://dev.mysql.com/doc/refman/8.0/en/upgrading-from-previous-series.html#upgrade-caching-sha2-password I think it would be fine to default to native password but some users might want to change that. I thought about maybe using the query string of the DATABASE_URL to pass along driver-specific settings... what do you think of that?
  • setting MYSQL_DEFAULT_AUTH wasn't working
    ** enums are incompatible with libmysql 8.0.4 on Windows 10 sgrif/mysqlclient-sys#15
    ** in order to set MYSQL_DEFAULT_AUTH I had to regenerate the bindings for mysqlclient-sys. The latest header changed some enum values, particularly my_bool went away and was replaced with a real bool. That took some minor changes to diesel/mysql to stay compatible. I imagine they'd bump their major version number then we could update diesel after that.
@sgrif
Copy link
Member

sgrif commented Feb 22, 2019

Do you have any interest in folding in some of the work I've done?

We're not looking to add any new backends into the main project, but we're certainly happy to do what we can to support you if you'd like to develop it as a third party backend. Or are you asking for changes in the mysql backend to support TiDB?

@ian-p-cooke
Copy link
Author

ian-p-cooke commented Feb 23, 2019

i’m asking for changes to the MySQL backend and diesel_cli so that it supports both native MySQL and TiDB. I’ve opened issues with TiDB for places where I’m finding incompatible behavior but I think we could tweak Diesel’s MySQL backend in the mean time without negatively affecting native MySQL users.

For example, after I got past the above issues I found that diesel setup was still failing, this time because TiDB doesn’t support DDL in prepared statements. I modified diesel_cli so that it didn’t use prepared statements for create and then finally I could diesel setup without errors. I’d rather not maintain a fork of diesel_cli and since my change should work equally well with MySQL I was hoping you’d accept a pull request for that.

I just wanted to check with you before putting in the work for any pull requests.

@sgrif
Copy link
Member

sgrif commented Feb 23, 2019

For issues like that, a separate backend is likely a better solution. The backend can share a lot of code with the default MySQL backend, but these things tend to drift apart more over time for different reasons, and trying to keep the same connection for both can lead to a lot of issues (we're planning to create a separate mariadb backend for the same reason)

@ian-p-cooke
Copy link
Author

Ok, for now I'll keep track of the changes I need and then when the mariadb backend progresses I can follow suit with a TiDB backend if my experiments turn out the way I want them to.

I found #1882 but it looks like it got stuck somewhere along the way. I'll inquire over there to see if that's still being worked on.

Thanks!

@walter211
Copy link

Subscribed this issue!

@weiznich
Copy link
Member

As pointed out by Siân we are not interested in adding support for TiDB to diesel itself. This could and should be handled by a third party crate maintained by someone that actually uses TiDB. If such a crate exists we are happy to accept a PR for integration within diesel-cli. If there is anything else missing diesel itself which blocks implementing such a third party backend feel free to open a new issue about this.

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

No branches or pull requests

4 participants