Purpose of Rc<Refcell<T>>

[95th]

I see most of the type returned from traits functions are Rc<Refcell>. What is the reason behind this? Shouldn't the user wrap it himself if he needs it?

[golddranks]

I agree, this feels somehow very un-Rusty. Are they meant to be a temporary clutch for accelerating the development for starters, or is there something in the design that requires them?

[95th]

I started on this something like an year back. I liked the way JDBC abstracts over lots of dbs.

I believe in the end, we want something like:

pub trait Driver: Sync + Send {
    /// Create a connection to the database. Note that connections are intended to be used
    /// in a single thread since most database connections are not thread-safe
    async fn connect<T>(&self, url: T) -> Result<impl Connection>
    where
        T: AsRef<str>;
}

pub trait Connection {
    async fn create_statement<T>(&mut self, sql: T) -> Result<impl Statement + '_>
    where
        T: AsRef<str>;
}

pub trait Statement {
    async fn execute_query(&mut self) -> Result<impl ResultSet + '_>;
}

So that we can write stuff like:

pub async fn do_stuff(driver: &impl Driver) -> Result<()> {
    let conn = driver.connect("url").await?;
    let mut stmt = conn.create_statement("select * from users").await?;
    let rs = stmt.execute_query().await?;
    // use rs here
    Ok(())
}

Now, this requires GATs, impl trait and async functions in trait methods.

For async, for now you could return a boxed future..
For impl trait, you could use associated types. (But it quickly becomes verbose)
For GAT, there is no real alternative here.

So, without GATs, we get (incorrect):

use std::future::Future;

pub trait Driver: Sync + Send {
    type Connection: Connection;

    /// Create a connection to the database. Note that connections are intended to be used
    /// in a single thread since most database connections are not thread-safe
    fn connect<T>(&self, url: T) -> Box<dyn Future<Output = Result<Self::Connection>> + '_>
    where
        T: AsRef<str>;
}

pub trait Connection {
    type Statement: Statement;

    fn create_statement<T>(
        &mut self,
        sql: T,
    ) -> Box<dyn Future<Output = Result<Self::Statement>> + '_>
    where
        T: AsRef<str>;
}

pub trait Statement {
    type ResultSet: ResultSet;

    fn execute_query(&mut self) -> Box<dyn Future<Output = Result<Self::ResultSet>> + '_>;
}

You can get rid boxed future without more associated types but then you can't specify that the future's lifetime is bound to self. So this is incorrect.

use std::future::Future;

pub trait Driver: Sync + Send {
    type Connection: Connection;
    type ConnectionFuture: Future<Output = Result<Self::Connection>>;

    /// Create a connection to the database. Note that connections are intended to be used
    /// in a single thread since most database connections are not thread-safe
    fn connect<T>(&self, url: T) -> Self::ConnectionFuture
    where
        T: AsRef<str>;
}

pub trait Connection {
    type Statement: Statement;
    type StatementFuture: Future<Output = Result<Self::Statement>>;

    fn create_statement<T>(&mut self, sql: T) -> Self::StatementFuture
    where
        T: AsRef<str>;
}

pub trait Statement {
    type ResultSet: ResultSet;
    type ResultSetFuture: Future<Output = Result<Self::ResultSet>>;

    fn execute_query(&mut self) -> Self::ResultSetFuture;
}

With just the GAT, we will be able to write the correct (although verbose) version:

pub trait Driver: Sync + Send {
    type Connection: Connection;
    type ConnectionFuture<'a>: Future<Output = Result<Self::Connection>> + 'a;

    /// Create a connection to the database. Note that connections are intended to be used
    /// in a single thread since most database connections are not thread-safe
    fn connect<T>(&self, url: T) -> Self::ConnectionFuture<'_>
    where
        T: AsRef<str>;
}

pub trait Connection {
    type Statement<'a>: Statement + 'a;
    type StatementFuture<'a>: Future<Output = Result<impl Statement + 'a>> + 'a;

    fn create_statement<T>(
        &mut self,
        sql: T,
    ) -> Self::StatementFuture<'_>
    where
        T: AsRef<str>;
}

pub trait Statement {
    type ResultSet<'a>: ResultSet + 'a;
    type ResultSetFuture<'a>: Future<Output = Result<Self::ResultSet<'a>>> + 'a;

    fn execute_query(&mut self) -> Self::ResultSetFuture<'_>;
}

The future rust releases should help with the verbosity i.e. async sugar for associated future and impl trait for associated SQL type.

You'll say "whats wrong with dyn Connection and dyn Preparedstatement. Well apart from being slower (I wonder if it matters that much since DB calls are going to be more expensive anyway), but it limits what we can do in trait methods.. You can't have generic params and use Self type in the trait because that is prohibited in trait objects.

[andygrove]

I'm all for moving away from Rc<RefCell<T>>. I used this approach because I didn't know better. Does someone want to create a PR to change this? I thought that dyn was required because there are different implementations and the type isn't always known at compile-time e.g. in the case of dynamically loaded drivers but I could be wrong.

[golddranks]

I think that dyn is basically a hard requirement for this crate, because, as I understand it, one of the design decisions is to be able to change the DB without recompiling. However, I think that shouldn't mean that's part of the public API. I imagine there could be a public types that encapsulate the dynamicity and inner mutability.

[golddranks]

There could be a struct Connection and each of the backend could know how to convert into that. The struct could (possibly using unsafe, or just containing a boxed dyn for starters) encapsulate the dynamicity. That enables the type layout to be more freely expressed by this crate, instead of leaving the decision to the language.

[jnicholls]

@golddranks Yeah good call on the inner dynamics with external API having a steady interface.