Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify partial model queries #1597

Merged
merged 29 commits into from Apr 26, 2023

Conversation

Goodjooy
Copy link
Contributor

@Goodjooy Goodjooy commented Apr 13, 2023

PR Info

New Features

  • traits
    • SelectColumns : A trait can only set columns
    • PartialModelTrait: A trait for partial model to select specific columns
  • functions
    • Select::into_partial_model
    • SelectTwo::into_partial_model
    • SelectTwoMany::into_partial_model
      • SelectTwoMany::stream_partial_model
      • SelectTwoMany::all_partial_model need consolidate result , consolidate result need primary key. Cannot assume partial model has the pk.
  • macro
    • DerivePartialModel automatic generate implement of PartialModelTrait
      • macro attrs
        • from_col
        • from_expr
        • flatten : can contain other PartialModelTrait, which will be a good solution on manual handle join
      • Support generic arguments

Copy link
Member

@tyt2y3 tyt2y3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the contribution.
It looks really good!
SelectPartialModel is exactly what I have in mind.

src/query/helper.rs Outdated Show resolved Hide resolved
@Goodjooy Goodjooy requested a review from tyt2y3 April 16, 2023 15:53
@tyt2y3
Copy link
Member

tyt2y3 commented Apr 17, 2023

need consolidate result , consolidate result need primary key. Cannot assume partial model has the pk.

You are right. That's the reason why into_model is private, I reckon.

@tyt2y3 tyt2y3 requested a review from billy1624 April 17, 2023 09:18
@tyt2y3
Copy link
Member

tyt2y3 commented Apr 17, 2023

@billy1624 please review as well

@Goodjooy
Copy link
Contributor Author

need consolidate result , consolidate result need primary key. Cannot assume partial model has the pk.

You are right. That's the reason why into_model is private, I reckon.

I have an a bit complex idea.

If a model have 2 primary key

struct Model{
    #[sea_orm(primary_key)]
    id1: i32,
    #[sea_orm(primary_key)]
    id2: i32,
    ...
}

then, might the derive macro can generate the following PartialModel

#[derive(DerivePartialModel,  FromQueryResult)]
#[sea_orm(entity = "Entity")]
struct PrimaryPartial{
    id1:i32,
    id2:i32
}

finally, I can use a PartialModel like following, the pk will be the model's primary key partial model, and the inner will be the partial model user want. We can using pk to consolidate result

#[derive(FromQueryResult, DerivePartialModel)]
struct WithPk<E: EntityTrait, P: PartialModelTrait> {
    #[sea_orm(flatten)]
    pk: <<E as EntityTrait>::PrimaryKey as PrimaryKeyTrait>::PartialModel,
    #[sea_orm(flatten)]
    inner: P,
}

However, those code cannot using in current sea-orm, because neither FromQueryResult nor DerivePartialModel not support flatten and DerivePartialModel not support any generic argument.

@Goodjooy
Copy link
Contributor Author

DerivePartialModel now support nest. It look like that

    #[derive(FromQueryResult, DerivePartialModel)]
    #[sea_orm(entity = "Entity")]
    struct TimeRecord {
        _access: DateTimeLocal,
        _modify: DateTimeLocal,
        _delete: Option<DateTimeLocal>,
    }

    #[derive(FromQueryResult, DerivePartialModel)]
    #[sea_orm(entity = "Entity")]
    struct Bitmap {
        _bitmap1: u64,
        _bitmap2: u64,
        _bitmap3: u64,
        _bitmap4: u64,
    }
    #[derive(DerivePartialModel)]

    struct NestPartialModel {
        #[sea_orm(flatten)]
        _record: TimeRecord,
        #[sea_orm(flatten)]
        _bitmap: Bitmap,
    }
    // manual implement
    impl FromQueryResult for NestPartialModel {
        fn from_query_result(
            res: &sea_orm::QueryResult,
            pre: &str,
        ) -> Result<Self, sea_orm::DbErr> {
            Ok(Self {
                _record: TimeRecord::from_query_result(res, pre)?,
                _bitmap: Bitmap::from_query_result(res, pre)?,
            })
        }
    }

It generate SQL like following(MySQL)

SELECT 
`foo_table`.`access`, `foo_table`.`modify`, `foo_table`.`delete`, 
`foo_table`.`bitmap1`, `foo_table`.`bitmap2`, `foo_table`.`bitmap3`, `foo_table`.`bitmap4` 
FROM `foo_table`

