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

MySQL UDS Support #450

Merged
merged 5 commits into from
Jun 27, 2020
Merged

MySQL UDS Support #450

merged 5 commits into from
Jun 27, 2020

Conversation

pimeys
Copy link
Contributor

@pimeys pimeys commented Jun 25, 2020

  • Adds support for Unix Domain Sockets on MySQL
  • Allows setting the socket path in PostgreSQL connection options
  • ... and MySQL connection options

Fixes issue #449

Copy link

@yoshuawuyts yoshuawuyts left a comment

Choose a reason for hiding this comment

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

Did a light code review -- I don't know enough about the workings of SQLx to comment on the correctness, but hopefully this is still useful.

sqlx-core/src/postgres/options.rs Outdated Show resolved Hide resolved
Comment on lines 38 to 49
#[cfg(unix)]
pub async fn connect_uds(path: impl AsRef<Path>) -> io::Result<Self> {
sqlx_rt::UnixStream::connect(path.as_ref())
.await
.map(Socket::Unix)
}

#[cfg(not(unix))]
pub async fn connect_uds(_: impl AsRef<Path>) -> io::Result<Self> {
Err(io::Error(
io::ErrorKind::Other,
"Unix domain sockets are not supported outside Unix platforms.",
))
}

Choose a reason for hiding this comment

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

Neat -- this is how std works these days as well. Also opens up the option to later enable it on Windows.

sqlx-core/src/net/socket.rs Outdated Show resolved Hide resolved
sqlx-core/src/mysql/options.rs Outdated Show resolved Hide resolved
sqlx-core/src/mysql/options.rs Outdated Show resolved Hide resolved
- Adds support for Unix Domain Sockets on MySQL
- Allows setting the socket path in PostgreSQL connection options
- ... and MySQL connection options
sqlx-core/src/postgres/options.rs Outdated Show resolved Hide resolved
sqlx-core/src/mysql/options.rs Outdated Show resolved Hide resolved
- If starts with a leading `/`, use socket
- If not, use host
@pimeys pimeys requested a review from mehcode June 25, 2020 14:32
@pimeys
Copy link
Contributor Author

pimeys commented Jun 25, 2020

Tested mysql with a test app, running mariadb locally and there's a socket file:

srwxrwxrwx 1 mysql mysql 0 Jun 25 16:05 /run/mysqld/mysqld.sock=

A test app with a simple select:

use sqlx::{Connect, MySqlConnection};

#[async_std::main]
async fn main() -> anyhow::Result<()> {
    let mut conn = MySqlConnection::connect("mysql://root@localhost/mysql?socket=/run/mysqld/mysqld.sock").await?;

    let row = sqlx::query("SELECT 1").fetch_one(&mut conn).await?;
    dbg!(row);

    Ok(())
}

Seems to connect and output the row.

@pimeys
Copy link
Contributor Author

pimeys commented Jun 25, 2020

There's a slight problem with PostgreSQL here, having a socket in /var/run/

srwxrwxrwx  1 postgres postgres    0 Jun 25 16:55 .s.PGSQL.5433

Now my test app is this:

use sqlx::{Connect, PgConnection, Row};

#[async_std::main]
async fn main() -> anyhow::Result<()> {
    let mut conn =
        PgConnection::connect("postgresql://postgres@localhost:5433/postgres?host=/var/run/postgresql").await?;

    let row = sqlx::query("SELECT 1 as val").fetch_one(&mut conn).await?;
    let val: i32 = row.get("val");

    dbg!(val);

    Ok(())
}

If I don't specity the hostname, I'm getting an error:

Error: error occurred while parsing a connection string: empty host

Caused by:
    empty host

Seems like the url parsing doesn't like when the hostname is missing. If I add localhost it connects using the socket, working correctly.

@mehcode mehcode merged commit 2115d02 into launchbadge:master Jun 27, 2020
@mehcode
Copy link
Member

mehcode commented Jun 27, 2020

Looks good to me. Thank you for cleaning up and generalizing the UDS code @pimeys ❤️


#[cfg(not(unix))]
pub async fn connect_uds(_: impl AsRef<Path>) -> io::Result<Self> {
Err(io::Error(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Woops, our CI didn't catch this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Pushed a fix to master.

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.

4 participants