Explanation of existance of IntoSimpleExpr instead of Into<SimpleExpr>

[AndreiOrmanji]

Please explain why there is an additional IntoSimpleExpr trait instead of implementing Rust build-in Into trait

[Expurple]

I agree. There are many IntoFoo traits in sea_orm and in sea_query.

Sometimes, only IntoFoo or only Into<Foo> is implemented for a type, which is annoying and inconsistent.

Sometimes, both traits are implemented, but it's not clear which one should be used. E.g., IntoSimpleExpr is practically unused. Most upstream methods for working with expressions accept Into<SimpleExpr>:

sea-query$ rg --stats 'Into<SimpleExpr>' | grep -P '\d+ matches'
138 matches

sea-query$ rg --stats 'IntoSimpleExpr' | grep -P '\d+ matches'
0 matches

sea-orm$ rg --stats 'Into<SimpleExpr>' | grep -P '\d+ matches'
3 matches

sea-orm$ rg --stats 'IntoSimpleExpr' | grep -P '\d+ matches'
15 matches

If there's a good reason for these duplicated traits to exist, it should be documented on their definitions, along with expected use cases vs Into<Foo>. I can make a PR, if someone at least explains it here, on a high level.

[tyt2y3]

Thanks for the questions. In most cases the reason is due to orphan rule. SeaORM don't own the underlying types (which is unfortunate, but I like having a crate boundary between SeaQuery / SeaORM), and can't do blanket impl. In particular, IntoSimpleExpr is not just Into<SimpleExpr>, it has an additional impl for impl<C: ColumnTrait> IntoSimpleExpr for C.

May be in some cases, the way we currently use them doesn't violate the orphan rule.

[Expurple]

impl<C: ColumnTrait> IntoSimpleExpr for C

Supertraits can often achieve a "reuse" effect similar to blanket impls. In #2548, I already wrote that in SeaORM 2.0 we should have ColumnTrait: Into<SimpleExpr>, because that makes a lot of sense and allows to call ExprTrait methods on SeaORM columns, by reusing the existing blanket impl<T> ExprTrait for T where T: Into<SimpleExpr>.

May be in some cases, the way we currently use them doesn't violate the orphan rule.

As far as I understand, IntoSimpleExpr is the only Into* trait that matches your explanation about crate boundaries. You needed a blanket conversion into a sea_query type, but with a bound from sea_orm. So, due to your dependency graph, the impl had to be in sea_orm. And due to the orphan rules, you could only implement a trait from sea_orm. Ok, makes sense. (Although this could also be solved by a supertrait, as I described).

All the other Into* traits still don't make any sense to me, because the trait and the "target" type are both in the same crate (either both in sea_query or both in sea_orm). The crate can just implement From for the "target" type, because it owns the type! There are no problematic cross-crate bounds with all these other traits.

[tyt2y3]

I think your understanding is correct. in the past, our default answer to 'a function can accept values either A or B' is to have a IntoTrait that's implemented for A and B.

so we can remove those other Into traits

[Expurple]

Cool. Appended this to #2548 and SeaQL/sea-query#795. Consider creating a label/milestone for breaking changes. Marking the original discussions with that should be more convenient than someone manually maintaining a list of links. And should age better, as some items move to Done

[Expurple]

To keep some backwards-compatibility, we can make IntoFoo: Into<Foo> and blanket impl IntoFoo for T where T: Into<Foo>, instead of removing these traits outright.

However, I'd still replace all IntoFoo uses in our APIs with Into<Foo>. And we need to document on every IntoFoo that it's a legacy trait kept for compatibility. We can deprecate these sometime later in 2.x., to make the 2.0 upgrade less annoying.