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

deps: upgrade to sqlx v0.8 #408

Merged
merged 5 commits into from
Dec 29, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ jobs:
fail-fast: false
matrix:
include:
- toolchain: ${{ env.RUST_VERSION }}
- toolchain: ${{ env.RUST_NIGHTLY_VERSION }}
- toolchain: "1.83.0"
- toolchain: "nightly-2024-12-20"

name: cargo test
runs-on: ubuntu-latest
Expand Down Expand Up @@ -64,8 +64,8 @@ jobs:

- name: Install toolchain
run: |
rustup toolchain install ${{ env.RUST_NIGHTLYVERSION }} --no-self-update --profile minimal --component miri
rustup override set ${{ env.RUST_NIGHTLYVERSION }}
rustup toolchain install ${{ env.RUST_NIGHTLY_VERSION }} --no-self-update --profile minimal --component miri
rustup override set ${{ env.RUST_NIGHTLY_VERSION }}
- name: Install cargo-nextest
uses: taiki-e/install-action@nextest
- uses: Swatinem/rust-cache@v2
Expand All @@ -85,8 +85,8 @@ jobs:

- name: Install toolchain
run: |
rustup toolchain install ${{ env.RUST_NIGHTLYVERSION }} --no-self-update --profile minimal
rustup override set ${{ env.RUST_NIGHTLYVERSION }}
rustup toolchain install ${{ env.RUST_NIGHTLY_VERSION }} --no-self-update --profile minimal
rustup override set ${{ env.RUST_NIGHTLY_VERSION }}
- name: Install cargo-nextest
uses: taiki-e/install-action@nextest
- uses: Swatinem/rust-cache@v2
Expand Down
2 changes: 1 addition & 1 deletion compact_str/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ quickcheck = { version = "1", optional = true, default-features = false }
rkyv = { version = "0.7", optional = true, default-features = false, features = ["size_32"] }
serde = { version = "1", optional = true, default-features = false, features = ["derive", "alloc"] }
smallvec = { version = "1", optional = true, features = ["union"] }
sqlx = { version = "0.7", optional = true, default-features = false }
sqlx = { version = "0.8", optional = true, default-features = false }

castaway = { version = "0.2.3", default-features = false, features = ["alloc"] }
cfg-if = "1"
Expand Down
28 changes: 16 additions & 12 deletions compact_str/src/features/sqlx.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
use sqlx::database::HasValueRef;
use sqlx::error::BoxDynError;
#[cfg(any(
feature = "sqlx-mysql",
feature = "sqlx-postgres",
feature = "sqlx-sqlite"
))]
use sqlx::{database::HasArguments, encode::IsNull, Encode};
use sqlx::{Database, Decode, Type, Value, ValueRef};
use sqlx::{encode::IsNull, Encode};
use sqlx::{Database, Decode, Type};

use crate::{CompactString, ToCompactString};

