SeaQL / seaql.github.io

📙 The official website of SeaQL
https://www.sea-ql.org/
Other
19 stars 44 forks source link

[SeaORM] migration guide for SeaQuery users #72

Open billy1624 opened 2 years ago

billy1624 commented 2 years ago

Proposed by https://github.com/SeaQL/sea-orm/issues/1227#issuecomment-1314947381

Content

I think there are two types of use cases.

Mapping query result into struct

This serve as a way to execute any SeaQuery statement. And the query result of select statement can be mapped into a struct in Rust.

Basically, as mentioned in this cookbook article: https://www.sea-ql.org/sea-orm-cookbook/003-run-sea-query-statement-on-sea-orm.html

Full SeaORM with access to alter the SeaQuery statement

Setup SeaORM entity with sea-orm-cli or defining it manually.

Then, You can get a mutable reference of the SeaQuery select statement from sea_orm::Select by QuerySelect::query method. With that you can construct complex query.

billy1624 commented 2 years ago

Ideas? @tyt2y3

tyt2y3 commented 2 years ago

I think the change in type system from SeaQuery to SeaORM is worth mentioning.

In SeaQuery, the type system is lower-level:

#[derive(Iden)]
enum Cake {
    Table,
    Id,
    Name,
}

Query::select().column(Cake::Name).from(Cake::Table).and_where(Expr::col(Cake::Id).eq(1))

In SeaORM, Entity (hence table name) is a separate type from columns:

#[derive(DeriveEntity, ..)]
#[sea_orm(table_name = "cake")]
pub struct Entity;

#[derive(DeriveActiveModel, ..)]
pub struct Model {
    pub id: i32,
    pub name: String,
}

Cake::find().filter(cake::Column::Id.eq(1))

Resulting in more concise code and a nicer experience.

billy1624 commented 1 year ago

Related discussion on Discord https://discord.com/channels/873880840487206962/900758302294704188/1045647323075710998

(Just a note to self)

nitnelave commented 1 year ago

Quoting from discord for posterity and people without an account:

if you're interested in seeing a sea-query -> sea-orm migration in the wild, here's the migration for LLDAP: https://github.com/nitnelave/lldap/pull/382 It contains a bit more than just the migration (while I was elbow deep in the code it was hard to avoid fixing up little things here and there, and now it's annoying to separate again), but it covers a good chunk of the API: Relations, Links, queries at the level of sea-orm, sea-query, raw SQL, implementing traits for better types in the DB (TryGetable, PrimaryKey, deriving FromQueryResult, ...), find_with_related, find_also_linked, in_subquery and so on. I've hit a couple of pain points here and there (the need for aliases when using find_also_linked, the lack of find_with_linked, the implicit order_by introduced by find_with_related, the fact that you can't derive FromQueryResult for structs with other structs as fields, and so on), but overall the migration went well. I am very glad that I had a pretty good test suite though, otherwise it would have been a disaster.

Some notes:

Finally, it would be nice if the entity generation would warn about column types that are not compatible with other DBs (I'm thinking of e.g. u8 with postgres).

tyt2y3 commented 1 year ago

Custom types require a quite verbose setup

https://github.com/SeaQL/sea-orm/issues/1349 tracking issue created

Deriving something extra on the column enum

I think we should simply add PartialEq, Eq derives to Column

I don't know if that's possible, but it would be nice to have the Column, ActiveModel and so on as inner types to the entity (at least as aliases)

I also wanted it (and tried many things) back then, and finally I believed that it cannot be done. https://github.com/rust-lang/rust/issues/8995 might be relevant

find_with_related will introduce a order_by on the primary key

This is due to how the aggregator currently works. It works by iterating through the rows without using a hashmap to resolve the association. If we change the implementation and uses a hashmap, we can drop this constraint.

However it is currently blocked by https://github.com/SeaQL/sea-orm/issues/1254

It would be really nice to have find_with_related ...

This PR already made it possible https://github.com/SeaQL/sea-orm/pull/1238 . At least it is very easy to write a wrapper function to construct the UsersAndGroups from Vec<User>, Vec<Vec<Group>>

Maybe automatically derive the required columns from the model

https://github.com/SeaQL/sea-orm/pull/1311 this might instead do what you want?

Maybe we need some extra documentations about what an inverse relation is and when to use it.

Well received

tyt2y3 commented 1 year ago

I think we should simply add PartialEq, Eq derives to Column

The actual reason we didn't add these is that it conflicts with ColumnTrait::eq which unfortunately will be a pain if we rename it to something else at this point

nitnelave commented 1 year ago

Thanks for the reply!

find_with_related will introduce a order_by on the primary key

This is due to how the aggregator currently works. It works by iterating through the rows without using a hashmap to resolve the association. If we change the implementation and uses a hashmap, we can drop this constraint.

I'm not necessarily in favor of switching to a hashmap (it sounds like it would probably be slower?) but it should at least be documented that it introduces an implicit order_by. Although it's true that a hashmap would make things easier on the API. Maybe expose both ways? e.g. add a find_with_related_ordered method or something like that, and replace the find_with_related with a hashmap.

https://github.com/SeaQL/sea-orm/pull/1311 this might instead do what you want?

This looks great :)

nitnelave commented 1 year ago

One small note on https://github.com/SeaQL/sea-orm/pull/1238: It's nice that it's possible, but it requires another round-trip to the DB, where the query could be done all at once.