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 connect attributes #164

Merged
merged 3 commits into from Jan 15, 2019
Merged

Support connect attributes #164

merged 3 commits into from Jan 15, 2019

Conversation

kubo
Copy link
Contributor

@kubo kubo commented Jan 4, 2019

This PR add connect attributes support as Perl DBD::MySQL.
I confirmed that this passes a test by cargo test connect_attrs for MySQL 5.7.

build.rs was added to get target_os and target_arch cfg features as string values.

The followings is the doc comment of Opts.connect_attrs


Connect attributes

This value is sent to the server as custom name-value attributes.

You can see them from performance_schema tables: session_account_connect_attrs
and session_connect_attrs
when the server is MySQL 5.6 or later,
or MariaDB 10.0 or later.

Note

Attribute names that begin with an underscore (_) are not set by
application programs because they are reserved for internal use.

The following attributes are sent in addition to ones set by programs.

name value
_client_name The client library name (rust-mysql-simple)
_client_version The client library version
_os The operation system (target_os cfg feature)
_pid The client proces ID
_platform The machine platform (target_arch cfg feature)
program_name The first element of std::env::args if program_name isn't set by programs.

@blackbeam
Copy link
Owner

Hi!
Thanks for your contribution. I'll review it.

@blackbeam blackbeam self-requested a review January 4, 2019 10:25
The first element of std::env::args_os() is usually the path
of the executable. However it may not exist.
@@ -242,6 +243,25 @@ impl<'a> Drop for Transaction<'a> {
}
}

// length of length encoded integer
fn lenenc_int_len(x: usize) -> usize {
Copy link
Owner

Choose a reason for hiding this comment

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

Please add the following todo comment. I'll move this to mysql_common crate later.

Suggested change
fn lenenc_int_len(x: usize) -> usize {
// TODO: Move to mysql_common
fn lenenc_int_len(x: usize) -> usize {

}

// length of length encoded string
fn lenenc_str_len(s: &str) -> usize {
Copy link
Owner

Choose a reason for hiding this comment

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

Please add the following todo comment. I'll move this to mysql_common crate later.

Suggested change
fn lenenc_str_len(s: &str) -> usize {
// TODO: Move to mysql_common
fn lenenc_str_len(s: &str) -> usize {

let value = env::var(name).expect(&format!("Could not get the environment variable {}", name));
println!("cargo:rustc-env={}={}", name, value);
}
}
Copy link
Owner

Choose a reason for hiding this comment

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

Could you please apply the diff as suggested by rustfmt here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did it. See the second commit.

@blackbeam blackbeam merged commit f9dd8d7 into blackbeam:master Jan 15, 2019
@kubo
Copy link
Contributor Author

kubo commented Jan 15, 2019

Thanks for accepting it.

I found a issue about tests. When the system variable performance_schema is off, connect attributes are not set to performance_schema.session_account_connect_attrs and this assertion fails. I'll make a new pull request which fixes it with todo comments you requested in this week.

@blackbeam
Copy link
Owner

Ok, thanks. I'll wait for it.

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.

None yet

2 participants