Skip to content

Commit

Permalink
Make Encode return a result (launchbadge#3126)
Browse files Browse the repository at this point in the history
* Make encode and encode_by_ref fallible

This only changes the trait for now and makes it compile, calling .expect() on all users. Those will be removed in a later commit.

* PgNumeric: Turn TryFrom Decimal to an infallible From

* Turn panics in Encode implementations into errors

* Add Encode error analogous to the Decode error

* Propagate decode errors through Arguments::add

This pushes the panics one level further to mostly bind calls. Those will also be removed later.

* Only check argument encoding at the end

* Use Result in Query internally

* Implement query_with functions in terms of _with_result

* Surface encode errors when executing a query.

* Remove remaining panics in AnyConnectionBackend implementations

* PostgreSQL BigDecimal: Return encode error immediately

* Arguments: Add len method to report how many arguments were added

* Query::bind: Report which argument failed to encode

* IsNull: Add is_null method

* MySqlArguments: Replace manual bitmap code with NullBitMap helper type

* Roll back buffer in MySqlArguments if encoding fails

* Roll back buffer in SqliteArguments if encoding fails

* Roll back PgArgumentBuffer if encoding fails
  • Loading branch information
FSMaxB authored and James Holman committed Jun 6, 2024
1 parent 5e026ab commit 98b9948
Show file tree
Hide file tree
Showing 92 changed files with 792 additions and 456 deletions.
18 changes: 12 additions & 6 deletions sqlx-core/src/any/arguments.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use crate::any::value::AnyValueKind;
use crate::any::Any;
use crate::arguments::Arguments;
use crate::encode::Encode;
use crate::encode::{Encode, IsNull};
use crate::error::BoxDynError;
use crate::types::Type;

pub struct AnyArguments<'q> {
Expand All @@ -16,11 +17,16 @@ impl<'q> Arguments<'q> for AnyArguments<'q> {
self.values.0.reserve(additional);
}

fn add<T>(&mut self, value: T)
fn add<T>(&mut self, value: T) -> Result<(), BoxDynError>
where
T: 'q + Encode<'q, Self::Database> + Type<Self::Database>,
{
let _ = value.encode(&mut self.values);
let _: IsNull = value.encode(&mut self.values)?;
Ok(())
}

fn len(&self) -> usize {
self.values.0.len()
}
}

Expand All @@ -36,7 +42,7 @@ impl<'q> Default for AnyArguments<'q> {

impl<'q> AnyArguments<'q> {
#[doc(hidden)]
pub fn convert_to<'a, A: Arguments<'a>>(&'a self) -> A
pub fn convert_to<'a, A: Arguments<'a>>(&'a self) -> Result<A, BoxDynError>
where
'q: 'a,
Option<i32>: Type<A::Database> + Encode<'a, A::Database>,
Expand All @@ -62,9 +68,9 @@ impl<'q> AnyArguments<'q> {
AnyValueKind::Double(d) => out.add(d),
AnyValueKind::Text(t) => out.add(&**t),
AnyValueKind::Blob(b) => out.add(&**b),
}
}?
}

out
Ok(out)
}
}
12 changes: 10 additions & 2 deletions sqlx-core/src/any/connection/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ use crate::executor::{Execute, Executor};
use either::Either;
use futures_core::future::BoxFuture;
use futures_core::stream::BoxStream;
use futures_util::{stream, FutureExt, StreamExt};
use std::future;

impl<'c> Executor<'c> for &'c mut AnyConnection {
type Database = Any;
Expand All @@ -17,7 +19,10 @@ impl<'c> Executor<'c> for &'c mut AnyConnection {
'c: 'e,
E: Execute<'q, Any>,
{
let arguments = query.take_arguments();
let arguments = match query.take_arguments().map_err(Error::Encode) {
Ok(arguments) => arguments,
Err(error) => return stream::once(future::ready(Err(error))).boxed(),
};
self.backend.fetch_many(query.sql(), arguments)
}

