Describe the feature
I would like to specify a column to be created when auto migration run only if the column does not yet exist in the table.
If the column already exist, then ignore the migration for the column.
Motivation
I need to create a virtual/ generated column in a postgres table, and I've manage to do that using:
... `gorm:"->;type:bool GENERATED ALWAYS AS (some_expression) STORED;default:(-);"`
It works when the column is not yet existed in the table. But when next automigration run and the column already existed in the table, it errors out because AFAIK altering a generated column is not supported in postgres.
Using the -:migration tags to ignore the column from migration works in this particular case:
... `gorm:"-:migration;->;type:bool GENERATED ALWAYS AS (some_expression) STORED;default:(-);"`
But if the column is somehow removed from the table (e.g. targeting fresh table/ database), the column won't be created on the next automigration run... :disappointed: The only way to create the column is by removing the -:migration tags...
aaand... we're back to square one... :shrug:
Related Issues
https://github.com/go-gorm/gorm/issues/4732 This comment is basically what this issue is all about
https://github.com/go-gorm/gorm/issues/3887
I actually share the same opinion with the OP on this one, instead of using workaround with the type, having the ability to differentiate between normal column and generated/ virtual column might be the better way going forward
Comment From: IronGeek
OK, so, I just found out:
if the generated column data type is string (varchar) AND if the column already exist in the table, the alter column does not happens on migration; but if the column data type is other than that (tested only with int or bool) the alter column happens on every migration regardless of whether the column already existed, and or changed in the table.
After digging some more, I think the issue is happening in the Migrator.MigrateColumn() function migrator.go#L385-L406:
func (m Migrator) MigrateColumn(value interface{}, field *schema.Field, columnType gorm.ColumnType) error {
// found, smart migrate
fullDataType := strings.ToLower(m.DB.Migrator().FullDataTypeOf(field).SQL)
realDataType := strings.ToLower(columnType.DatabaseTypeName())
alterColumn := false
// check size
if length, ok := columnType.Length(); length != int64(field.Size) {
if length > 0 && field.Size > 0 {
alterColumn = true
} else {
// has size in data type and not equal
// Since the following code is frequently called in the for loop, reg optimization is needed here
matches := regRealDataType.FindAllStringSubmatch(realDataType, -1)
matches2 := regFullDataType.FindAllStringSubmatch(fullDataType, -1)
if (len(matches) == 1 && matches[0][1] != fmt.Sprint(field.Size) || !field.PrimaryKey) &&
(len(matches2) == 1 && matches2[0][1] != fmt.Sprint(length) && ok) {
alterColumn = true
}
}
}
...
Here's my finding with the following tests:
- golang
...type:int GENERATED ALWAYS AS (42) STORED;default:(-);
| Variables | Value |
| --------- | ----- |
| fullDataType | "int generated always as (42) stored" |
| realDataType | "int4" |
| field.Size | 64 |
| columnType.Length() | 32, true |
| matches | |
| matches2 | |
| matches[0][1] | |
| matches[0][1] | |
-
golang ...type:bool GENERATED ALWAYS AS (md5(name) = hash) STORED;default:(-);| Variables | Value | | --------- | ----- | | fullDataType |"bool generated always as (md5(name) = hash) stored"| | realDataType |"bool"| | field.Size |0| | columnType.Length() |8, true| | matches |[][]string len: 0, cap: 0, nil| | matches2 |[][]string len: 1, cap: 10, [["bool generated always as (md5(","5"]]| | matches[0][1 |-| | matches[0][1 |5| -
golang ...type:varchar GENERATED ALWAYS AS (md5(name)) STORED;default:(-);| Variables | Value | | --------- | ----- | | fullDataType |"varchar generated always as (md5(name)) stored"| | realDataType |"varchar"| | field.Size |0| | columnType.Length() |0, false| | matches | | | matches2 | | | matches[0][1 | | | matches[0][1 | |
if the column type is string (varchar) the field.Size and length (columnType.Length()) always match, so the condition for line migrator.go#L393 is never satisfied.
...
if length, ok := columnType.Length(); length != int64(field.Size) {
...
On the contrary if the column type is int or bool the condition will always be true, unless user explicitly specified the column size with the size tags (e.g ...size:8;type:bool GENERATED ALWAYS AS (TRUE) STORED;default:(-);), but for int or bool data type this really shouldn't be necessary, ...right?
...
if length > 0 && field.Size > 0 {
alterColumn = true
}
...
IMO, the logic for field size comparison above is also probably the cause for this related issue mentioned in #4915.
The last problem lies in the regex matching in lines migrator.go#L399-L400 for capturing field sizes. The regex pattern [^\d](\d+)[^\d]? might be sufficient for normal column type, but since the type tags are now also used for generated/ virtual column, the regex will probably generate a lot of false positive results. As seen on the example above, for generated/ virtual column:
type:bool GENERATED ALWAYS AS (md5(name) = hash) STORED;default:(-);
would actualy try to match the pattern [^\d](\d+)[^\d]? with bool generated always as (md5(name) = hash) stored, which will mistakenly resulted in field size of 5 (from the md5 string).
Errm, should I file this finding in new issue? or keep it here?
Comment From: jinzhu
Hello @IronGeek
We have updated the Auto migrate logic a bit, could you confirm the issue still exists? or can you write a playground PR?
Thank you
Comment From: khalilsarwari
@jinzhu This issue still exists with the new logic as far as I can tell; I have just created a playground PR for it
Comment From: zakaria-chahboun
Yes it's happens on SQLite too #5590
Comment From: jhartman86
@jinzhu this is still relevant: gorm version: 1.24.5 gorm postgres driver: 1.4.8
cc @IronGeek thansk for digging to figure it out
Comment From: seriousm4x
This issue shouldn't be closed. I'm sill running into the same issue and the 2 referenced issues above are also still relevant.