Allow creating a sea_orm::Databse from sqlx::ConnectOptions

[05storm26]

Seems like doing what is requested in #398 is too difficult.

However allowing users to create a sea_orm::Database from sqlx::ConnectOptions implementations like MySqlConnectOptions seems to be doable.

For me this is good enough.

Also I have added a new feature options sqlx-any. If this is enabled the sqlx is any feature is turned on and you can create an sea_orm::Database from AnyConnectOptions as well.

~However this depends on this PR https://github.com/launchbadge/sqlx/pull/1610 (not yet merged!)~ MERGED and released

[05storm26]

@billy1624 Can you approve the run now?

[billy1624]

Sure!

[05storm26]

I think the rocket example that failed has beed fixed.

Also I have went through and added Hash, PartialEq, Clone derives on all types where possible, in the second commit.

[05storm26]

I'm curious about why we need to derive Hash?

I am not sure. Maybe someone needs it. If you want to use ConnectOptions in a hashmap as keys maybe. I don't know why you would do that just an idea.

I am generally on the side that we should implement for all types as much from the traits: Clone, Clone, PartialEq, Hash as possible.

[billy1624]

Will launchbadge/sqlx#1610 be in SQLx 0.6? And when will 0.6 lands?

[05storm26]

Due to changes requested by sqlx owner I had to make a small change in this pr as well.

[billy1624]

Oh, I wonder why the tests failed

[05storm26]

the trait std::hash::Hash is not implemented for sea_query::Value

Seems like maybe there was a sea_query change that removed Hash from Value. No idea why. I think it is useful to have these basic traits implemented like Debug, Clone, PartialEq, Hash for all possible types.

Currently there is two commits in this PR the actual change and also a second one which adds these useful trait derives on multiple types. Probably that is the one breaking.

Should I just remove that? Or should we fix this in sea_query?

[05storm26]

But I am still not sure what to do with the get_url method on DatabaseConnection, when the connection was not created from an url string but a sqlx::ConnectOptions....

[billy1624]

Should I just remove that? Or should we fix this in sea_query?

I think we can remove the Hash derive

[billy1624]

However allowing users to create a sea_orm::Database from sqlx::ConnectOptions implementations like MySqlConnectOptions seems to be doable.

Now that I re-think this. Shouldn't we just add a method to create DatabaseConnection from sqlx::ConnectOptions? Similar to from_sqlx_*_pool methods.

https://github.com/SeaQL/sea-orm/blob/483de17da885431749962bc8e3d807a020bdb1e7/src/driver/sqlx_mysql.rs#L66-L72

[05storm26]

@billy1624 I have rebased this on top of master. Please run the tests for it.

[05storm26]

Now that I re-think this. Shouldn't we just add a method to create DatabaseConnection from sqlx::ConnectOptions? Similar to from_sqlx_*_pool methods.

For me it looks like that should be constructed by calling Database::connect. So I change that in this PR to accept an sqlx::ConnectOptions as well.

[05storm26]

Is clippy supposed to run without warning? Because currently master HEAD has some clippy warnings:

$ cargo clippy --features "sqlx-all macros with-json with-chrono with-uuid sqlx-all runtime-async-std-native-tls"  -- -D warnings
    Checking sea-orm v0.10.3 (/home/adam/sea-orm)
error: redundant closure
   --> src/database/transaction.rs:278:25
    |
278 |             err.map_err(|e| sqlx_error_to_query_err(e))
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `sqlx_error_to_query_err`
    |
    = note: `-D clippy::redundant-closure` implied by `-D warnings`
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure

error: passing a unit value to a function
   --> src/driver/sqlx_mysql.rs:216:9
    |
216 |         Ok(self.pool.close().await)
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `-D clippy::unit-arg` implied by `-D warnings`
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unit_arg
help: move the expression in front of the call and replace it with the unit literal `()`
    |
216 ~         self.pool.close().await;
217 +         Ok(())
    |

error: passing a unit value to a function
   --> src/driver/sqlx_postgres.rs:231:9
    |
231 |         Ok(self.pool.close().await)
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unit_arg
help: move the expression in front of the call and replace it with the unit literal `()`
    |
231 ~         self.pool.close().await;
232 +         Ok(())
    |

error: passing a unit value to a function
   --> src/driver/sqlx_sqlite.rs:219:9
    |
219 |         Ok(self.pool.close().await)
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unit_arg
help: move the expression in front of the call and replace it with the unit literal `()`
    |
219 ~         self.pool.close().await;
220 +         Ok(())
    |

[05storm26]

I don't think this is ever going to be merged so I am just gonna let it go.

[billy1624]

Hey @05storm26, sorry and thanks again for the effort!