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

Mapping of int8 and int16 to SQL Server types is not optimal #119

Open
Yunaido opened this issue Nov 10, 2023 · 0 comments
Open

Mapping of int8 and int16 to SQL Server types is not optimal #119

Yunaido opened this issue Nov 10, 2023 · 0 comments

Comments

@Yunaido
Copy link

Yunaido commented Nov 10, 2023

Hello,

I've noticed an issue with the way GORM maps Go's int8 and int16 types to SQL Server types. In the current implementation, int8 is mapped to smallint and int16 is mapped to int.

sqlserver/sqlserver.go

Lines 188 to 202 in b8d91cb

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

This mapping seems odd because int8 in Go is an 8-bit integer, and SQL Server has a matching tinyint type which is also 8 bits. Mapping int8 to smallint (which is 16 bits) seems unnecessary.

Similarly, int16 in Go is a 16-bit integer, but it's being mapped to int in SQL Server, which is a 32-bit integer. SQL Server's smallint would be a better match for int16 because it's also 16 bits.

I propose that the mapping should be changed as follows:

int8 in Go should map to tinyint in SQL Server
int16 in Go should map to smallint in SQL Server

switch {
case field.Size <= 8:
  sqlType = "tinyint"
case field.Size <= 16:
  sqlType = "smallint"
case field.Size <= 32:
  sqlType = "int"
default:
  sqlType = "bigint"
}

This would make the type mapping more intuitive and efficient, and it would prevent unnecessary widening of the integer types.

Please let me know if you need any additional information about this issue.

Thank you for your consideration.

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 a pull request may close this issue.

1 participant