Sqlite status?

[Progdrasil]

I've been toying with the idea of making a similar Query Builder for Sqlite. But adding support to this library seems like a much better idea.

In the readme it says that sqlite support is a work in progress. What still needs to be done on that front?

[tyt2y3]

Good morning! Thank you for your interest. I think the builder backend is there already. https://github.com/SeaQL/sea-query/blob/master/src/backend/sqlite/query.rs

But currently the test suite is empty https://github.com/SeaQL/sea-query/blob/master/tests/sqlite/online.rs , so we say it's still WIP. If it fits your use case, we'd welcome contribution!

Please see if the built result is syntactically correct, and perhaps add a few test cases where you see fit. If there are issues, we can discuss.

[Progdrasil]

So I've finally had the time to start evaluating this library with a colleague of mine to replace our internal ad-hoc query builder. And we've hit a few snags. Before bombarding the repository with issues I'd rather get the okay from the maintainers. Of course we'll provide the associated PRs on the related approved features. Most of these snags aren't show stoppers though and we have work arounds for most.

Value type

First off regarding the Value type, If we take for example Rusqlite they have a very similar trait as the postgres-types ToSql (they're even named the same). This permits the use of non-primitive types and newtypes in their queries. From what I understood from the code base, a sqlite driver would be necessary for use with this trait. A From blanket implementation on all types implementing rusqlite::ToSql that converts the output of type rusqlite::types::ToSqliteOutput into this crates Value type. Then a trait similar to the PostgresDriver trait to convert the values to rusqlite's trait objects.

Our current possible workaround is to create newtypes around sea_query::value::{Value, Values} To implement From<impl ToSql> and ToSql on them. But having support directly in the library would be great.

Sql functions

Second point would be about sql functions. One such that we use frequently is the FIRST_VALUE window function. From what I gather, sql functions live in the Expr type as methods. I'm not sure how window functions would fit into this though since it takes special subqueries. Heres an example of a query we use:

SELECT DISTINCT name FROM table_n
    JOIN table_d ON table_n.id = table_d.n_id,
    JOIN table_s ON table_s.id = (
        SELECT DISTINCT FIRST_VALUE(table_d.s_id) OVER (
            PARTITION BY table_d.n_id
            ORDER BY table_d.created DESC
        )
        FROM table_d
        WHERE table_d.n_id = table_n.id
    );

We can substitute it with the following though:

let sub_query_result: Rc<dyn sea_query::Iden> =
    Rc::new(sea_query::Alias::new("sub_query_result"));

Query::select()
    .distinct()
    .columns(TableN::Name)
    .from(TableN::Table)
    .join(
        JoinType::Join,
        TableD::Table,
        Expr::tbl(TableN::Table, TableN::Id)
            .equals(TableD::Table, TableD::NId),
    )
    .join_subquery(
        JoinType::Join,
        Query::select()
            .distinct()
            .column(TableD::SId)
            .order_by(
                (TableD::Table, TableD::Created),
                Order::Desc,
            )
            .from(TableD::Table)
            .and_where(
                Expr::tbl(TableD::Table, TableD::NestId)
                    .equals(TableN::Table, TableN::Id)
            )
            .limit(1)
            .take()),
        sub_query_result.clone(),
        Expr::tbl(TableS::Table, TableS::Id)
            .equals(sub_query_result.clone(), TableD::SId)
    )

Which works for this particular case, but others might not, only when we attack all of them and verify that our tests still pass will we know.

Use IntoIterator

This is a more general one. Thoughout exploring the api we noticed many apis take Vecs by ownership in order to use .into_iter() pretty much imediately afterwards. Would it be possible to make those functions be generic over the iterator type? Take for example SelectStatement::columns which has a signature as follows:

pub fn columns<T>(&mut self, cols: Vec<T>) -> &mut Self
    where T: IntoColumnRef

This could become

pub fn columns<T, I>(&mut self, cols: I) -> &mut Self
    where
        T: IntoColumnRef,
        I: IntoIterator<Item = T>

My colleague tried it out and only needed to change the deprecated table_column function body which just needed the collect to specify the collection type. Although this would clearly be a breaking change it would make it more idiomatic

[tyt2y3]

