Impl Queryable, AsExpression, From/ToSql for ranges from std

[0nkery]

Hi, I implemented Queryable, Expression, FromSql, ToSql traits for Range* types from std.

It would be awesome to have these types when working with various intervals in Diesel.

[0nkery]

RangeInclusive and RangeToInclusive seem to fail in the tests because the inferred Bound is always Excluded.

[weiznich]

From a short look over the code: It seems like the FromSql implementations could possibly fail if the range does not have the right format. Our general policy for FromSql implementations included in diesel core is that they should basically never fail.

That means adding the ToSql implementations and related stuff is fine, but adding FromSql is against that policy.

cc @diesel-rs/core

[0nkery]

Seems like it's good to have precise Rust type to represent ranges with specific bounds. But I could remove some (redundant maybe) types like RangeInclusive and RangeToInclusive.

If I leave out FromSql these types cannot be Queryable, I guess?

[weiznich]

Seems like it's good to have precise Rust type to represent ranges with specific bounds. But I could remove some (redundant maybe) types like RangeInclusive and RangeToInclusive.

Having those is additional types is no problem in my opinion.

If I leave out FromSql these types cannot be Queryable, I guess?

Right, not implementing FromSql implies that Queryable is not supported.

[0nkery]

That means adding the ToSql implementations and related stuff is fine, but adding FromSql is against that policy.

I see these types as kind of validation. What is validation policy? Maybe FromSql-like impls (maybe for completely separate trait) should be added to another crate?

[weiznich]

I see these types as kind of validation. What is validation policy? Maybe FromSql-like impls (maybe for completely separate trait) should be added to another crate?

The policy contains 2 rules:

• It's a commonly used type
• The conversion cannot fail for all possible values of that type.

The second rule is not fulfilled because postgres range could contain values that are not valid for a specific Range* type in rust. (For example [0,1] for Range<i32>)

I do not think that a FromSql like trait would be useable in this context without also extending the Connection trait.

[c410-f3r]

Any news?

[weiznich]

@c410-f3r I do not see any updates here so why ask this question?

[c410-f3r]

It is worth to ask for updates after fifteen months of inactivity. Perhaps it will bring light to this PR, drawing people's attention; perhaps it will serve as a motivation for the author to finish his work; perhaps I will try to fix the commented problems as this feature is in my interest.

Discouraging participation may lead to stagnation and stalled PR's. Nevertheless, I can happily remove my question if it is indeed not desired.

[weiznich]

@c410-f3r It's much about the wording here. I consider just asking Any news? on an old stale thread as a question similar to "Why is this not fixed yet?". You really should consider that this project is run by volunteers in their free time. A better way would be to just ask the OP if they plan to continue on their work here and communicate that you would be interested in doing that on your own if not.

From a technical point of view: My concerns raised here are remaining valid and are not answered yet.