an incomplete attempt at sqlite support

[rich-murphey]

This is an incomplete attempt to add sqlite support.

I hope some part of it is useful.

I got stuck trying to decide how to resolve an issue reported by the ormx::Table macro: "temporary value dropped while borrowed".

It does not report which value is dropped, so I'm not sure why this is occruing. One wild guess is that ormx::Table impl returns a future referencing fields in self. But that doesn't make sense because in the caller, self outlives the return from await on that future.

   Compiling example-sqlite v0.1.0 (/u20c/b/rich/w/ormx/example-sqlite)
error[E0716]: temporary value dropped while borrowed
  --> example-sqlite/src/main.rs:64:17
   |
64 | #[derive(Debug, ormx::Table)]
   |                 ^^^^^^^^^^^
   |                 |
   |                 creates a temporary which is freed while still in use
   |                 temporary value is freed at the end of this statement
   |                 borrow later used by call
   |
   = note: consider using a `let` binding to create a longer lived value
   = note: this error originates in the derive macro `ormx::Table` (in Nightly builds, run with -Z macro-backtrace for more info)

where the expanded macro contains this:

impl ormx::Table for User {
    type Id = i64;
    fn id(&self) -> Self::Id {
        self.user_id
    }
    fn get<'a, 'c: 'a>(
        db: impl sqlx::Executor<'c, Database = ormx::Db> + 'a,
        id: Self::Id,
    ) -> ormx::exports::futures::future::BoxFuture<'a, sqlx::Result<Self>> {
        Box::pin(async move {
            { { # [allow (clippy :: all)]
                   { use :: sqlx :: Arguments as _ ;
                     let arg0 = & (id) ;
                     ...
                     . fetch_one (db) . await
        })
    }                                                                                                                                                                                                                                                                                                                                        Syntax Error: expected SEMICOLON [syntax-error]
....

I'm not sure how to get better error messages to narrow down the issue.

[NyxCode]

first of all, thanks for your work! really appreciate it.

I remember running into this - this is caused by the streams returned by query(_as)! on sqlite. They seem to borrow a local variable and cannot be returned from methods.
I briefly talked with mehcode on the sqlx discord about this - here are some links to the conversation:

• https://discordapp.com/channels/665528275556106240/694821775866986527/826194724565287012
• https://discordapp.com/channels/665528275556106240/694821775866986527/828389402308378634
• https://discordapp.com/channels/665528275556106240/694821775866986527/907167597991194626

I think we should just put the functions which return streams behind a #[cfg(not(feature = "sqlite"))] feature gate until the API of sqlx is consistent across backends in this regard

[rich-murphey]

Thanks so much! I really appreciate the pointers to the discussion, and the encouragement.

I'm interested in trying an idea about the lifetime issue, potentially just for the sqlite case, and temporarily till sqlx 0.6 may make it unnecessary.

I've seen the same kind of issue occur between the interfaces of actix-web an sqlx. An acix-web streaming response needs a BoxStream that owns it's data, whereas a sqlx stream references it's data. One possible solution is to have an outer wrapper stream that owns the query parameter values, and owns the inner sqlx stream of rows.

Here's an example. Note the SelfRefStream is an outer stream that owns the query parameters referenced by an inner sqlx stream.

#[post("/widgets")]
pub async fn widgets(
    web::Json(params): web::Json<WidgetParams>,
    pool: web::Data<PgPool>,
) -> HttpResponse {
    HttpResponse::Ok()
        .content_type("application/json")
        .streaming(
            // a stream of text (Bytes) containing a JSON array of sqlx records
            ByteStream::new(
                // a stream of WidgetRecords that owns pool and params

                SelfRefStream::build(
                    (pool.as_ref().clone(), params),
                    // a stream of WidgetRecords that borrows pool and params
                    move |(pool, params)| {
                        sqlx::query_as!(
                            WidgetRecord,
                            "SELECT * FROM widgets LIMIT $1 OFFSET $2 ",
                            params.limit,
                            params.offset
                        )
                            .fetch(pool)
                    }),

                |buf: &mut BytesWriter, record: &WidgetRecord| {
                    // this writes a WidgetRecords as JSON text to the output buffer
                    serde_json::to_writer(buf, record).map_err(ErrorInternalServerError)
                },
            ),
        )
}

and here's the implementation of SelfRefStream and more discussion about this example.

Maybe this has some potential for a temporary solution. It has some overhead for the additional allocations when the SelfRefStream takes ownership of the query parameters, so perhaps it should be limited to only where it's rquired -- for sqlite only.

[rich-murphey]

The above commit is a proof of concept.

One crucial issue is shown above in the actix-web widgets() method. Note that widgets() creates a stream, and returns the stream to actix-web. The lifetime of the stream is longer than the lifetime of the widgets() method. For that reason, the stream cannot reference values that live only within the widgets() method. Rather, the stream must own all of the parameters used to create it.

Similarly, ormx methods such as _streamall and _stream_allpaginated return a stream that in the general case, will outlive the caller of these methods. To achieve that, all of the ormx methods that return a stream need to take ownership of the parameters, including the executor, and any offset or limit on paged stream. In order to do that, these methods take a &Pool, and clone the pool in order for the stream to own the pool object. This is cheap since Pool is an Arc.

I should probably discuss much more, but gad, debugging proc macros has been a little overwhelming, and I need a break. I'm sure there are issues, but this compiles and runs example-sqlite, so I thought I'd share this first draft rather than delay further.

I have not tested how these changes affect postgres and mysql. In that respect, these changes are incomplete, and this only works for sqlite. More work is needed to ensure the changes affect only the sqlite feature.

[rich-murphey]

This coming week I plan to take another pass at this. I'll try to reduce this down to the minimum changes necessary to make the SQLite support equivalent to postgres support.

[rich-murphey]

I've made some progress. The SelfRefStream is not needed whenever the stream is consumed within the scope it is constructed. This in turn means, all of the methods can take an Executor, so that they accept either a pool or connection.

I'll clean up the diffs and submit probably by early next week.

[rich-murphey]

I minimized the changes, and tested both example-postgres and example-sqlite.

For sqlite, this is still incomplete in that I was not able to implement the conditional_query_as!() macro. I don't yet understand enough of the __build_query macro to identify where changes are needed to support sqlite.

If you find any of this useful, just let me know if changes or further work is preferred!

[rich-murphey]

abonander opened a sqlx PR 1551 that may make it unnecessary to use a wrapper to take ownership of limit and offset (see SqliteArgumentValue::into_static). I haven't actually tried it, but it looks like converting the arguments to a 'static lifetime would satisfy the borrow checker.

That may also make the conditional_query_as!() macro work. It should eliminate many of these changes. I'll plan to further reduce this PR when sqlx PR 1551 is merged.

[rich-murphey]

On Discord, abonander said that sqlx PR 1551 changes are internal and do not change the sqlx API, so it won't solve the ownership issues.

[Quiark]

Hi @rich-murphey isn't the ormx-macros/src/backend/sqlite file missing?

[NyxCode]

I'm closing this for now.
I haven't touched ormx for quite a while, so I have to assume most of this is sadly outdated.