Gorm Question: Correct column type usage in migrations

Your Question

Hi, i'd like to open a pull request to improve the migration process, focusing on the AlterColumn method and its sqlserver's implementation, but before i start, i'd like to have this question answered.

Quoting https://gorm.io/docs/data_types.html:

GORM will read column’s database type from tag type, if not found, will check if the struct > implemented interface GormDBDataTypeInterface or GormDataTypeInterface and will use its result as data type

But this doesn't seem to apply to the migrations as in the methods FullDataTypeOf and DataTypeOf the TagSettings["TYPE"] and the GormDataType method seem to be ignored, while the GormDBDataType method is handled.

Is this intended? Or should the TagSettings["TYPE"] and the GormDataType method be added in the DataTypeOf mehod?

I'd expect the following logic:

if val, ok := field.TagSettings["TYPE"]; ok {
     // Use the tag
} else if dataTyper, ok := fieldValue.Interface().(GormDataTypeInterface); ok {
    // Use the GormDataType interface
} else if dbDataTyper, ok := fieldValue.Interface().(GormDBDataTypeInterface); ok {
    // Use the GormDBDataType interface
}

The document you expected this should be explained

It is already explained here, but the implementation seems incomplete.

Expected answer

Should the logic regarding the migration be updated or should the documentation be changed? Has the gorm:"type:..." tag been deprecated?

Thank you for your time.

Comment From: a631807682

Should the logic regarding the migration be updated or should the documentation be changed?

Documentation should be changed, GormDBDataTypeInterface does not exist anymore, only schema.GormDataTypeInterface and migrator.GormDataTypeInterface exists, and will use migrator.GormDataTypeInterface first.

Has the gorm:"type:..." tag been deprecated?

field.DataType and TagSettings["TYPE"] are the same.

https://github.com/go-gorm/gorm/blob/master/schema/field.go#L274 https://github.com/go-gorm/gorm/blob/master/schema/field.go#L306

So the converted logic is:

if dataTyper, ok := fieldValue.Interface().(migrator.GormDataTypeInterface); ok {
...
} else if val, ok := field.TagSettings["TYPE"]; ok {
...
} else if dbDataTyper, ok := fieldValue.Interface().(schema.GormDBDataTypeInterface); ok {
...
}

Comment From: Fulmineo64

Hi, thank you for your answer.

My objective is here is to utilize properly the type tag in the column as this

type Test struct {
    TEST_FIELD int `gorm:"type:int"`
}

gets created as this

TABLE Test (
    TEST_FIELD bigint NULL
)

Which i would expect to be an int as well in the table, NOT a bigint.

So is it correct that FullDataTypeOf and DataTypeOf only check for the GormDBDataType method (defined in migrator.GormDataTypeInterface)?

// CURRENT VERSION
// DataTypeOf return field's db data type
func (m Migrator) DataTypeOf(field *schema.Field) string {
    fieldValue := reflect.New(field.IndirectFieldType)
    if dataTyper, ok := fieldValue.Interface().(GormDataTypeInterface); ok {
        if dataType := dataTyper.GormDBDataType(m.DB, field); dataType != "" {
            return dataType
        }
    }

    return m.Dialector.DataTypeOf(field)
}

Or should it be more like this?

// UPDATED VERSION
// DataTypeOf return field's db data type
func (m Migrator) DataTypeOf(field *schema.Field) string {
    fieldValue := reflect.New(field.IndirectFieldType)
    if dataTyper, ok := fieldValue.Interface().(GormDataTypeInterface); ok {
         if dataType := dataTyper.GormDBDataType(m.DB, field); dataType != "" {
         return dataType
             }
    } else if dataTyper, ok := fieldValue.Interface().(schema.GormDataTypeInterface); ok {
           if dataType := dataTyper.GormDataType(); dataType != "" {
                return dataType
        }
        } else if val, ok := field.TagSettings["TYPE"]; ok {
              return val
        } 

    return m.Dialector.DataTypeOf(field)
}

Thank you!

Comment From: a631807682

So is it correct that FullDataTypeOf and DataTypeOf only check for the GormDBDataType method (defined in migrator.GormDataTypeInterface)?

Already checked in m.Dialector.DataTypeOf(field) by field.DataType, field.DataType is set by GormDataTypeInterface and field.TagSettings["TYPE"]

Comment From: Fulmineo64

Oh I see, so it is sqlserver's Dialector that overrides my "int" field type, here:

func (dialector Dialector) DataTypeOf(field *schema.Field) string {
    switch field.DataType {
    case schema.Bool:
        return "bit"
    case schema.Int, schema.Uint:
        var sqlType string
        switch {
        case field.Size < 16:
            sqlType = "smallint"
        case field.Size < 31:
            sqlType = "int"
        default:
            sqlType = "bigint"
        }

        if field.AutoIncrement {
            return sqlType + " IDENTITY(1,1)"
        }
        return sqlType
    /* Other cases omitted for brevity */
    }

    return string(field.DataType)
}

The above case overrides my specified "int" type to "bigint", because the keyword "int" matches with the const schema.Int, so it gets treated as if it wasn't specified.

So I shouldn't edit gorm's DataTypeOf, instead i should add an extra check to sqlserver's migrator to NOT override my "int" field.

Something like this:

// UPDATED version
func (dialector Dialector) DataTypeOf(field *schema.Field) string {
    switch field.DataType {
    case schema.Bool:
        return "bit"
    case schema.Int, schema.Uint:
        // Prevents the "int" type to be overridden when specified in the type tag
                if val, ok := field.TagSettings["TYPE"]; ok && val == "int" {
            return string(field.DataType)
        }
        var sqlType string
        switch {
        case field.Size < 16:
            sqlType = "smallint"
        case field.Size < 31:
            sqlType = "int"
        default:
            sqlType = "bigint"
        }

        if field.AutoIncrement {
            return sqlType + " IDENTITY(1,1)"
        }
        return sqlType
        /* Other cases omitted for brevity */
    }

    return string(field.DataType)
}

Thank you for the hint!

Comment From: Fulmineo64

I've created the following pull request for the sqlserver driver: https://github.com/go-gorm/sqlserver/pull/86