First draft of a dynamic select clause feature in diesel-dynamic-schema

[weiznich]

Blocked on diesel-rs/diesel#2182

[Razican]

Hi, is there a way to test this implementation? I tried importing both Diesel and diesel-dynamic-schema from the Git sources and I'm getting compiling errors, I'm using rustc 1.41.0-nightly (27d6f55f4 2019-12-11):

[dependencies.diesel]
git = "https://github.com/GiGainfosystems/diesel"
branch = "feature/dynamic_queries"
features = ["postgres", "chrono", "uuidv07"]

[dependencies.diesel-dynamic-schema]
git = "https://github.com/GiGainfosystems/diesel-dynamic-schema.git"
branch = "feature/dynamic_query"

I get a bunch of

error: cannot find macro `sql_function_proc` in this scope
   --> /home/razican/.cargo/git/checkouts/diesel-ef4452c01f35a8ae/25233a5/diesel/src/expression/functions/mod.rs:167:9
    |
165 | / macro_rules! sql_function {
166 | |     ($($args:tt)*) => {
167 | |         sql_function_proc! { $($args)* }
    | |         ^^^^^^^^^^^^^^^^^ help: a macro with a similar name exists: `sql_function`
168 | |     }
169 | | }
    | |_- similarly named macro `sql_function` defined here
    | 
   ::: /home/razican/.cargo/git/checkouts/diesel-ef4452c01f35a8ae/25233a5/diesel/src/expression/functions/aggregate_folding.rs:3:1
    |
3   | / sql_function! {
4   | |     /// Represents a SQL `SUM` function. This function can only take types which are
5   | |     /// Foldable.
6   | |     ///
...   |
21  | |     fn sum<ST: Foldable>(expr: ST) -> ST::Sum;
22  | | }
    | |_- in this macro invocation

And

error: cannot find derive macro `NonAggregate` in this scope
  --> /home/razican/.cargo/git/checkouts/diesel-ef4452c01f35a8ae/25233a5/diesel/src/expression/operators.rs:64:65
   |
64 |         #[derive(Debug, Clone, Copy, QueryId, DieselNumericOps, NonAggregate)]
   |                                                                 ^^^^^^^^^^^^
   | 
  ::: /home/razican/.cargo/git/checkouts/diesel-ef4452c01f35a8ae/25233a5/diesel/src/pg/expression/operators.rs:11:1
   |
11 | postfix_operator!(NullsLast, " NULLS LAST", (), backend: Pg);
   | ------------------------------------------------------------- in this macro invocation

And even:

error[E0277]: the trait bound `expression::operators::Eq<pg::metadata_lookup::pg_type::columns::typname, expression::bound::Bound<sql_types::Text, &str>>: expression::NonAggregate` is not satisfied
  --> /home/razican/.cargo/git/checkouts/diesel-ef4452c01f35a8ae/25233a5/diesel/src/pg/metadata_lookup.rs:34:25
   |
34 |                 .filter(typname.eq(type_name))
   |                         ^^^^^^^^^^^^^^^^^^^^^ the trait `expression::NonAggregate` is not implemented for `expression::operators::Eq<pg::metadata_lookup::pg_type::columns::typname, expression::bound::Bound<sql_types::Text, &str>>`
   |
   = note: required because of the requirements on the impl of `query_dsl::filter_dsl::FilterDsl<expression::operators::Eq<pg::metadata_lookup::pg_type::columns::typname, expression::bound::Bound<sql_types::Text, &str>>>` for `query_builder::select_statement::SelectStatement<pg::metadata_lookup::pg_type::table, query_builder::select_clause::SelectClause<(pg::metadata_lookup::pg_type::columns::oid, pg::metadata_lookup::pg_type::columns::typarray)>>

Am I doing something wrong? I'm using Rocket with Diesel, with the same repo (https://github.com/Razican/Rocket/tree/async_dynamic_diesel).

[weiznich]

Just using a git dependency don't work for diesel, because we've a internal dependency on diesel_derives. You need to use a [replace] or [patch.crates-io] section for diesel and diesel_derives.

[Razican]

Cool! that worked. I will now try this :)

For those with the same issue, the required configuration was the following:

[dependencies]
diesel = { version = "=1.4.3", features = ["postgres", "chrono", "uuidv07"] }

[replace]
"diesel:1.4.3" = { git = "https://github.com/GiGainfosystems/diesel", branch = "feature/dynamic_queries" }
"diesel_derives:1.4.1" = { git = "https://github.com/GiGainfosystems/diesel", branch = "feature/dynamic_queries" }

The features can be any, and there is no need to fork Rocket if you are using it.

[Razican]

Hi @weiznich, I seem to be running into problems with this. I am using the code from the dynamic_query3() test, but I'm getting multiple errors:

error[E0277]: the trait bound `diesel_dynamic_schema::dynamic_select::DynamicSelectClause<'_, diesel::pg::Pg, diesel_dynamic_schema::table::Table<&str, &str>>: diesel::Expression` is not satisfied
   --> src/lib.rs:107:15
    |
107 |         users.select(select).load(&*db_conn).unwrap();
    |               ^^^^^^ the trait `diesel::Expression` is not implemented for `diesel_dynamic_schema::dynamic_select::DynamicSelectClause<'_, diesel::pg::Pg, diesel_dynamic_schema::table::Table<&str, &str>>`
    |
    = note: required because of the requirements on the impl of `diesel::query_dsl::select_dsl::SelectDsl<diesel_dynamic_schema::dynamic_select::DynamicSelectClause<'_, diesel::pg::Pg, diesel_dynamic_schema::table::Table<&str, &str>>>` for `diesel_dynamic_schema::table::Table<&str, &str>`

error[E0277]: the trait bound `diesel_dynamic_schema::dynamic_select::DynamicSelectClause<'_, diesel::pg::Pg, diesel_dynamic_schema::table::Table<&str, &str>>: diesel::Expression` is not satisfied
   --> src/lib.rs:107:22
    |
107 |         users.select(select).load(&*db_conn).unwrap();
    |                      ^^^^^^ the trait `diesel::Expression` is not implemented for `diesel_dynamic_schema::dynamic_select::DynamicSelectClause<'_, diesel::pg::Pg, diesel_dynamic_schema::table::Table<&str, &str>>`

error[E0277]: the trait bound `diesel_dynamic_schema::dynamic_select::DynamicSelectClause<'_, diesel::pg::Pg, diesel_dynamic_schema::table::Table<&str, &str>>: diesel::Expression` is not satisfied
   --> src/lib.rs:107:30
    |
107 |         users.select(select).load(&*db_conn).unwrap();
    |                              ^^^^ the trait `diesel::Expression` is not implemented for `diesel_dynamic_schema::dynamic_select::DynamicSelectClause<'_, diesel::pg::Pg, diesel_dynamic_schema::table::Table<&str, &str>>`
    |
    = note: required because of the requirements on the impl of `diesel::query_dsl::LoadQuery<_, _>` for `diesel::query_builder::SelectStatement<diesel_dynamic_schema::table::Table<&str, &str>, diesel::query_builder::select_clause::SelectClause<diesel_dynamic_schema::dynamic_select::DynamicSelectClause<'_, diesel::pg::Pg, diesel_dynamic_schema::table::Table<&str, &str>>>>`

error[E0277]: the trait bound `diesel_dynamic_schema::dynamic_value::DynamicRow<diesel_dynamic_schema::dynamic_value::NamedField<list_table::{{closure}}#0::MyDynamicValue>>: diesel::Queryable<_, diesel::pg::Pg>` is not satisfied
   --> src/lib.rs:107:30
    |
107 |         users.select(select).load(&*db_conn).unwrap();
    |                              ^^^^ the trait `diesel::Queryable<_, diesel::pg::Pg>` is not implemented for `diesel_dynamic_schema::dynamic_value::DynamicRow<diesel_dynamic_schema::dynamic_value::NamedField<list_table::{{closure}}#0::MyDynamicValue>>`
    |
    = note: required because of the requirements on the impl of `diesel::query_dsl::LoadQuery<diesel::PgConnection, diesel_dynamic_schema::dynamic_value::DynamicRow<diesel_dynamic_schema::dynamic_value::NamedField<list_table::{{closure}}#0::MyDynamicValue>>>` for `diesel::query_builder::SelectStatement<diesel_dynamic_schema::table::Table<&str, &str>, diesel::query_builder::select_clause::SelectClause<diesel_dynamic_schema::dynamic_select::DynamicSelectClause<'_, diesel::pg::Pg, diesel_dynamic_schema::table::Table<&str, &str>>>>`

error: aborting due to 4 previous errors

I checked the code, and it seems that the Expression trait for DynamicSelectClause requires the postgres feature, not available in the crate's Cargo.toml:

#[cfg(feature = "postgres")]
impl<'a, QS> Expression for DynamicSelectClause<'a, diesel::pg::Pg, QS> {
    type SqlType = crate::dynamic_value::Any;
}

Maybe I'm doing something wrong? I tried to compile the crate and to run the tests, but I'm getting similar errors to those in Travis.

[weiznich]

@Razican That PR version was not really meant to be used from someone, it was just there for discussion of the API. I've uploaded a fixed version that should work as long as you use the same diesel version everywhere.

[Razican]

Thanks @weiznich, it compiles now. I will try it out to see how it feels to use it, but it seems pretty good to me for now. I would maybe add the MyDynamicValue as a type of this crate, with all the Diesel types, in order to enforce consistency.

[weiznich]

In my opinion exposing something like MyDynamicValue is not a great idea, because it really depends on the use case what values you want to support there. Maybe adding a custom derive for the FromSql impl could be a variant. (That's something that should be discussed with @diesel-rs/contributors ).

[Razican]

After working with this for some time, I feel that DynamicRow would benefit from implementing Iterator<NamedField<I>> or IntoIterator, or something to be able to iterate on the fields of the row, without having to know the column names in advance.

I had to implement this to convert the row into JSON:

// Transform the list of records to JSON format.
Ok(record_list
    .into_iter()
    .map(|row| {
        let mut record = Record::new();
        for column_name in &column_list {
            if let Some(value) = row.get_by_name(column_name) {
                let _ = record.insert(column_name.to_owned(), value.clone().into());
            }
        }
        record
    })
    .collect())

But it would be easier being able to iterate over the row fields. It would be more efficient and I avoid passing the list of columns.

[Razican]

Some extra useful feature in DynamicSelectClause would be to be able to get internal fields, or to at least be able to PartialEq it. This is useful for testing purposes:

Since all the select clause is dynamic, it makes sense to be able to test that the proper types are being added to it.

[Razican]

Hi @weiznich does this now depend on the 2.0 branch of Diesel? Is there a way to make it work wit 1.4? If not, I guess it will not be possible to integrate this with a Rocket application for the time being, right?

[weiznich]

@Razican This depends on the diesel 2.0 branch from the beginning. There is no way to make this ever work with 1.4. Additionally to make this clear again: This is a development version. I do not want to support running that in third party applications in any way, for obvious reasons. If you really want to use a random unfinished WIP PR in your application your are on your own. Otherwise the fasted way to get this workable in an official release is by contributing to diesel/diesel-dynamic-schema for the remaining release blocker.