@Goodjooy Goodjooy requested a review from tyt2y3 April 17, 2023 10:55
@Goodjooy Goodjooy marked this pull request as ready for review April 17, 2023 10:59
@Goodjooy Goodjooy marked this pull request as draft April 18, 2023 09:08
@Goodjooy
Copy link
Contributor Author

Goodjooy commented Apr 18, 2023

I think it is a good idea to support nest and generic argument for DerivePartialModel .

@tyt2y3
Copy link
Member

tyt2y3 commented Apr 18, 2023

Just being curious, why do you have underscore in front of all attributes?

@Goodjooy
Copy link
Contributor Author

Just being curious, why do you have underscore in front of all attributes?

The code snippet is in derive_test, without it will result warning.

@SeaQL SeaQL deleted a comment from Goodjooy Apr 18, 2023
@tyt2y3
Copy link
Member

tyt2y3 commented Apr 19, 2023

Yeah flatten makes sense. However, out of the scope of this PR, this is useful only if FromQueryResult supports flatten as well. So I would say let's merge everything up to 75fbe44, and add flatten in another PR. We will then tackle both FromQueryResult and DerivePartialModel there.

i.e. decoupling them would make them easier to review individually

@Goodjooy Goodjooy force-pushed the simplify_partial_model_queries branch from 018a2e8 to 75fbe44 Compare April 19, 2023 11:23
@Goodjooy
Copy link
Contributor Author

Yeah flatten makes sense. However, out of the scope of this PR, this is useful only if FromQueryResult supports flatten as well. So I would say let's merge everything up to 75fbe44, and add flatten in another PR. We will then tackle both FromQueryResult and DerivePartialModel there.

i.e. decoupling them would make them easier to review individually

Aright, the flatten part and generic support part has been removed. I will open another PR to add flatten support on DerivePartialModel and FromQueryResult

@Goodjooy Goodjooy marked this pull request as ready for review April 19, 2023 11:51
src/entity/prelude.rs Outdated Show resolved Hide resolved
Copy link
Member

@billy1624 billy1624 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just added stream_partial_model to Select<E> and SelectTwo<E,F> 3b09860

Copy link
Member

@billy1624 billy1624 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Goodjooy,

I also added into_partial_model to Cursor<S> bd1e989

And, some test cases b68f9de

tests/cursor_tests.rs Outdated Show resolved Hide resolved
@tyt2y3 tyt2y3 changed the base branch from master to pulls/1597 April 26, 2023 10:25
@tyt2y3 tyt2y3 merged commit 3341740 into SeaQL:pulls/1597 Apr 26, 2023
72 checks passed
darkmmon added a commit to darkmmon/seaql.github.io that referenced this pull request Jul 11, 2023
billy1624 added a commit to SeaQL/seaql.github.io that referenced this pull request Jul 19, 2023
* Optional Field SeaQL/sea-orm#1513

* .gitignore SeaQL/sea-orm#1334

* migration table custom name SeaQL/sea-orm#1511

* OR condition relation SeaQL/sea-orm#1433

* DerivePartialModel SeaQL/sea-orm#1597

* space for migration file naming SeaQL/sea-orm#1570

* composite key up to 12 SeaQL/sea-orm#1508

* seaography integration SeaQL/sea-orm#1599

* QuerySelect SimpleExpr SeaQL/sea-orm#1702

* sqlErr SeaQL/sea-orm#1707

* migration check SeaQL/sea-orm#1519

* postgres array SeaQL/sea-orm#1565

* param intoString SeaQL/sea-orm#1439

* **skipped** re-export SeaQL/sea-orm#1661

* ping SeaQL/sea-orm#1627

* on empty do nothing SeaQL/sea-orm#1708

* on conflict do nothing SeaQL/sea-orm#1712

* **skipped** upgrade versions

* active enum fail safe SeaQL/sea-orm#1374

* relation generation check SeaQL/sea-orm#1435

* entity generation bug SeaQL/sea-schema#105

* **skipped** bug fix that does not require edits

* EnumIter change SeaQL/sea-orm#1535

* completed and fixed a previous todo SeaQL/sea-orm#1570

* amended wordings and structures

* Edit

* Remove temp file

---------

Co-authored-by: Billy Chan <ccw.billy.123@gmail.com>
@github-actions
Copy link

github-actions bot commented Aug 2, 2023

🎉 Released In 0.12.1 🎉

Thank you everyone for the contribution!
This feature is now available in the latest release. Now is a good time to upgrade!
Your participation is what makes us unique; your adoption is what drives us forward.
You can support SeaQL 🌊 by starring our repos, sharing our libraries and becoming a sponsor ⭐.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants