Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

InsertModel and UpdateModel #101

Closed
tyt2y3 opened this issue Aug 22, 2021 · 6 comments
Closed

InsertModel and UpdateModel #101

tyt2y3 opened this issue Aug 22, 2021 · 6 comments

Comments

@tyt2y3
Copy link
Member

tyt2y3 commented Aug 22, 2021

Following up on #97 #94 #96

I have a rough idea:

  1. to annotate the attributes of Entity's Model, where each attribute has three possible 'requirements': auto_identity (auto generated primary key), required (implied and thus can be omitted) and optional

  2. from there, we can derive two more structs, InsertModel and UpdateModel
    For InsertModel, auto_identity will be omitted, and required fields will be wrapped with RequiredValue, which must be Set. Optional fields will be wrapped with ActiveValue, which can be Set or Unset.
    For UpdateModel, auto_identity will be wrapped with RequiredValue, while all other fields will be wrapped with ActiveValue (thus are optional)

  3. change the Insert and Update API such that they accept IntoInsertModel and IntoUpdateModel respectively. ActiveModel can still be converted automatically into InsertModel and UpdateModel, but will panic if the 'requirements' mismatch

@tyt2y3 tyt2y3 changed the title Attributes on ActiveModel Attributes on Entity Model Aug 22, 2021
@tqwewe
Copy link
Contributor

tqwewe commented Aug 24, 2021

I like what you've desribed here... looks promising.

I think this could be related to: #105 - perhaps a refactor to the API can be considered.

@tyt2y3
Copy link
Member Author

tyt2y3 commented Sep 6, 2021

After #105, I think it will provide us some compile-time info to work with, i.e. the default_value and default_expr tags.

As such, we'd be able to generate two extra models InsertModel, UpdateModel along with ActiveModel, under the following rules:
For insert model, skip the 'auto_increment primary_key' columns. Fields without default will be wrapped with RequiredValue, ActiveValue otherwise.
For update model, the 'primary_key' columns will be wrapped with RequiredValue while all other fields will be wrapped with ActiveValue

@tyt2y3 tyt2y3 added this to the 0.3.0 milestone Sep 7, 2021
@tyt2y3 tyt2y3 changed the title Attributes on Entity Model InsertModel and UpdateModel Sep 7, 2021
@billy1624
Copy link
Member

billy1624 commented Sep 7, 2021

Btw... why we cant set an value for primary key with auto_increment during insert?

@tqwewe
Copy link
Contributor

tqwewe commented Sep 7, 2021

It is possible to specify the primary key even if a primary key column is auto_increment, but I can't imagine many situations where that'd be useful to anyone.

I'm also thinking that the update and insert models can be used with async-graphql under a feature flag in the future, and you typically wouldn't allow the primary key to be set when inserting a new item. But for the async-graphql case, it could just add #[graphql(skip)] to the primary key on the insert model if it were to be kept.

@nicoulaj
Copy link

nicoulaj commented Sep 8, 2021

Please also note all GENERATED ALWAYS columns should be excluded from insert/update queries or use DEFAULT as value. See for example Postgres or MySQL.

A generated column cannot be written to directly. In INSERT or UPDATE commands, a value cannot be specified for a generated column, but the keyword DEFAULT may be specified.

Maybe those could be directly excluded for InsertModel/UpdateModel ? In ActiveModel, maybe ActiveValue could be split into ReadableActiveValue + WritableActiveValue for these columns ?

@tyt2y3
Copy link
Member Author

tyt2y3 commented Sep 8, 2021

Thanks @nicoulaj I think you raised a good point not yet considered.

@billy1624 billy1624 removed this from the 0.4.x - Enumeration milestone Nov 25, 2021
@SeaQL SeaQL locked and limited conversation to collaborators May 15, 2022
@tyt2y3 tyt2y3 converted this issue into discussion #729 May 15, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants