Remove impl TryGetable for Option<T>

[nappa85]

This PR continues what's I've begun with SeaQL/sea-query#112 and discussed in #107

This time I'm not really sure this is the best solution, but I haven't been able to come up with something better.

There are several concurring situations that made the problem worse:

• the sqlx error is converted to String, doing string matching here would be a waste of resources and could fail with any little changes from sqlx
• there are different impl of the trait, based on features and what's supported by single backends

What I've done:

• added an intermediate error type only for TryGetable trait (breaking change), the error type can be converted into the old error type via try operator but distincts Null situations from other errors
• every TryGetable impl try to fetch the Option variant and then, if None, convert it into the Null variant

Pros:

• custom impl of optional TryGetable implementors are now available

Con:

• API change and new error type

Let me know what do you think about it

[nappa85]

Another solution that saves the APIs could be adding another Trait, like TryGetableInner with actual impl and custom error, and

impl<T: TryGetableInner> TryGetable for  T {
    fn try_get(res: &QueryResult, pre: &str, col: &str) -> Result<Self, DbErr> {
        Ok(<T as TryGetableInner>::try_get(res, pre, col)?)
    }
}

[tqwewe]

Thanks for this PR!
I just get a clippy warning saying this should be opt.ok_or(TryGetError::Null) instead of the closure.
https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations

[nappa85]

Yes, you're right, I've run clippy but I forgot to play with features to ensure every code path was covered

[tqwewe]

Yeah it can be easy to forget and difficult to test sometimes.. I'm still trying to figure out a good way to get past this.. for now I just change the default features in Cargo.toml temporarily

[nappa85]

On some projects I ended up writing a bash script that launch several cargo test with all feature combinations, but it becomes quickly unmaintenable if you change the features on version bumps

[tyt2y3]

So we rely heavily on GitHub actions to remind us the 'buildability' of essential feature combinations.
We should add more to SeaORM too.
https://github.com/SeaQL/sea-query/blob/cfae4eab32fcd1a87103cffcf8760315f404656f/.github/workflows/rust.yml#L35-L38

[tyt2y3]

Thank you. I am okay with breaking API, as we are still early. Saying so, we should release it as early as possible, perhaps in 0.2.0.
So does this PR depends on the PR on SeaQuery? I should have merged that first right?

[nappa85]

Thank you. I am okay with breaking API, as we are still early. Saying so, we should release it as early as possible, perhaps in 0.2.0.
So does this PR depends on the PR on SeaQuery? I should have merged that first right?

They should be independent, this one doesn't use Nullable trait introduced with the sea-query PR

[tyt2y3]

0.1.3