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

Update from 1.7.0 to 1.7.1: unknown type for gorm.DeletedAt #194

Closed
joao-intelliway opened this issue May 21, 2024 · 6 comments · Fixed by #195
Closed

Update from 1.7.0 to 1.7.1: unknown type for gorm.DeletedAt #194

joao-intelliway opened this issue May 21, 2024 · 6 comments · Fixed by #195

Comments

@joao-intelliway
Copy link

Describe the bug
After updating from 1.7.0 to 1.7.1 it stopped working.

❯ go run .

2024/05/21 14:29:53 /home/user/test/main.go:23 SLOW SQL >= 200ms
[1182.851ms] [rows:-] SELECT c.COLUMN_NAME, t.CONSTRAINT_TYPE FROM INFORMATION_SCHEMA.TABLE_CONSTRAINTS t JOIN INFORMATION_SCHEMA.CONSTRAINT_COLUMN_USAGE c ON c.CONSTRAINT_NAME=t.CONSTRAINT_NAME WHERE t.CONSTRAINT_TYPE IN ('PRIMARY KEY', 'UNIQUE') AND c.TABLE_CATALOG = 'master' AND c.TABLE_NAME = 'user_tests'

2024/05/21 14:29:53 /home/user/test/main.go:26 mssql: unknown type for gorm.DeletedAt
[3.518ms] [rows:0] INSERT INTO "user_tests" ("created_at","updated_at","deleted_at","name") OUTPUT INSERTED."id" VALUES ('2024-05-21 14:29:53.064','2024-05-21 14:29:53.064',NULL,'John Doe');
2024/05/21 14:29:53 Failed to create user:mssql: unknown type for gorm.DeletedAt
exit status 1

To Reproduce

❯ docker run -e "ACCEPT_EULA=Y" -e "MSSQL_SA_PASSWORD=testpasswordX1" -e "MSSQL_PID=Evaluation" -p 1433:1433  -d mcr.microsoft.com/mssql/server:2022-preview-ubuntu-22.04

1477f01afa2518debe3db75444d4ea26647b513bdfcd813fbbb2aebefc696f6a
package main

import (
	"fmt"
	"log"

	"gorm.io/driver/sqlserver"
	"gorm.io/gorm"
)

type UserTest struct {
	gorm.Model
	Name string
}

func main() {
	dsn := "sqlserver://sa:testpassword@localhost:1433?database=master"
	db, err := gorm.Open(sqlserver.Open(dsn), &gorm.Config{})
	if err != nil {
		log.Fatal("Failed to connect to database:", err)
	}

	db.AutoMigrate(&UserTest{})

	newUser := UserTest{Name: "John Doe"}
	err = db.Create(&newUser).Error
	if err != nil {
		log.Fatal("Failed to create user:", err)
	}
	var user UserTest
	db.First(&user, newUser.ID)

	fmt.Printf("User ID: %d, Name: %s\n", user.ID, user.Name)

	err = db.Delete(&newUser).Error
	if err != nil {
		log.Fatal("Failed to delete user:", err)
	}
}
module test

go 1.22.1

require (
	gorm.io/driver/sqlserver v1.5.3
	gorm.io/gorm v1.25.10
)

require (
	github.com/golang-sql/civil v0.0.0-20220223132316-b832511892a9 // indirect
	github.com/golang-sql/sqlexp v0.1.0 // indirect
	github.com/jinzhu/inflection v1.0.0 // indirect
	github.com/jinzhu/now v1.1.5 // indirect
	github.com/microsoft/go-mssqldb v1.7.1 // indirect
	golang.org/x/crypto v0.23.0 // indirect
	golang.org/x/text v0.15.0 // indirect
)

Expected behavior
When I run with: github.com/microsoft/go-mssqldb v1.7.0

❯ go run .
User ID: 1, Name: John Doe

Further technical details

SQL Server version: SQL Server 2022 on Ubuntu 22.04 (It doesn't work with SQL Server 2022 on Ubuntu 20.04 either)
Operating system: Microsoft Windows 10 Business (10.0.19045)
Docker: 4.29.0 (145265)
WSL: Debian GNU/Linux 12
WSL Kernel: 5.15.146.1-microsoft-standard-WSL2

@shueybubbles
Copy link
Collaborator

thx for opening an issue!
I think this function is missing a case for sql.NullTime

func (s *Stmt) makeParam(val driver.Value) (res param, err error) {

@mgross-ebner
Copy link

mgross-ebner commented May 23, 2024

I don't want to hijack the issue, but NullByte and NullInt16 also seem to be missing.

@shueybubbles
Copy link
Collaborator

thx I will update #195

@shueybubbles
Copy link
Collaborator

It's rather unfortunate that the Valuer implementation of sql.NullByte and its peers return an int64.
SQL always encrypted requires input type for parameters to match their database schema typs exactly. We can't pass a bigint parameter for a tinyint column.

Similarly, even for non-encrypted columns, people have complained that upscaling everything to bigint leads to suboptimal plans on the server.
Is there any reason NOT to convert sql.Nullxxx integer type parameters to their same-sized SQL types instead of to bigint ? @joao-intelliway @mgross-ebner @stuartpa

@shueybubbles
Copy link
Collaborator

@joao-intelliway can you double check the fix with v1.7.2 ?
if it's good I will turn the tag into a release.

@joao-intelliway
Copy link
Author

Fixed, thank you!

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.

3 participants