proposal: use TryFrom instead of From in query_as!

[vidhanio]

Is your feature request related to a problem? Please describe.
currently, the query_as! macro performs Into::into on returned items to convert them into the types of the struct fields. This solution only works, obviously, when the conversion is infallible. what if the conversion is fallible? sqlx requires one to implement Decode to fallibly convert from database types. The problem is that these have to be checked at run-time, as their type checking is done by the author in the impl Type, etc. However, what if your type can be converted (fallibly) from a built-in, already type-checked sqlx type (Uuid, String, etc.)?

Describe the solution you'd like
query_as! should use .try_into() in its generated code instead of into(). If it fails, it should take the TryInto::Error and box it (much like Decode::decode) and bubble it up as an sqlx::Error.

From what I can tell, this would not break any previous code, due to the pre-existing blanket impl of impl<T, U> TryFrom<U> for T where U: From<T>, and the Err would just be Infallible.

Here is an example of when this would be useful:

struct Username(String);

impl TryFrom<String> for Username {
	type Error = &'static str;

	fn try_from(s: String) -> Result<Self, Self::Error> {
		for c in c.chars() {
			if !('a'..='z').contains(c) {
				return "invalid character: (must be a-z)";
			}
		}

		return Self(s)
	}
}

struct User {
	username: Username,
	bio: String,
}

sqlx::query_as!(User, "SELECT username, bio FROM users"); // valid query, as `Username` implements `TryFrom` a built-in sqlx type.

Of course, this may seem like unneeded validation for a database retrieval, but one may want to avoid implementing From due to it exposing a hole in data validation, meaning they have to go through the hassle of implementing Decode then still not have compile-time type safety for their query.

Describe alternatives you've considered
Manually implementing Decode.

[bm-w]

Bump — I totally agree with this proposal!

[abonander]

We won't accept a blanket change to TryInto for the reasons outlined in #2649 (comment), but here's my alternative proposal:

We can extend #[derive(sqlx::Type)] to allow a custom Decode shim similar to #[serde(deserialize_with = "<path to function>)]":

#[derive(sqlx::Type)]
#[sqlx(transparent)] // Means encode to/decode from `String`
#[sqlx(decode_with = "decode_username")]
struct Username(String);

fn decode_username<DB: Database>(val: <DB as HasValueRef<'_>>::ValueRef) -> Result<Username, Box<dyn Error + Send + Sync + 'static>> {
    let username = String::decode(val)?;
    Ok(Username::try_from(username)?)
}

Or, since that function signature turns out to be rather gnarly, we could just add a special marker to use TryFrom, e.g.:

#[derive(sqlx::Type)]
#[sqlx(decode(try_from = "String"))]
struct Username(String);

[bm-w]

Agree with your reasons, and happy to see this alternative #[derive(sqlx::Type)] idea. Are you suggesting to implement both decode_with = … and decode(try_from = …)? I’d say both have their benefits: the former is most flexible, while the latter will be more ergonomic in most cases (but likely can‘t be used in some). I think decode(from_str) should be considered as well (i.e. using FromStr).

Are you open to @vidhanio or I taking a shot at implementing this? I’d be happy to!

[abonander]

I think decode(from_str) should be considered as well (i.e. using FromStr).

That is also one I'd like to see. It would be a great alternative solution to implementing direct support for every datatype from any third-party crate that someone wants to shove into or pull out of a database.

As for which one(s) to implement, that's dealer's choice really. They can be implemented independently or all at once.

[vidhanio]

sorry for the late reply, studying for a midterm 😅

just a question to confirm my intuition, this solution would not provide compile time type safety, correct? i assume this is a "type-erased" situation where at runtime the query would fail if the column was of the wrong type?

[abonander]

Type safety with custom types is limited right now either way, as the macros don't currently have a way of statically typechecking them. That's what #514 is meant to solve.

However, that is a breaking change whereas this is backwards compatible.

[gyzerok]

I've got sort of blocked by the same problem and wanted to bring my thoughts here.

First of all if I may comment on the proposed solutions the following looks appealing to me. It's almost the same as was proposed, but in my opinion decode(..) bit is unnecessary and without it API is in line with field attributes for FromRow which helps with the intuition of how to use the library.

#[derive(sqlx::Type)]
#[sqlx(try_from = "String")]
struct Username(String);

It looks like for the time being having an adjusted copy of the macro inside my own repository would be enough as a workaround. Unfortunately I am not as experienced in Rust so maybe @abonander you could provide a code sample here in the issue which could be copy-pasted directly? It would be highly appreciated.

Now thinking about the solution overall I see 2 different point of views here:

1. On one hand reusing TryFrom implementation is great, because it helps achieving additional safety with the newtype pattern. At least in my codebases I use "parse dont validate" idea extensively and find it really useful.
2. On the other hand it might make sense to treat parsing from the outside world (say request params) differently from obtaining value from the db. If we consider db as a source of truth then having "wrong" value there is not necessarily wrong by itself. Maybe we would like to adjust validation strictness on the API surface area, but have existing values as is. In that case I would still try to avoid having From<T> conversation, so user-space code can't violate the constants.

As an outcome from 2 it might make sense to design a different API, so granularity of parsing/decoding is possible.

[RobbieMcKinstry]

Hello! I wanted to ask about the stated work-around. I'm in the exact situation described by @vidhanio: I'm implementing an Email type...

#[derive(sqlx::Type)]
#[sqlx(transparent)]
pub struct Email(String);

My email is transparently a String, but I want to practice "Make illegal states unrepresentable" by only implementing TryFrom<String> for Email, and not implementing From<String> for Email.

The original issue indicates that I can implement Decode<'r, DB: Database> for Email as a workaround -- my Email type won't be compile-time typechecked, but I'll still be able to use query_file_as!. That's good enough for me for now; however, I can't seem to get this to work.

Here's my full code snippet:

#[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, ToSchema)]
pub struct Email(String);

impl<'r, DB: Database> Decode<'r, DB> for Email
where
    &'r str: Decode<'r, DB>,
{
    fn decode(
        value: <DB as sqlx::database::HasValueRef<'r>>::ValueRef,
    ) -> Result<Self, sqlx::error::BoxDynError> {
        let string = <&str as Decode<DB>>::decode(value)?.to_owned();
        let result = Email::try_from(string).map_err(|err| Box::new(err))?;
        Ok(result)
    }
}

This yields the same error as before I implemented Decode:

the trait bound `email::Email: From<std::string::String>` is not satisfied
...
the trait `From<std::string::String>` is not implemented for `email::Email`
   = note: required for `std::string::String` to implement `Into<email::Email>`

Am I missing something? Any help would be appreciated. ❤️

[jplatte]

@RobbieMcKinstry you'll have to query the column as "column_name: _" to disable type checking on it.

[RobbieMcKinstry]

Thank you very much! I'll give that a try. :D I probably should have realized that, since I acknowledged "my type won't be compile-time typechecked".

[RobbieMcKinstry]

You were correct, by the way. Thank you very much for your insight. :)