Expand All @@ -29,7 +34,10 @@ impl<'c> Executor<'c> for &'c mut AnyConnection {
'c: 'e,
E: Execute<'q, Self::Database>,
{
let arguments = query.take_arguments();
let arguments = match query.take_arguments().map_err(Error::Encode) {
Ok(arguments) => arguments,
Err(error) => return future::ready(Err(error)).boxed(),
};
self.backend.fetch_optional(query.sql(), arguments)
}

Expand Down
7 changes: 5 additions & 2 deletions sqlx-core/src/any/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,15 @@ impl<'q, T> Encode<'q, Any> for Option<T>
where
T: Encode<'q, Any> + 'q,
{
fn encode_by_ref(&self, buf: &mut AnyArgumentBuffer<'q>) -> crate::encode::IsNull {
fn encode_by_ref(
&self,
buf: &mut AnyArgumentBuffer<'q>,
) -> Result<crate::encode::IsNull, crate::error::BoxDynError> {
if let Some(value) = self {
value.encode_by_ref(buf)
} else {
buf.0.push(AnyValueKind::Null);
crate::encode::IsNull::Yes
Ok(crate::encode::IsNull::Yes)
}
}
}
14 changes: 10 additions & 4 deletions sqlx-core/src/any/types/blob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,12 @@ impl Type<Any> for [u8] {
}

impl<'q> Encode<'q, Any> for &'q [u8] {
fn encode_by_ref(&self, buf: &mut <Any as Database>::ArgumentBuffer<'q>) -> IsNull {
fn encode_by_ref(
&self,
buf: &mut <Any as Database>::ArgumentBuffer<'q>,
) -> Result<IsNull, BoxDynError> {
buf.0.push(AnyValueKind::Blob((*self).into()));
IsNull::No
Ok(IsNull::No)
}
}

Expand All @@ -42,9 +45,12 @@ impl Type<Any> for Vec<u8> {
}

impl<'q> Encode<'q, Any> for Vec<u8> {
fn encode_by_ref(&self, buf: &mut <Any as Database>::ArgumentBuffer<'q>) -> IsNull {
fn encode_by_ref(
&self,
buf: &mut <Any as Database>::ArgumentBuffer<'q>,
) -> Result<IsNull, BoxDynError> {
buf.0.push(AnyValueKind::Blob(Cow::Owned(self.clone())));
IsNull::No
Ok(IsNull::No)
}
}

Expand Down
7 changes: 5 additions & 2 deletions sqlx-core/src/any/types/bool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,12 @@ impl Type<Any> for bool {
}

impl<'q> Encode<'q, Any> for bool {
fn encode_by_ref(&self, buf: &mut <Any as Database>::ArgumentBuffer<'q>) -> IsNull {
fn encode_by_ref(
&self,
buf: &mut <Any as Database>::ArgumentBuffer<'q>,
) -> Result<IsNull, BoxDynError> {
buf.0.push(AnyValueKind::Bool(*self));
IsNull::No
Ok(IsNull::No)
}
}

Expand Down
11 changes: 7 additions & 4 deletions sqlx-core/src/any/types/float.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ impl Type<Any> for f32 {
}

impl<'q> Encode<'q, Any> for f32 {
fn encode_by_ref(&self, buf: &mut AnyArgumentBuffer<'q>) -> IsNull {
fn encode_by_ref(&self, buf: &mut AnyArgumentBuffer<'q>) -> Result<IsNull, BoxDynError> {
buf.0.push(AnyValueKind::Real(*self));
IsNull::No
Ok(IsNull::No)
}
}

Expand All @@ -38,9 +38,12 @@ impl Type<Any> for f64 {
}

impl<'q> Encode<'q, Any> for f64 {
fn encode_by_ref(&self, buf: &mut <Any as Database>::ArgumentBuffer<'q>) -> IsNull {
fn encode_by_ref(
&self,
buf: &mut <Any as Database>::ArgumentBuffer<'q>,
) -> Result<IsNull, BoxDynError> {
buf.0.push(AnyValueKind::Double(*self));
IsNull::No
Ok(IsNull::No)
}
}

Expand Down
21 changes: 15 additions & 6 deletions sqlx-core/src/any/types/int.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,12 @@ impl Type<Any> for i16 {
}

impl<'q> Encode<'q, Any> for i16 {
fn encode_by_ref(&self, buf: &mut <Any as Database>::ArgumentBuffer<'q>) -> IsNull {
fn encode_by_ref(
&self,
buf: &mut <Any as Database>::ArgumentBuffer<'q>,
) -> Result<IsNull, BoxDynError> {
buf.0.push(AnyValueKind::SmallInt(*self));
IsNull::No
Ok(IsNull::No)
}
}

Expand All @@ -43,9 +46,12 @@ impl Type<Any> for i32 {
}

impl<'q> Encode<'q, Any> for i32 {
fn encode_by_ref(&self, buf: &mut <Any as Database>::ArgumentBuffer<'q>) -> IsNull {
fn encode_by_ref(
&self,
buf: &mut <Any as Database>::ArgumentBuffer<'q>,
) -> Result<IsNull, BoxDynError> {
buf.0.push(AnyValueKind::Integer(*self));
IsNull::No
Ok(IsNull::No)
}
}

Expand All @@ -68,9 +74,12 @@ impl Type<Any> for i64 {
}

impl<'q> Encode<'q, Any> for i64 {
fn encode_by_ref(&self, buf: &mut <Any as Database>::ArgumentBuffer<'q>) -> IsNull {
fn encode_by_ref(
&self,
buf: &mut <Any as Database>::ArgumentBuffer<'q>,
) -> Result<IsNull, BoxDynError> {
buf.0.push(AnyValueKind::BigInt(*self));
IsNull::No
Ok(IsNull::No)
}
}

Expand Down
16 changes: 11 additions & 5 deletions sqlx-core/src/any/types/str.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,18 @@ impl Type<Any> for str {
}

impl<'a> Encode<'a, Any> for &'a str {
fn encode(self, buf: &mut <Any as Database>::ArgumentBuffer<'a>) -> IsNull
fn encode(self, buf: &mut <Any as Database>::ArgumentBuffer<'a>) -> Result<IsNull, BoxDynError>
where
Self: Sized,
{
buf.0.push(AnyValueKind::Text(self.into()));
IsNull::No
Ok(IsNull::No)
}

fn encode_by_ref(&self, buf: &mut <Any as Database>::ArgumentBuffer<'a>) -> IsNull {
fn encode_by_ref(
&self,
buf: &mut <Any as Database>::ArgumentBuffer<'a>,
) -> Result<IsNull, BoxDynError> {
(*self).encode(buf)
}
}
Expand All @@ -50,9 +53,12 @@ impl Type<Any> for String {
}

impl<'q> Encode<'q, Any> for String {
fn encode_by_ref(&self, buf: &mut <Any as Database>::ArgumentBuffer<'q>) -> IsNull {
fn encode_by_ref(
&self,
buf: &mut <Any as Database>::ArgumentBuffer<'q>,
) -> Result<IsNull, BoxDynError> {
buf.0.push(AnyValueKind::Text(Cow::Owned(self.clone())));
IsNull::No
Ok(IsNull::No)
}
}

Expand Down
6 changes: 5 additions & 1 deletion sqlx-core/src/arguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use crate::database::Database;
use crate::encode::Encode;
use crate::error::BoxDynError;
use crate::types::Type;
use std::fmt::{self, Write};

Expand All @@ -14,10 +15,13 @@ pub trait Arguments<'q>: Send + Sized + Default {
fn reserve(&mut self, additional: usize, size: usize);

/// Add the value to the end of the arguments.
fn add<T>(&mut self, value: T)
fn add<T>(&mut self, value: T) -> Result<(), BoxDynError>
where
T: 'q + Encode<'q, Self::Database> + Type<Self::Database>;

/// The number of arguments that were already added.
fn len(&self) -> usize;

fn format_placeholder<W: Write>(&self, writer: &mut W) -> fmt::Result {
writer.write_str("?")
}
Expand Down
32 changes: 22 additions & 10 deletions sqlx-core/src/encode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
use std::mem;

use crate::database::Database;
use crate::error::BoxDynError;

/// The return type of [Encode::encode].
#[must_use]
pub enum IsNull {
/// The value is null; no data was written.
Yes,
Expand All @@ -15,11 +17,16 @@ pub enum IsNull {
No,
}

impl IsNull {
pub fn is_null(&self) -> bool {
matches!(self, IsNull::Yes)
}
}

/// Encode a single value to be sent to the database.
pub trait Encode<'q, DB: Database> {
/// Writes the value of `self` into `buf` in the expected format for the database.
#[must_use]
fn encode(self, buf: &mut <DB as Database>::ArgumentBuffer<'q>) -> IsNull
fn encode(self, buf: &mut <DB as Database>::ArgumentBuffer<'q>) -> Result<IsNull, BoxDynError>
where
Self: Sized,
{
Expand All @@ -30,8 +37,10 @@ pub trait Encode<'q, DB: Database> {
///
/// Where possible, make use of `encode` instead as it can take advantage of re-using
/// memory.
#[must_use]
fn encode_by_ref(&self, buf: &mut <DB as Database>::ArgumentBuffer<'q>) -> IsNull;
fn encode_by_ref(
&self,
buf: &mut <DB as Database>::ArgumentBuffer<'q>,
) -> Result<IsNull, BoxDynError>;

fn produces(&self) -> Option<DB::TypeInfo> {
// `produces` is inherently a hook to allow database drivers to produce value-dependent
Expand All @@ -50,12 +59,15 @@ where
T: Encode<'q, DB>,
{
#[inline]
fn encode(self, buf: &mut <DB as Database>::ArgumentBuffer<'q>) -> IsNull {
fn encode(self, buf: &mut <DB as Database>::ArgumentBuffer<'q>) -> Result<IsNull, BoxDynError> {
<T as Encode<DB>>::encode_by_ref(self, buf)
}

#[inline]
fn encode_by_ref(&self, buf: &mut <DB as Database>::ArgumentBuffer<'q>) -> IsNull {
fn encode_by_ref(
&self,
buf: &mut <DB as Database>::ArgumentBuffer<'q>,
) -> Result<IsNull, BoxDynError> {
<&T as Encode<DB>>::encode(self, buf)
}

Expand Down Expand Up @@ -90,23 +102,23 @@ macro_rules! impl_encode_for_option {
fn encode(
self,
buf: &mut <$DB as $crate::database::Database>::ArgumentBuffer<'q>,
) -> $crate::encode::IsNull {
) -> Result<$crate::encode::IsNull, $crate::error::BoxDynError> {
if let Some(v) = self {
v.encode(buf)
} else {
$crate::encode::IsNull::Yes
Ok($crate::encode::IsNull::Yes)
}
}

#[inline]
fn encode_by_ref(
&self,
buf: &mut <$DB as $crate::database::Database>::ArgumentBuffer<'q>,
) -> $crate::encode::IsNull {
) -> Result<$crate::encode::IsNull, $crate::error::BoxDynError> {
if let Some(v) = self {
v.encode_by_ref(buf)
} else {
$crate::encode::IsNull::Yes
Ok($crate::encode::IsNull::Yes)
}
}

Expand Down
4 changes: 4 additions & 0 deletions sqlx-core/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ pub enum Error {
source: BoxDynError,
},

/// Error occured while encoding a value.
#[error("error occured while encoding a value: {0}")]
Encode(#[source] BoxDynError),

/// Error occurred while decoding a value.
#[error("error occurred while decoding: {0}")]
Decode(#[source] BoxDynError),
Expand Down
Loading

0 comments on commit 98b9948

Please sign in to comment.