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

add CLIENT_FOUND_ROWS flag to mysql connect #2887

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,8 @@ for Rust libraries in [RFC #1105](https://github.com/rust-lang/rfcs/blob/master/
* The sqlite backend now uses a single batch insert statement if there are now default values present
in the values clause

* The MySQL connection is using the CLIENT_FOUND_ROWS from now on. This means that updating rows without changing any values will return the number of matched rows (like most other SQL servers do), as opposed to the number of changed rows.

### Fixed

* Many types were incorrectly considered non-aggregate when they should not
Expand Down
19 changes: 19 additions & 0 deletions diesel/src/mysql/connection/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,4 +166,23 @@ mod tests {
assert!(connection.execute("SELECT 1").is_ok());
assert!(connection.execute("SELECT 1").is_ok());
}

#[test]
fn check_client_found_rows_flag() {
let conn = &mut crate::test_helpers::connection();
conn.execute("DROP TABLE IF EXISTS update_test CASCADE")
.unwrap();

conn.execute("CREATE TABLE update_test(id INTEGER PRIMARY KEY, num INTEGER NOT NULL)")
.unwrap();

conn.execute("INSERT INTO update_test(id, num) VALUES (1, 5)")
.unwrap();

let output = conn
.execute("UPDATE update_test SET num = 5 WHERE id = 1")
.unwrap();

assert_eq!(output, 1);
}
}
3 changes: 2 additions & 1 deletion diesel/src/mysql/connection/raw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ impl RawConnection {
let database = connection_options.database();
let port = connection_options.port();
let unix_socket = connection_options.unix_socket();
let client_flags = connection_options.client_flags();

unsafe {
// Make sure you don't use the fake one!
Expand All @@ -63,7 +64,7 @@ impl RawConnection {
unix_socket
.map(CStr::as_ptr)
.unwrap_or_else(|| ptr::null_mut()),
0,
client_flags.bits().into(),
)
};

Expand Down
39 changes: 39 additions & 0 deletions diesel/src/mysql/connection/url.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,44 @@ use std::ffi::{CStr, CString};

use crate::result::{ConnectionError, ConnectionResult};

bitflags::bitflags! {
pub struct CapabilityFlags: u32 {
const CLIENT_LONG_PASSWORD = 0x00000001;
const CLIENT_FOUND_ROWS = 0x00000002;
const CLIENT_LONG_FLAG = 0x00000004;
const CLIENT_CONNECT_WITH_DB = 0x00000008;
const CLIENT_NO_SCHEMA = 0x00000010;
const CLIENT_COMPRESS = 0x00000020;
const CLIENT_ODBC = 0x00000040;
const CLIENT_LOCAL_FILES = 0x00000080;
const CLIENT_IGNORE_SPACE = 0x00000100;
const CLIENT_PROTOCOL_41 = 0x00000200;
const CLIENT_INTERACTIVE = 0x00000400;
const CLIENT_SSL = 0x00000800;
const CLIENT_IGNORE_SIGPIPE = 0x00001000;
const CLIENT_TRANSACTIONS = 0x00002000;
const CLIENT_RESERVED = 0x00004000;
const CLIENT_SECURE_CONNECTION = 0x00008000;
const CLIENT_MULTI_STATEMENTS = 0x00010000;
const CLIENT_MULTI_RESULTS = 0x00020000;
const CLIENT_PS_MULTI_RESULTS = 0x00040000;
const CLIENT_PLUGIN_AUTH = 0x00080000;
const CLIENT_CONNECT_ATTRS = 0x00100000;
const CLIENT_PLUGIN_AUTH_LENENC_CLIENT_DATA = 0x00200000;
const CLIENT_CAN_HANDLE_EXPIRED_PASSWORDS = 0x00400000;
const CLIENT_SESSION_TRACK = 0x00800000;
const CLIENT_DEPRECATE_EOF = 0x01000000;
}
}

pub struct ConnectionOptions {
host: Option<CString>,
user: CString,
password: Option<CString>,
database: Option<CString>,
port: Option<u16>,
unix_socket: Option<CString>,
client_flags: CapabilityFlags,
}

impl ConnectionOptions {
Expand Down Expand Up @@ -59,13 +90,17 @@ impl ConnectionOptions {
Some(segment) => Some(CString::new(segment.as_bytes())?),
};

// this is not present in the database_url, using a default value
Copy link
Member

@Ten0 Ten0 Sep 3, 2021

Choose a reason for hiding this comment

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

Maybe some people have a use for the old behavior though.
Perhaps ultimately we want to make the connection open-able from ConnectionOptions and implement FromStr on it, as well as some things to update the flags that don't break the typing, such as this one?
May be slightly drifting off-topic but I feel like the requirement of having establish on Connection is a bit too much: it's not super flexible as it requires that the url is the only input, and it has to be something that always errors on r2d2 connections... It feels like it's something each connection should implement separately, if relevant and in the relevant manner.

Not a blocker for me though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that was my impression as well, establish() is too restrictive, there's no way to manipulate ConnectionOptions outside of parse() currently. This is understandable of course, because ConnectionOptions is MySQL specific currently, so there could not be any ways to manipulate it through the Connection trait which is not database engine specific.
I'm not sure if there's a use for something like ConnectionOptions for other database engines, but if so, maybe there could be a non-db-specific ConnectionOptions trait in the future?

let client_flags = CapabilityFlags::CLIENT_FOUND_ROWS;

Ok(ConnectionOptions {
host: host,
user: user,
password: password,
database: database,
port: url.port(),
unix_socket: unix_socket,
client_flags: client_flags,
})
}

Expand All @@ -92,6 +127,10 @@ impl ConnectionOptions {
pub fn unix_socket(&self) -> Option<&CStr> {
self.unix_socket.as_deref()
}

pub fn client_flags(&self) -> CapabilityFlags {
self.client_flags
}
}

fn decode_into_cstring(s: &str) -> ConnectionResult<CString> {
Expand Down