Expand All @@ -28,17 +27,19 @@ where
DB: Database,
for<'x> &'x str: Decode<'x, DB> + Type<DB>,
{
fn decode(value: <DB as HasValueRef<'r>>::ValueRef) -> Result<Self, BoxDynError> {
let value = value.to_owned();
let value: &str = value.try_decode()?;
fn decode(value: <DB as Database>::ValueRef<'r>) -> Result<Self, BoxDynError> {
let value = <&str as Decode<DB>>::decode(value)?;
Ok(value.try_to_compact_string()?)
}
}

#[cfg(feature = "sqlx-mysql")]
#[cfg_attr(docsrs, doc(cfg(feature = "sqlx-mysql")))]
impl<'q> Encode<'q, sqlx::MySql> for CompactString {
fn encode_by_ref(&self, buf: &mut <sqlx::MySql as HasArguments<'q>>::ArgumentBuffer) -> IsNull {
fn encode_by_ref(
&self,
buf: &mut <sqlx::MySql as Database>::ArgumentBuffer<'q>,
) -> Result<IsNull, BoxDynError> {
Encode::<'_, sqlx::MySql>::encode_by_ref(&self.as_str(), buf)
}

Expand All @@ -58,8 +59,8 @@ impl<'q> Encode<'q, sqlx::MySql> for CompactString {
impl<'q> Encode<'q, sqlx::Postgres> for CompactString {
fn encode_by_ref(
&self,
buf: &mut <sqlx::Postgres as HasArguments<'q>>::ArgumentBuffer,
) -> IsNull {
buf: &mut <sqlx::Postgres as Database>::ArgumentBuffer<'q>,
) -> Result<IsNull, BoxDynError> {
Encode::<'_, sqlx::Postgres>::encode_by_ref(&self.as_str(), buf)
}

Expand All @@ -85,14 +86,17 @@ impl sqlx::postgres::PgHasArrayType for CompactString {
#[cfg(feature = "sqlx-sqlite")]
#[cfg_attr(docsrs, doc(cfg(feature = "sqlx-sqlite")))]
impl<'q> Encode<'q, sqlx::Sqlite> for CompactString {
fn encode(self, buf: &mut <sqlx::Sqlite as HasArguments<'q>>::ArgumentBuffer) -> IsNull {
fn encode(
self,
buf: &mut <sqlx::Sqlite as Database>::ArgumentBuffer<'q>,
) -> Result<IsNull, BoxDynError> {
Encode::<'_, sqlx::Sqlite>::encode(self.into_string(), buf)
}

Choose a reason for hiding this comment

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

Suggested change
Encode::<'_, sqlx::Sqlite>::encode(self.into_string(), buf)
Encode::<'_, sqlx::Sqlite>::encode(self.as_str(), buf)

Needless allocation?

Copy link
Owner Author

Choose a reason for hiding this comment

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

I tried this change but it doesn't work. AFAICT what's happening is buf here is a SqliteArgumentValue which contains a Cow<'q, str>. Because we have self as an owned value, trying to use self.as_str() doesn't work because self gets dropped at the end of this scope, but needs to live for 'q.

Choose a reason for hiding this comment

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

Yes, the 'q lifetime complicates life when Postgres or MySQL backend is used.


fn encode_by_ref(
&self,
buf: &mut <sqlx::Sqlite as HasArguments<'q>>::ArgumentBuffer,
) -> IsNull {
buf: &mut <sqlx::Sqlite as Database>::ArgumentBuffer<'q>,
) -> Result<IsNull, BoxDynError> {
Encode::<'_, sqlx::Sqlite>::encode(alloc::string::String::from(self.as_str()), buf)
}

Choose a reason for hiding this comment

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

Suggested change
Encode::<'_, sqlx::Sqlite>::encode(alloc::string::String::from(self.as_str()), buf)
Encode::<'_, sqlx::Sqlite>::encode(self.as_str(), buf)

Copy link
Owner Author

Choose a reason for hiding this comment

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

Similar to the above, here we have a SqliteArgumentValue. Trying to use self.as_str() doesn't work because then the lifetime of &self would need to be longer than 'q, but there's no way to enforce that because then our impl would contain more lifetimes than the definition of the Encode trait


Expand Down
2 changes: 1 addition & 1 deletion examples/sqlx/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ edition = "2021"

[dependencies]
compact_str = { version = "0.8.0-beta", path = "../../compact_str", features = ["sqlx-mysql", "sqlx-postgres", "sqlx-sqlite"] }
sqlx = { version = "0.7", features = ["runtime-tokio", "mysql", "postgres", "sqlite"] }
sqlx = { version = "0.8", features = ["runtime-tokio", "mysql", "postgres", "sqlite"] }
tempfile = "3"
tokio = { version = "1.20.0", features = ["rt", "macros"]}
10 changes: 5 additions & 5 deletions examples/sqlx/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ macro_rules! test_body {
// insert a new todo
let mut transaction = conn.begin().await?;
let mut args = <$DbArguments>::default();
args.add(TITLE);
args.add(false);
args.add(TITLE).unwrap();
args.add(false).unwrap();
let q = query_with("INSERT INTO todos (title, done) VALUES ($1, $2)", args);
transaction.execute(q).await?;
transaction.commit().await?;
Expand All @@ -68,8 +68,8 @@ macro_rules! test_body {
let mut transaction = conn.begin().await?;
println!("Hello!");
let mut args = <$DbArguments>::default();
args.add(true);
args.add(id);
args.add(true).unwrap();
args.add(id).unwrap();
let q = query_with("UPDATE todos SET done = $1 WHERE id = $2", args);
transaction.execute(q).await?;
transaction.commit().await?;
Expand All @@ -94,7 +94,7 @@ macro_rules! test_body {
// we are done, delete our todo
let mut transaction = conn.begin().await?;
let mut args = <$DbArguments>::default();
args.add(TITLE);
args.add(TITLE).unwrap();
let q = query_with("DELETE FROM todos WHERE title = $1", args);
transaction.execute(q).await?;
transaction.commit().await?;
Expand Down
Loading