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

Distinguish custom types #6083

Closed
wants to merge 4 commits into from

Conversation

black-06
Copy link
Contributor

  • Do only one thing
  • Non breaking API changes
  • Tested

What did this pull request do?

Distinguish between detected and custom type (#6033).
In driver, we will skip to getSchemaCustomType if CustomDataType is specified.
See MySQL#109 for more context.

User Case Description

type Test struct {
    Date time.Time `gorm:"type:date"`
    Time time.Time `gorm:"type:time"`
    DefaultDateTime time.Time
}
CREATE TABLE `tests` (`date` date,`time` time,`default_date_time` datetime)

schema/field.go Outdated Show resolved Hide resolved
@jinzhu
Copy link
Member

jinzhu commented Mar 10, 2023

Currently, seems like the field.IsTaggedType field is equal to checking the existing field.TagSettings["TYPE"]?

If so, it doesn't really make sense to have it?

@a631807682
Copy link
Member

a631807682 commented Mar 10, 2023

Currently, seems like the field.IsTaggedType field is equal to checking the existing field.TagSettings["TYPE"]?

If so, it doesn't really make sense to have it?

This is the same thing. I hope to describe all the field information in parse. It is not easy to understand the field.TagSettings["type"] and field.DataTyp in the driver, we need to see the gorm code to know why we do this.

@jinzhu
Copy link
Member

jinzhu commented Mar 13, 2023

Currently, seems like the field.IsTaggedType field is equal to checking the existing field.TagSettings["TYPE"]?
If so, it doesn't really make sense to have it?

This is the same thing. I hope to describe all the field information in parse. It is not easy to understand the field.TagSettings["type"] and field.DataTyp in the driver, we need to see the gorm code to know why we do this.

The Schema and Field was thought of as low-level API, users need to know the details to use them.

But I understand your concerns, maybe we can define some development rules, and have a field to describe those information if it is used for two or three different purposes to avoid API overbloat?

@a631807682
Copy link
Member

Currently, seems like the field.IsTaggedType field is equal to checking the existing field.TagSettings["TYPE"]?
If so, it doesn't really make sense to have it?

This is the same thing. I hope to describe all the field information in parse. It is not easy to understand the field.TagSettings["type"] and field.DataTyp in the driver, we need to see the gorm code to know why we do this.

The Schema and Field was thought of as low-level API, users need to know the details to use them.

But I understand your concerns, maybe we can define some development rules, and have a field to describe those information if it is used for two or three different purposes to avoid API overbloat?

I agree

@a631807682 a631807682 mentioned this pull request Mar 13, 2023
3 tasks
@black-06
Copy link
Contributor Author

Currently, seems like the field.IsTaggedType field is equal to checking the existing field.TagSettings["TYPE"]?
If so, it doesn't really make sense to have it?

This is the same thing. I hope to describe all the field information in parse. It is not easy to understand the field.TagSettings["type"] and field.DataTyp in the driver, we need to see the gorm code to know why we do this.

The Schema and Field was thought of as low-level API, users need to know the details to use them.
But I understand your concerns, maybe we can define some development rules, and have a field to describe those information if it is used for two or three different purposes to avoid API overbloat?

I agree

Close it?

@jinzhu jinzhu closed this Mar 14, 2023
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.

3 participants