Trying to make dynamic filtering work on Diesel 2.0

Razican · September 25, 2020, 11:55am

Hello!
This is related to this topic, but since things have changed since then, I think it’s best to start a new topic.

I have the following code, that was working fine, but latest changes for Diesel 2.0 have made it fail in compilation, and the error messages are pretty difficult to understand:

// Some type definitions to simplify the code:
type DynTable = diesel_dynamic_schema::Table<String>;
type DynExpr = Box<dyn BoxableExpression<DynTable, Pg, SqlType = diesel::sql_types::Bool>>;

pub struct FilterClause {
    field: String,
    operator: Operator,
    value: Option<String>,
}

impl FilterClause {
    fn expression_ty<T, E, V>(&self, table: &DynTable) -> Result<DynExpr>
    where
        T: FromStr<Err = E> + AsExpression<V>,
        E: StdError + Send + Sync + 'static,
        V: SingleValue + 'static,
        <T as AsExpression<V>>::Expression:
            NonAggregate + QueryFragment<Pg> + SelectableExpression<DynTable> + 'static,
    {
        use diesel::ExpressionMethods;

        let field = self.get_field::<V>(table);

        let res: DynExpr = match (self.operator, self.value.as_ref()) {
            (Operator::IsEmpty, None) => Box::new(field.is_null()),
            (Operator::IsNotEmpty, None) => Box::new(field.is_not_null()),
            (Operator::SameAs, Some(value)) => Box::new(field.eq(table.column(value.to_owned()))),
            (Operator::NotSameAs, Some(value)) => {
                Box::new(field.ne(table.column(value.to_owned())))
            }
            (Operator::In, Some(value)) => {
                let list: Vec<T> = value
                    .parse::<StrList<T>>()
                    .context("could not parse list")?
                    .into();
                Box::new(field.eq_any(list))
            }
            (Operator::NotIn, Some(value)) => {
                let list: Vec<T> = value
                    .parse::<StrList<T>>()
                    .context("could not parse list")?
                    .into();
                Box::new(field.ne_all(list))
            }
            (Operator::Equals, Some(value)) => Box::new(field.eq(value.parse::<T>()?)),
            (Operator::DoesNotEqual, Some(value)) => Box::new(field.ne(value.parse::<T>()?)),
            (Operator::GreaterThan, Some(value)) => Box::new(field.gt(value.parse::<T>()?)),
            (Operator::GreaterOrEqual, Some(value)) => Box::new(field.ge(value.parse::<T>()?)),
            (Operator::LessThan, Some(value)) => Box::new(field.lt(value.parse::<T>()?)),
            (Operator::LessOrEqual, Some(value)) => Box::new(field.le(value.parse::<T>()?)),
            (Operator::On, Some(_value)) => unimplemented!("ON operator for filtering"),
            _ => unreachable!("should not have arrived here"), // TODO: log error and don't panic
        };

        Ok(res)
    }
}

The Operator type is just a simple enum that has the different possible operations.

This generates many, many errors, and of different types. For example:

error[E0277]: the trait bound `<V as diesel::sql_types::SqlType>::IsNull: diesel::sql_types::OneIsNullable<<V as diesel::sql_types::SqlType>::IsNull>` is not satisfied
   --> common/src/database/filter/filter_clause.rs:244:17
    |
244 |                 Box::new(field.ne(table.column(value.to_owned())))
    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `diesel::sql_types::OneIsNullable<<V as diesel::sql_types::SqlType>::IsNull>` is not implemented for `<V as diesel::sql_types::SqlType>::IsNull`
    |
    = note: required because of the requirements on the impl of `diesel::Expression` for `diesel::expression::operators::NotEq<diesel_dynamic_schema::Column<diesel_dynamic_schema::Table<std::string::String>, std::string::String, V>, diesel_dynamic_schema::Column<diesel_dynamic_schema::Table<std::string::String>, std::string::String, V>>`
    = note: required for the cast to the object type `dyn diesel::BoxableExpression<diesel_dynamic_schema::Table<std::string::String>, diesel::pg::Pg, SqlType = diesel::sql_types::Bool, IsAggregate = diesel::expression::is_aggregate::No>`
help: consider further restricting the associated type
    |
233 |             NonAggregate + QueryFragment<Pg> + SelectableExpression<DynTable> + 'static, <V as diesel::sql_types::SqlType>::IsNull: diesel::sql_types::OneIsNullable<<V as diesel::sql_types::SqlType>::IsNull>
    |                                                                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

If I add those boundaries, then I get the following error:

error[E0277]: the trait bound `<<V as diesel::sql_types::SqlType>::IsNull as diesel::sql_types::OneIsNullable<<V as diesel::sql_types::SqlType>::IsNull>>::Out: diesel::sql_types::MaybeNullableType<diesel::sql_types::Bool>` is not satisfied
   --> common/src/database/filter/filter_clause.rs:245:17
    |
245 |                 Box::new(field.ne(table.column(value.to_owned())))
    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `diesel::sql_types::MaybeNullableType<diesel::sql_types::Bool>` is not implemented for `<<V as diesel::sql_types::SqlType>::IsNull as diesel::sql_types::OneIsNullable<<V as diesel::sql_types::SqlType>::IsNull>>::Out`
    |
    = note: required because of the requirements on the impl of `diesel::Expression` for `diesel::expression::operators::NotEq<diesel_dynamic_schema::Column<diesel_dynamic_schema::Table<std::string::String>, std::string::String, V>, diesel_dynamic_schema::Column<diesel_dynamic_schema::Table<std::string::String>, std::string::String, V>>`
    = note: required for the cast to the object type `dyn diesel::BoxableExpression<diesel_dynamic_schema::Table<std::string::String>, diesel::pg::Pg, SqlType = diesel::sql_types::Bool, IsAggregate = diesel::expression::is_aggregate::No>`
help: consider further restricting the associated type
    |
234 |         <V as SqlType>::IsNull: OneIsNullable<<V as SqlType>::IsNull>, <<V as diesel::sql_types::SqlType>::IsNull as diesel::sql_types::OneIsNullable<<V as diesel::sql_types::SqlType>::IsNull>>::Out: diesel::sql_types::MaybeNullableType<diesel::sql_types::Bool>
    |                                                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

And adding this new boundary, gives the following error:

error[E0271]: type mismatch resolving `<diesel::expression::operators::LtEq<diesel_dynamic_schema::Column<diesel_dynamic_schema::Table<std::string::String>, std::string::String, V>, <T as diesel::expression::AsExpression<V>>::Expression> as diesel::Expression>::SqlType == diesel::sql_types::Bool`
   --> common/src/database/filter/filter_clause.rs:242:28
    |
242 |           let res: DynExpr = match (self.operator, self.value.as_ref()) {
    |  ____________________________^
243 | |             (Operator::IsEmpty, None) => Box::new(field.is_null()),
244 | |             (Operator::IsNotEmpty, None) => Box::new(field.is_not_null()),
245 | |             (Operator::SameAs, Some(value)) => Box::new(field.eq(table.column(value.to_owned()))),
...   |
270 | |             _ => unreachable!("should not have arrived here"), // TODO: log error and don't panic
271 | |         };
    | |_________^ expected struct `diesel::sql_types::Bool`, found associated type
    |
    = note:       expected struct `diesel::sql_types::Bool`
            found associated type `<<<V as diesel::sql_types::SqlType>::IsNull as diesel::sql_types::OneIsNullable<<V as diesel::sql_types::SqlType>::IsNull>>::Out as diesel::sql_types::MaybeNullableType<diesel::sql_types::Bool>>::Out`
    = help: consider constraining the associated type `<<<V as diesel::sql_types::SqlType>::IsNull as diesel::sql_types::OneIsNullable<<V as diesel::sql_types::SqlType>::IsNull>>::Out as diesel::sql_types::MaybeNullableType<diesel::sql_types::Bool>>::Out` to `diesel::sql_types::Bool`
    = note: for more information, visit https://doc.rust-lang.org/book/ch19-03-advanced-traits.html
    = note: required for the cast to the object type `dyn diesel::BoxableExpression<diesel_dynamic_schema::Table<std::string::String>, diesel::pg::Pg, SqlType = diesel::sql_types::Bool, IsAggregate = diesel::expression::is_aggregate::No>`

So it seems I cannot make it work. I was trying to go through the documentation, but there is no “guide-like” documentation to understand all these traits and types as a whole, and what should you expect on each case, so I don’t know if there is something directly wrong with my approach, or if there is something I’m missing.

weiznich · September 25, 2020, 12:11pm

That’s likely a fallout of this commit. I would call this expected as the old behavior of handling expressions on nullable values was just incomplete (or better simplified). As for the documentation: Please not that you are using an unreleased version of diesel. Additionally this touches the very core of diesel, which we normally don’t want to expose public. Additionally documenting this stuff is really a lot work, which at least I do not have the time for.

As for fixing your issue, I see two possible solutions:

Either correctly calculate the the required types so that for V::IsNull == NotNull + T == Bool and for all other combinations (IsNullable or Option<T>) == Nullable<Bool>
Or restrict the implementation to such fields where V::IsNull == NotNull and T is not an Option.

Razican · September 25, 2020, 2:48pm

I understand this, and I would rather not go so deep in using inner diesel core structures. I would just like to be able to dynamically filter, order and select in table unknown at compile time (and in the future, ideally, create, drop, alter and delete in those tables). I am hoping to do this with diesel_dynamic_schema, but I don’t manage to make it easily work. Is there a better way to achieve this with diesel?

For this case, for checking if a fields equals a value, I guess we wouldn’t need to check if the value can be null or not, right?

weiznich · September 28, 2020, 9:46am

Just to write that down somewhere: First of all diesel is fundamentally designed to check things at compile time. What you are trying to do is fundamentally opposed to the basic principles used for designing diesel. That does not mean it is not possible to use diesel (or any crate built on top of diesel) for such a goal, but that does mean it is not our priority to support such a use case (at least for diesel 1.0, maybe also for 2.0). And it certainly means that there is currently no easy documented way to do this. Adding to that you are trying to use the master branch of diesel + some the unreleased diesel_dynamic_schema crate. While we are happy for getting reports regarding to specific problems for our crates, at least I do not have the motivation/capacity to provide in depth support for any version and any use case of those crates for free. You can surely use them, but don’t expect that any detail is documented in depth or that someone will sit down and make your use case working. From someone trying to use a development version of something I expect at least the following:

You don’t need a complete guide, that basically describes how to implement your use case
You are willing to contribute something (like documentation, bug fixes, …) back to upstream

That said I can clearly see that there can be reasons to support such a behavior. While not all is perfect I think we are getting slowly there with diesel_dynamic_schema, but as always my day has only 24 hours, I cannot implement all at once and I prioritize implementing features myself that I need for my projects, not features that someone else needs. The fastest way to make sure diesel has a good story around really supporting dynamic schemas is to step up yourself and start working on this feature.
(Hopefully that all does not sound to harsh, I just don’t have the capacity to figure out all trait bounds errors of generic diesel code for everyone.)

So after that coming back to your error messages: They are not really connected to your dynamic schema use case, but more connected to that your are trying to abstract over different column types. The same thing would happen with a statically at compile time known schema. As written in the last sentence: Diesel’s dsl is designed to check types in your query at compile time, this implies you need to provide all information for the compile that are requested. That’s not the case here. The problem here is that DynExpr::SqlType will now depend (for the reasons already outlined) whether V::SqlType is nullable or not. You somehow need to in cooperate that in your trait bounds or find an other way to write to write that functionality. I personally would try to go for a variant involving much less generic types, so that you implement most methods for concrete sql types.

It would be great if SQL was that easy. Grab some SQL command line and check what are the return values of the following queries:

SELECT true = true
SELECT true = NULL
SELECT NULL = true
SELECT NULL = NULL
SELECT false = NULL
SELECT true AND NULL
SELECT true OR NULL

Razican · September 28, 2020, 12:38pm

Thank you very much for the detailed explanation. I will try to implement some of the things you mention, and see if I can contribute back somehow to make these things a bit easier.

Topic		Replies	Views
Dynamic filtering of a query in a dynamic schema	3	978	January 30, 2020
Selecting dynamic values w/o `FROM` for later insertion	2	495	November 18, 2018
Best practice for dynamic filtering	3	650	August 11, 2020
String to Column	6	1220	July 18, 2020
Diesel error when using a subquery: expected struct `diesel::query_source::MoreThanOnce`, found struct `diesel::query_source::Once`	2	1976	July 29, 2019

Trying to make dynamic filtering work on Diesel 2.0

Related topics