add some multirange operations support

[guissalustiano]

Add multirange contains support under #4240

It is a small test to check the format before moving everything else

[guissalustiano]

Hi @weiznich, sorry for keeping this PR open for so long. I would like to use select to write some tests. It's simpler to write instead create a table, insert table and write some filters. But I'm having some problems with how to use the range extension with literal value, the compiler doesn't know how to use the proper trait.

I want to write something like https://github.com/diesel-rs/diesel/pull/4261/commits/063297d7e33aaade96c17da397919e123d623115

[weiznich]

No worries for taking some time on this.

I would like to use select to write some tests. It's simpler to write instead create a table, insert table and write some filters. But I'm having some problems with how to use the range extension with literal value, the compiler doesn't know how to use the proper trait.

It's a good idea to skip the table setup as that should make the tests easier. As for the problem: I think something like this should work:

diesel::select((1..5).into_sql::<Range<Integer>>().contains(4)).load::<bool>(conn).unwrap();

The into_sql() call explicitly converts the rust side value into a SQL side expression (bind in this case), which then allows the DSL to work as expected.

[guissalustiano]

Hi @weiznich, thanks for the patient. I think it adds a test to all the new behavior. With this PR we can use the operations between Range and Range, or Multirange and Multirange. I would like to also support operation between Multirange and Range (and vice versa), for that, I would need to change the T here to receive RangeOrMultirange, but I don't know how to do that. Could you help-me with that? (We can merge this PR and an open another one for that) https://github.com/diesel-rs/diesel/blob/381be195688db339fe2927e49bc818ab86754dd9/diesel/src/pg/expression/expression_methods.rs#L950-L956

[weiznich]

I would like to also support operation between Multirange and Range (and vice versa), for that, I would need to change the T here to receive RangeOrMultirange, but I don't know how to do that. Could you help-me with that? (We can merge this PR and an open another one for that)

 fn contains_range<T>(self, other: T) -> dsl::ContainsRange<Self, T> 
where 
    Self::SqlType: SqlType, 
    T: AsExpression<Self::SqlType>, 

Technically this should be possible by introducing another generic parameter on the function signature above. The AsExpression<Self::SqlType> part essentially says: Give me something that can by turned in an SQL expression with the same SQL type (Range/Multirange) than the left hand side. We could now have a Rhs generic type there that just says, we expect some T that can be turned into the Rhs type, and expect that Rhs type to be either Range or MultiRange. Now the problem with that approach is that I'm not sure if that plays well with type inference. I would expect that the compiler than cannot infer the right type of Rhs on it's own, which would require that users always need to annotate these calls, which is not that great. I'm not sure on the last point, as I've not tried it yet, so it might be that rustc is clever enough to see that there is often just one type that fulfills this and does not require explict type annotations in that case.

[guissalustiano]

I would like to also support operation between Multirange and Range (and vice versa), for that, I would need to change the T here to receive RangeOrMultirange, but I don't know how to do that. Could you help-me with that? (We can merge this PR and an open another one for that)

 fn contains_range<T>(self, other: T) -> dsl::ContainsRange<Self, T> 
where 
    Self::SqlType: SqlType, 
    T: AsExpression<Self::SqlType>, 

Technically this should be possible by introducing another generic parameter on the function signature above. The AsExpression<Self::SqlType> part essentially says: Give me something that can by turned in an SQL expression with the same SQL type (Range/Multirange) than the left hand side. We could now have a Rhs generic type there that just says, we expect some T that can be turned into the Rhs type, and expect that Rhs type to be either Range or MultiRange. Now the problem with that approach is that I'm not sure if that plays well with type inference. I would expect that the compiler than cannot infer the right type of Rhs on it's own, which would require that users always need to annotate these calls, which is not that great. I'm not sure on the last point, as I've not tried it yet, so it might be that rustc is clever enough to see that there is often just one type that fulfills this and does not require explict type annotations in that case.

Thanks so much for the explanation. Can we merge this PR? I'll open another PR to explore this approach.

[weiznich]

Can we merge this PR?

I will do that as soon as I fixed the CI. That turns out to be more problematic this time for whatever reason :( Follow https://github.com/diesel-rs/diesel/pull/4238 for updates.

[guissalustiano]

Oh thanks!