First of all, thank you for the detailed analysis! Although we do not use Sqlite heavily, it is still our goal to support Sqlite and provide integration for it. I am willing to spend some effort on it too, so let's align and avoid duplication.

• Value Sorry for a bit unfamilar to the use case. I understand the latter part "Then a trait similar to the PostgresDriver trait to convert the values to rusqlite's trait objects.", but why is "A From blanket implementation on all types implementing rusqlite::ToSql that converts the output of type rusqlite::types::ToSqliteOutput into this crates Value type" necessary? From my imagination, rusqlite is the consumer of sea_query 's output. I am okay with any added convenience, just being curious on the use case.

• Function I guess the problem is not in FIRST_VALUE, as custom function is possible now through https://docs.rs/sea-query/0.9.4/sea_query/func/struct.Func.html#method.cust and like .expr(Func::cust(MyFunc::first_value).into_simple_expr()). I do have a plan to aggregate all built-in functions under extensions, providing something like extension::sqlite::Func::first_value. So I think the problem is in the syntax OVER and PARTITION BY, as these are not supported yet.

This requires some working on the syntax tree representation. If you can provide a couple of test cases, I will work on the API to support these.

• IntoIterator Yes I agree. I'd appreciate a PR on this.

[Progdrasil]

For the Value take for example using a non-primitive type as the primary key, such as a Uuid. Now i could do the following:

    let instance_id = uuid::Uuid::new_v4();
    Query::select()
        .column(Test::Name)
        .from(Test::Table)
        .and_where(Expr::tbl(Test::Table, Test::Id).eq(instance_id.as_bytes().to_vec()));

However its pretty verbose and requires knowledge of the implementation of the type in the underlying library (rusqlite).

However, lets say you have the following:

impl <T> From<T> for Value where T: rusqlite::ToSql { /*.. */ }

it could be simplified to:

    let instance_id = uuid::Uuid::new_v4();
    Query::select()
        .column(Test::Name)
        .from(Test::Table)
        .and_where(Expr::tbl(Test::Table, Test::Id).eq(instance_id));

[Progdrasil]

For the function, the PARTITION BY and OVER seem to be specific to window functions (sorry for it being a tutorial link, i couldn't find a standard's documentation for this that general, but here's the documentation for sqlite and postgres)

I can provide some example tests for those that we use, but i'm by no means a sql expert so there might be better ways to do them.

[tyt2y3]

For the Value take for example using a non-primitive type as the primary key, such as a Uuid. Now i could do the following:

I am keen to solve this inconvenience. Though I think UUID is a special case because it can be losslessly represented in bytes. I'd prefer to create one more variant under Value for UUIDv4 and UUIDv6 respectively and guard them behind feature with-uuid. Like what has been done for DateTime support. This way, no conversion is necessary, and the value will be passed as-is to the underlying driver.

The real problem is, sea-query cannot depend directly on the sqlite crate. postgres has a separate crate postgres-types, and thus I can implement ToSql without requiring the full library. The rationale is, sea-query is a library but not the ORM. So dependency should be 'provided by the user'.

To mitigate this, I actually tried exporting a macro in sqlx support. marco exported in https://github.com/SeaQL/sea-query/blob/master/src/driver/sqlx_postgres.rs and imported from https://github.com/SeaQL/sea-query/blob/master/examples/sqlx_postgres/src/main.rs#L5

Do you think a similar solution for Sqlite is good for you? Do we have other alternatives?

[Progdrasil]

I can understand that, we'll most likely do a newtype to implement ToSql from sqlite to pass value as parameters to the database connection. Hopefully maintainers of rusqlite will approve the refactor of types to another crate. If not we'll stick with our workaround.

[tyt2y3]

I tried making a macro based solution for driving rusqlite. See if you like it!

[tyt2y3]

We have already tackled UUID, rusqlite driver and IntoIterator on 0.10.0 I'd like to close this issue for now if you have no amendment. Please create a new issue for window functions support.

[Progdrasil]

Yes, the macro based solution seems to work pretty well. I can't wait to try and integrate the new version!

[duchainer]

Tried the features "rusqlite" and "with-uuid", they work as intended for what I've tried. Thanks a lot!

[tyt2y3]

Cheers. Thank you for your contribution too.