/diesel

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

r2d2 mysql 8.0 can not work. #1826

Closed
damody opened this Issue Aug 23, 2018 · 2 comments

Comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@damody @onesk @sgrif
@damody

damody commented Aug 23, 2018 •
edited

Setup

Versions

  • Rust: 1.28
  • Diesel: 1.3
  • Database: mysql 8.0
  • Operating System ubuntu 18.04

Feature Flags

  • diesel:

Problem Description

always fail in
https://github.com/damody/r2d2mysql/blob/81e94cd757eea1c15f0342f7bd75593b32b5a0b9/src/main.rs#L71

What are you trying to accomplish?

just connect

What is the expected output?

need to connection success

What is the actual output?

time out and get
Commands out of sync; you can\'t run this command now

Are you seeing any additional errors?

Steps to reproduce

  1. install mysql 8.0 in ubuntu 18.04
  2. create eat user and eat db
CREATE DATABASE eat;
CREATE USER 'eat'@'localhost' IDENTIFIED BY 'eateat';
GRANT ALL PRIVILEGES ON eat.* TO 'eat'@'localhost';
  1. cargo run

Checklist

  • I have already looked over the issue tracker for similar issues.
  • This issue can be reproduced on Rust's stable channel. (Your issue will be
    closed if this is not the case)
@onesk

This comment has been minimized.

Sign in to view

onesk commented Sep 12, 2018

Can confirm this regression as of 1.3.2. Seems like a similar problem to #728 and is at least partially mitigated by commenting the health check in ConnectionManager.
@sgrif

This comment has been minimized.

Sign in to view
Member

sgrif commented Sep 12, 2018 •
edited

I've found the problem and will have a fix shortly. We will release 1.3.3 with this fix once it has been merged

sgrif added a commit that referenced this issue Sep 12, 2018

@sgrif
Fix compatibility with MySQL 8.0 
MySQL's client library puts the responsibility on us to clear the
network buffers before continuing. This is problematic in cases where we
execute a query that returns results (and thus has additional frames
that need to be flushed), but in a context where we're ignoring them.

The old implementation used `mysql_next_result`, which advances to the
next result in the query, and returns whether we just advanced to the
final result or not. Because it returns false on the last result, and
not if there are no more results to advance to, we had to call
`mysql_store_result` a final time after the last call to
`mysql_next_result`.

The problem case is when a query only has a single result. In MySQL 6
and earlier, the second call to `mysql_store_result` would return a null
pointer. In MySQL 8, it will return an error instead. To fix this, I've
separated out the "more results", which will give us the behavior we
wish `mysql_next_result` had.

I'm honestly not sure why migrations weren't affected by this, as I'd
expect to get the same behavior in a migration that contained only a
single statement with no results, but apparently this only occurs if
there is a statement that does have results.

I have not updated the version of MySQL on Travis, because I want a CI
run to make sure I didn't break things on 6.x. I will update it to point
at 8.0 before merging.

This change does not require additional tests, as we already have
regression tests in place for this case.

This fix justifies a patch release backported to 1.3.

Fixes #1826.
6dfbafe

sgrif added a commit that referenced this issue Sep 12, 2018

@sgrif
Fix compatibility with MySQL 8.0 
MySQL's client library puts the responsibility on us to clear the
network buffers before continuing. This is problematic in cases where we
execute a query that returns results (and thus has additional frames
that need to be flushed), but in a context where we're ignoring them.

The old implementation used `mysql_next_result`, which advances to the
next result in the query, and returns whether we just advanced to the
final result or not. Because it returns false on the last result, and
not if there are no more results to advance to, we had to call
`mysql_store_result` a final time after the last call to
`mysql_next_result`.

The problem case is when a query only has a single result. In MySQL 6
and earlier, the second call to `mysql_store_result` would return a null
pointer. In MySQL 8, it will return an error instead. To fix this, I've
separated out the "more results", which will give us the behavior we
wish `mysql_next_result` had.

I'm honestly not sure why migrations weren't affected by this, as I'd
expect to get the same behavior in a migration that contained only a
single statement with no results, but apparently this only occurs if
there is a statement that does have results.

I have not updated the version of MySQL on Travis, because I want a CI
run to make sure I didn't break things on 6.x. I will update it to point
at 8.0 before merging.

This change does not require additional tests, as we already have
regression tests in place for this case.

This fix justifies a patch release backported to 1.3.

Fixes #1826.
2f4d204

@sgrif sgrif referenced this issue Sep 12, 2018

Merged

Fix compatibility with MySQL 8.0 #1847

@sgrif sgrif closed this in #1847 Sep 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment