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 3 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
8 changes: 6 additions & 2 deletions diesel/src/mysql/connection/raw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::ptr::{self, NonNull};
use std::sync::Once;

use super::stmt::Statement;
use super::url::ConnectionOptions;
use super::url::{CapabilityFlags, ConnectionOptions};
use crate::result::{ConnectionError, ConnectionResult, QueryResult};

pub struct RawConnection(NonNull<ffi::MYSQL>);
Expand Down 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,10 @@ impl RawConnection {
unix_socket
.map(CStr::as_ptr)
.unwrap_or_else(|| ptr::null_mut()),
0,
client_flags
.unwrap_or_else(CapabilityFlags::empty)
.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: Option<CapabilityFlags>,
blackghost1987 marked this conversation as resolved.
Show resolved Hide resolved
}

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 = Some(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) -> Option<CapabilityFlags> {
self.client_flags
}
}

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