blackbeam/mysql_async

Idiomatic row retrieval for custom types

AshfordN opened this issue · 6 comments

When retrieving a row from a database to construct a custom type, should I convert the row to a tuple — making use of the FromRow implementation on tuples — then construct the type with the resulting values, or should I implement FromRow for the custom type directly?
In the first case my code would look something like this:

struct User {
    name: String,
    age: u8
}

async fn get_user(id: u32, conn: &Conn) -> Result<User, Error> {
    let stmt = r"SELECT name, age from users WHERE id = :id".with(params!{"id" => id});
    let (name, age) = stmt.first(conn).await?.unwrap();
    let user = User{name, age};
    Ok(user)
}

In the second case, even though the FromRow trait is exposed, the supporting API seems to be lacking and forces you to either: clone the row multiple times to return the error or mess around with reference counting to avoid cloning.
Here is the simplest way I can think of, to implement FromRow for the custom user type:

impl FromRow for User {
    fn from_row_opt(row: Row) -> Result<Self, FromRowError> {
        let name = row.get(0).ok_or(FromRowError(row.clone()))?;
        let age = row.get(1).ok_or(FromRowError(row.clone()))?;

        // NOTE: the calls to row.get() and row.clone() are expensive 
        // since they are doing deep copies of potentially large data

        let user = Self {name, age};
        Ok(user)
    }
}

and here is a slightly less expensive, but unnecessarily verbose, way of doing this, using reference counting:

fn get_value<T: FromValue>(row: Rc<Row>, idx: usize) -> Result<T, FromRowError> {
    row.get(idx)
        .ok_or_else(|| FromRowError(Rc::try_unwrap(row.clone()).expect("row has multiple owners")))

    // NOTE: we still have an expensive call to row.get() that cannot be replaced with row.take()
    // since row is not mutable
}

impl FromRow for User {
    fn from_row_opt(row: Row) -> Result<Self, FromRowError> {
        let row = Rc::new(row);
        let name = get_value(row.clone(), 0)?;
        let age = get_value(row, 1)?;

        let user = Self { name, age };
        Ok(user)
    }
}

and even so, reference counting isn't really all that ideal. I'm not even confident that the above code doesn't have subtle bugs.

Now, with that being said, I'm still somewhat new to rust and this particular mysql driver, so any clarity and assistance would greatly appreciated.

Hi.

mysql_common uses Row::take and hidden Row::place to avoid clones.

Btw, I'm working on proc macros for FromRow and FromValue, hope I'll be able to ship them soon. For now I suggest you to try the macro mentioned here.

I've looked at the macro you linked to, and I think it's a solid way of doing it, but, I also think that in practice, macros would be inherently too inflexible for something like this. There may be cases where the fields of a data structure do not exactly follow the column names or even the data that is returned from the database. For example, consider something like the following:

struct User {
  name: String,
  height: f64
}

#[inline(always)]
fn feet_to_cm(feet: f64) -> f64 {
  feet * 30.48
}

async fn get_user(id: u32, conn: &Conn) -> Result<User, Error> {
  let stmt = r"SELECT name, height from users WHERE id = :id".with(params!{"id" => id});
  let (name, height) = stmt.first(conn).await?.unwrap();
  let hieght = feet_to_cm(height); // this conversion is context specific
  let user = User{name, height};
  Ok(user)
}

There would be no way for you to incorporate the above logic when implementing FromRow from a macro. Now I know that this is a contrived example and some developers may be quick to point out that I should implement feet_to_cm() as User::height_in_cm() instead, but there may be good reasons to do it this way. If the database stores imperial units, for whatever reason (perhaps because it's a legacy database), but the rest of the software uses metric, then there is really no reason to hold on the imperial units returned from the database. It makes sense to do the conversion from within the FromRow implementation.

I also think that in practice, macros would be inherently too inflexible for something like this

This is true. Please note, that there is no intention to build an ORM. I just often find myself writing the same code again and again, so I decided to put that into a set of proc-macros (will be opt-in). However, I need FromRow derive to be able to handle generics, so something like this would be possible:

struct Feet(f64);
struct Cm(f64);

impl FromValue for Feet {
    // ...
}

impl FromValue for Cm {
    // ...
}

#[derive(FromRow)]
struct User<T> {
  name: String,
  height: T,
}

Please note, that there is no intention to build an ORM

I understand, but maybe you should introduce a helper attribute for the derive macro, so that I can do something like this:

#[derive(FromRow)]
struct User {
  #[mysql(colname = "user_name")] // attribute for the column name
  name: String,
  age: u8
}

async fn get_user(id: u32, conn: &Conn) -> Result<User, Error> {
  let stmt = r"SELECT user_name, age from users WHERE id = :id".with(params!{"id" => id});
  let user: User = stmt.first(conn).await?.unwrap();
  Ok(user)
}

That would eliminate some of the inflexibility of using macros here. I'll admit that I haven't gone over all the details to gauge the difficulty in implementing that enhancement, but I think it would be a nice feature to have.
For my niche case, I've copied over the take_or_place macro to my local code. The slight repetition and manual tracking of the extracted columns is a worth while trade off for performance I guess. Perhaps you could consider exporting that macro?

For my niche case, I've copied over the take_or_place macro to my local code. The slight repetition and manual tracking of the extracted columns is a worth while trade off for performance

This is, of course, only necessary when the conversion is handling an arity of more than 12 though, since I could've done something like this:

impl FromRow for User {
  fn from_row_opt(row: Row) -> Result<Self, FromRowError> {
    let (name, age) = from_row_opt(row)?;
    let name = format_name(name); // context-specific operation
    Self {name, age}
  }
}

So I guess this could be considered a very special case.

Derive macros are available since v0.32.0