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

Fix DataType Define #111

Closed
wants to merge 3 commits into from
Closed

Fix DataType Define #111

wants to merge 3 commits into from

Conversation

piyongcai
Copy link

  • Do only one thing
  • Non breaking API changes
  • Tested(use original test code)

What did this pull request do?

User Case Description

  1. because select schema from db, return int2,int4,int8. but code name is smallint,integer,bigint. it will cause alter column.
    a. smallint -> int2
    b. integer -> int4
    c. bigint -> int8
  2. timestamptz set size to 0,because parse field get size is 0, not 64, it will cause alter column.

piyongcai added 2 commits May 26, 2022 10:56
1. because select schema from db, return int2,int4,int8. but code name is smallint,integer,bigint. it will cause alter column.
 a. smallint -> int2
 b. integer -> int4
 c. bigint -> int8
2. in postgresql, set timestamptz Precision == 0, This means being accurate to the seconds.
return fmt.Sprintf("timestamptz(%d)", field.Precision)
}
return "timestamptz"
// in postgresql, Precision == 0, This means being accurate to the seconds.
Copy link
Member

Choose a reason for hiding this comment

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

the default value of precision is 0, that why we ignored the zero value.

Copy link
Author

Choose a reason for hiding this comment

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

the default value of precision is 0, that why we ignored the zero value.

if set default value, it will set type = "timestamptz", it same as "timestamptz(6)".

in current logic, no way to define type "timestamptz(0)"

Copy link
Member

Choose a reason for hiding this comment

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

but this changed the default behavior and removed the default precision?

Maybe we need to check field's tag by using field.TagSettings["PRECISION"] if the precision's value is 0?

Copy link
Member

Choose a reason for hiding this comment

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

The change here doesn't seem to make sense, because there was a problem here, It only works if the user doesn't set type.
field.DataType is set by user, when we using gorm:"type:"timestamptz", it's not equal schema.Time, and when we using gorm:"type:"time", it's means time of day (no date).

https://www.postgresql.org/docs/14/datatype-datetime.html

Edit

MyTime time.Time `gorm:"precision:6"`

The reason why this triggers alter is because we mistakenly mistook precision for size
https://github.com/go-gorm/gorm/blob/master/migrator/migrator.go#L423

@quexer
Copy link

quexer commented Jul 16, 2022

Get stuck with this issue. Looking forward to this can be merged and released soon. @a631807682 @jinzhu
Thanks!

@jinzhu
Copy link
Member

jinzhu commented Oct 7, 2022

Fixed by commit #133

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

4 participants