fernandobatels / rsfbclient

Rust Firebird Client
MIT License
76 stars 11 forks source link

API Ergonomics: Consider reorganizing lifetimes in rsfbclient main crate traits/structs #51

Closed dancingbug closed 4 years ago

dancingbug commented 4 years ago

The lifetime in Queryable<'a, R> (and Transaction<'a,C>, StmtIter<'a, R,C> etc) will tend to infect whatever uses it. Unfortunately, due to some warts of the rust language, this leads to unexpected compilation errors in some cases. For example:

  const GTT_QRY: &str =
  "select rdb$relation_name
     from rdb$relations
    where     rdb$relation_name = 'GTT_TABLE'
          and rdb$relation_type > 3;";

  const GTT_CREATE: &str =
    "create global temporary table GTT_TABLE
     (table_id INT not null primary key)
     on commit preserve rows;";

  pub fn with_temporary_table<'a, E>( conn: &'a mut E ) -> ()
    where 
      E: Execute + Queryable<'a, (String,)>
  {
    let result : Vec<(String,)> = conn.query(GTT_QRY, ()).unwrap();
    let result2 =  conn.execute(GTT_CREATE, ());
  }

error[E0499]: cannot borrow `*conn` as mutable more than once at a time
   --> src/main.rs:100:20
    |
95  |   pub fn with_temporary_table<'a, E>( conn: &'a mut E ) -> ()
    |                               -- lifetime `'a` defined here
...
99  |     let result : Vec<(String,)> = conn.query(GTT_QRY, ()).unwrap();
    |                                   -----------------------
    |                                   |
    |                                   first mutable borrow occurs here
    |                                   argument requires that `*conn` is borrowed for `'a`
100 |     let result2 =  conn.execute(GTT_CREATE, ());
    |                    ^^^^ second mutable borrow occurs here

pardon if i'm misunderstanding something, but I also also don't see why R of Queryable<'a, R> should be bounded by 'a at the trait level like it is in its where clause either: where R : FromRow + 'a It seems like the lifetime of R should be unrelated to the connection/transaction/iterator at the trait level: Once we have an R on hand, we should be able to do whatever we want with it, or the Connection, in either order, if our impl allows it.

jairinhohw commented 4 years ago

I've made some changes here, if you can take a look to see if it fixes your problems or have a suggestion on a better way to implement.

dancingbug commented 4 years ago

Your changes work for me. I went a bit further as you saw (#54), leaving no params on Queryable, which feels quite a bit nicer to use.

However, this change is introducing the only Box<...> in the main crate, which seems undesirable (does this affect embedded? Edit: Silly me, I had mistakenly believed 'Firebird embedded' meant embedded in the hardware sense up until today )

I think there should be a better way. I might look into it more. Is there a detailed roadmap or a best place to read/ask about general design considerations?

jairinhohw commented 4 years ago

I don't think there is need to worry about the Box, but this can be discussed in the PR (#54).

About the detailed roadmap, we don't have nothing like this that I'm aware.