[LoiKos]

Hi,

I'm new to Rust and i run into this problem while moving from query_as to query_as!.

I have an custom enum to handle a type field in the db:

#[derive(Deserialize, Serialize)]
pub enum ConfigurationType {
   Global,
   SCOH
}

And i need to perform an insert using it like this:

pub struct Configuration {
    pub id: sqlx::types::Uuid, // this field is auto-generated in db
    pub enabled: bool,
    pub r#type: ConfigurationType,
}
    
impl Configuration {
    pub async fn generate(
        tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
    ) -> Result<Vec<Configuration>, sqlx::Error> {
        let res = sqlx::query_as!(Configuration,
                 r#"
            INSERT INTO table (type, enable)
            VALUES ($1, false), ($2, false)
            RETURNING *"#,
            ConfigurationType::Global as ConfigurationType,
            ConfigurationType::SCOH as ConfigurationType,
         ).fetch_all(&mut **tx).await?;
         
         Ok(res)
     }
}

I try to use Decode:

impl<'r, DB: Database> Decode<'r, DB> for ConfigurationType
where
    String: sqlx::Decode<'r, DB>,
{
    fn decode(
        value: <DB as HasValueRef<'r>>::ValueRef,
    ) -> Result<Enum, Box<dyn Error + 'static + Send + Sync>> {
        let value = <String as Decode<DB>>::decode(value)?;

        match value.as_str() {
            "global" => Ok(ConfigurationType::Global),
            "scoh" => Ok(ConfigurationType::SCOH),
            _ => Err("Invalid string as configuration type".into()),
        }
    }
}

but this is not working it still asking to implement the From trait

the trait bound `ConfigurationType: std::convert::From<std::string::String>` is not satisfied required for `std::string::String` to implement `Into<ConfigurationType>

So i don't find any other solution than to implement the From with !panic :

impl From<String> for ConfigurationType {
     fn from(value: String) -> Self {
         match value.as_str() {
             "global" => ConfigurationType::Global,
             "scoh" => ConfigurationType::SCOH,
             _ => panic!("String {} cannot be used as ConfigurationType!", value),
         }
     }
 }

I don't know if there is another way to solve this problem , but imho i understand why FromRow is missing but without any guidance macro are a bit hard to understand and use.

[Qqwy]

The best thing to do would of course be to support FromRow directly in query_as! but it is clear from #514 that there are a lot of technical hurdles to make that happen.

The idea to use TryFrom is nice, but I agree with @gyzerok that parsing should be granular, and data that is stored in the DB should not be treated the same as 'generic user data'. We can trust the DB to a higher extent as data coming from 'anywhere', under the assumption that it is very likely that the data was written only after passing checks in our apps before.
If we enable plain TryFrom everywhere, we would lose out on a lot of type-safety, because for example i8::try_from(u64) exists but will fail at runtime for any value larger than 63.

Instead, what about a specialized trait named e.g. TryFromType:

• It has a blanket implementation for all infallible From/Into conversions (to make sure this is backwards-compatible with the current implementation of query_as! that uses .into())
• It is public, but intentionally not implemented by default for any other type, which means that by default no type safety is lost (and the principle of least surprise remains intact). A user can decide explicitly in their library or app if it makes sense to add an implementation for their special column type.
• On conversion failure, it could re-use the existing ColumnDecode error.
• This is a non-breaking change.

The intention for this trait is to only be implemented for types for which parse failure is rare; i.e. parse failure means that something other than (bug-free code of) your app has been messing up the DB's contents.

For example:

• A custom ID type that uses the unsigned 63 bits but has to be stored in an i64 because there is no u64 in Sqlite or Postgres. TryFromType would only fail iff someone inserts a negative i64 outside of the purview of Sqlx into the DB anyway.
• In essence it is a generalization of the currently special builtin cases for uuid/json/timestamps in the drivers for DBs that don't support them natively. Decoding fails iff someone were to insert an unparsable value in the backing TEXT column.