feat: Added support for datetime format #957

inoth · 2024-12-06T07:42:54Z

Cannot set the entity field to time.Time when connected via gorm.
Add formatBinaryValue support for time.Time type

type UserInfo struct {
	ID        int        
	CreatedAt time.Time  
	UpdatedAt *time.Time 
}

func (qc *UserInfo) TableName() string {
	return "user_info"
}

func main() {

	User := "root"
	Passwd := "password"
	Host := "localhost"
	Port := 4000
	DbName := "faker_db"

	constr := fmt.Sprintf("%s:%s@tcp(%s:%d)/%s?charset=utf8mb4&parseTime=True&loc=Local", User, Passwd, Host, Port, DbName)
	client, err := gorm.Open(mysql.New(mysql.Config{
		DSN:                       constr,
		DefaultStringSize:         1 << 10,
		DisableDatetimePrecision:  true,
		DontSupportRenameIndex:    true,  
		DontSupportRenameColumn:   true,  
		SkipInitializeWithVersion: false, 
	}))
	if err != nil {
		panic(fmt.Errorf("failed to connect to mysql: %v", err))
	}

	var user UserInfo
	err = client.Where("id = ?", 1).First(&user).Error
	if err != nil {
		log.Println(err)
		return
	}
	fmt.Printf("%+v\n", user)
}

{CreatedAt:2024-08-13 22:16:46 +0800 CST UpdatedAt:2024-08-15 15:23:43 +0800 CST}

dveeden

This seems fine to me.

Some small things:

errcheck (via golangci-lint) reports a few issues
I think this should have some tests

How would this affect upgrades and compatibility?

Thanks for the detailed example in the description

dveeden · 2024-12-06T09:27:11Z

Your example didn't include imports or content of go.mod and seems to use a go-sql-driver style dsn. Could you provide a full example?

This is what I used to test this:

package main

import (
	"fmt"
	"log"
	"time"

	_ "github.com/go-mysql-org/go-mysql/driver"
	"gorm.io/driver/mysql"
	"gorm.io/gorm"
)

type UserInfo struct {
	ID        int
	CreatedAt time.Time
	UpdatedAt *time.Time
}

func (qc *UserInfo) TableName() string {
	return "user_info"
}

func main() {

	User := "root"
	Passwd := ""
	Host := "127.0.0.1"
	Port := 4000
	DbName := "test"

	constr := fmt.Sprintf("%s:%s@%s:%d/%s", User, Passwd, Host, Port, DbName)
	client, err := gorm.Open(mysql.New(mysql.Config{
		DriverName:                "gomysql",
		DSN:                       constr,
		DefaultStringSize:         1 << 10,
		DisableDatetimePrecision:  true,
		DontSupportRenameIndex:    true,
		DontSupportRenameColumn:   true,
		SkipInitializeWithVersion: false,
	}))
	if err != nil {
		panic(fmt.Errorf("failed to connect to mysql: %v", err))
	}

	var user UserInfo
	err = client.Where("id = ?", 1).First(&user).Error
	if err != nil {
		log.Println(err)
		return
	}
	fmt.Printf("%+v\n", user)
}

module issue957

go 1.23.3

require (
	github.com/go-mysql-org/go-mysql v1.10.0
	gorm.io/driver/mysql v1.5.7
	gorm.io/gorm v1.25.12
)

require (
	github.com/Masterminds/semver v1.5.0 // indirect
	github.com/go-sql-driver/mysql v1.7.1 // indirect
	github.com/google/uuid v1.3.0 // indirect
	github.com/jinzhu/inflection v1.0.0 // indirect
	github.com/jinzhu/now v1.1.5 // indirect
	github.com/klauspost/compress v1.17.8 // indirect
	github.com/pingcap/errors v0.11.5-0.20240311024730-e056997136bb // indirect
	github.com/pingcap/log v1.1.1-0.20230317032135-a0d097d16e22 // indirect
	github.com/pingcap/tidb/pkg/parser v0.0.0-20241118164214-4f047be191be // indirect
	github.com/siddontang/go v0.0.0-20180604090527-bdc77568d726 // indirect
	github.com/siddontang/go-log v0.0.0-20180807004314-8d05993dda07 // indirect
	go.uber.org/atomic v1.11.0 // indirect
	go.uber.org/multierr v1.11.0 // indirect
	go.uber.org/zap v1.27.0 // indirect
	golang.org/x/text v0.20.0 // indirect
	gopkg.in/natefinch/lumberjack.v2 v2.2.1 // indirect
)

replace github.com/go-mysql-org/go-mysql => /home/dvaneeden/dev/go-mysql

The replace is to use a checked out version of this PR

I also had to do this:

diff --git a/driver/driver.go b/driver/driver.go
index 4054db2..fa36dc8 100644
--- a/driver/driver.go
+++ b/driver/driver.go
@@ -389,13 +389,15 @@ func (r *rows) Next(dest []sqldriver.Value) error {
        return nil
 }
 
+var driverName = "mysql"
+
 func init() {
        options["compress"] = CompressOption
        options["collation"] = CollationOption
        options["readTimeout"] = ReadTimeoutOption
        options["writeTimeout"] = WriteTimeoutOption
 
-       sql.Register("mysql", driver{})
+       sql.Register(driverName, driver{})
 }
 
 // SetCustomTLSConfig sets a custom TLSConfig for the address (host:port) of the supplied DSN.

go run -ldflags '-X "github.com/go-mysql-org/go-mysql/driver.driverName=gomysql"' main.go

This is to avoid the name collision between go-mysql and go-sql-driver/mysql which both register mysql as driver name by default. Normally one would only import one or the other but here gorm.io/driver/mysql depends on go-sql-driver/mysql. See also https://gorm.io/docs/connecting_to_the_database.html#Customize-Driver

The result is this:

$ go run -ldflags '-X "github.com/go-mysql-org/go-mysql/driver.driverName=gomysql"' main.go 

2024/12/06 10:26:45 /tmp/issue957/main.go:46 sql: Scan error on column index 1, name "CreatedAt": unsupported Scan, storing driver.Value type []uint8 into type *time.Time
[1.591ms] [rows:1] SELECT * FROM `user_info` WHERE id = 1 ORDER BY `user_info`.`id` LIMIT 1
2024/12/06 10:26:45 sql: Scan error on column index 1, name "CreatedAt": unsupported Scan, storing driver.Value type []uint8 into type *time.Time

So it looks like this isn't working as intended.

dveeden · 2024-12-06T09:39:55Z

I created #958 to make it easier to use go-mysql with GORM

inoth · 2024-12-06T10:18:05Z

您的示例未包含 go.mod 的导入或内容，并且似乎使用了 go-sql-driver 样式的 dsn。您能提供一个完整的示例吗？

这是我用来测试这个的：

package main

import (
	"fmt"
	"log"
	"time"

	_ "github.com/go-mysql-org/go-mysql/driver"
	"gorm.io/driver/mysql"
	"gorm.io/gorm"
)

type UserInfo struct {
	ID        int
	CreatedAt time.Time
	UpdatedAt *time.Time
}

func (qc *UserInfo) TableName() string {
	return "user_info"
}

func main() {

	User := "root"
	Passwd := ""
	Host := "127.0.0.1"
	Port := 4000
	DbName := "test"

	constr := fmt.Sprintf("%s:%s@%s:%d/%s", User, Passwd, Host, Port, DbName)
	client, err := gorm.Open(mysql.New(mysql.Config{
		DriverName:                "gomysql",
		DSN:                       constr,
		DefaultStringSize:         1 << 10,
		DisableDatetimePrecision:  true,
		DontSupportRenameIndex:    true,
		DontSupportRenameColumn:   true,
		SkipInitializeWithVersion: false,
	}))
	if err != nil {
		panic(fmt.Errorf("failed to connect to mysql: %v", err))
	}

	var user UserInfo
	err = client.Where("id = ?", 1).First(&user).Error
	if err != nil {
		log.Println(err)
		return
	}
	fmt.Printf("%+v\n", user)
}

module issue957

go 1.23.3

require (
	github.com/go-mysql-org/go-mysql v1.10.0
	gorm.io/driver/mysql v1.5.7
	gorm.io/gorm v1.25.12
)

require (
	github.com/Masterminds/semver v1.5.0 // indirect
	github.com/go-sql-driver/mysql v1.7.1 // indirect
	github.com/google/uuid v1.3.0 // indirect
	github.com/jinzhu/inflection v1.0.0 // indirect
	github.com/jinzhu/now v1.1.5 // indirect
	github.com/klauspost/compress v1.17.8 // indirect
	github.com/pingcap/errors v0.11.5-0.20240311024730-e056997136bb // indirect
	github.com/pingcap/log v1.1.1-0.20230317032135-a0d097d16e22 // indirect
	github.com/pingcap/tidb/pkg/parser v0.0.0-20241118164214-4f047be191be // indirect
	github.com/siddontang/go v0.0.0-20180604090527-bdc77568d726 // indirect
	github.com/siddontang/go-log v0.0.0-20180807004314-8d05993dda07 // indirect
	go.uber.org/atomic v1.11.0 // indirect
	go.uber.org/multierr v1.11.0 // indirect
	go.uber.org/zap v1.27.0 // indirect
	golang.org/x/text v0.20.0 // indirect
	gopkg.in/natefinch/lumberjack.v2 v2.2.1 // indirect
)

replace github.com/go-mysql-org/go-mysql => /home/dvaneeden/dev/go-mysql

替换是使用此 PR 的签出版本

我还必须这样做：

diff --git a/driver/driver.go b/driver/driver.go
index 4054db2..fa36dc8 100644
--- a/driver/driver.go
+++ b/driver/driver.go
@@ -389,13 +389,15 @@ func (r *rows) Next(dest []sqldriver.Value) error {
        return nil
 }
 
+var driverName = "mysql"
+
 func init() {
        options["compress"] = CompressOption
        options["collation"] = CollationOption
        options["readTimeout"] = ReadTimeoutOption
        options["writeTimeout"] = WriteTimeoutOption
 
-       sql.Register("mysql", driver{})
+       sql.Register(driverName, driver{})
 }
 
 // SetCustomTLSConfig sets a custom TLSConfig for the address (host:port) of the supplied DSN.

go run -ldflags '-X "github.com/go-mysql-org/go-mysql/driver.driverName=gomysql"' main.go

这是为了避免 go-mysql 和 go-sql-driver/mysql 之间的名称冲突，这两个mysql名称默认都注册为驱动程序名称。通常只会导入其中一个，但这里 gorm.io/driver/mysql依赖于 go-sql-driver/mysql。另请参阅https://gorm.io/docs/connecting_to_the_database.html#Customize-Driver

结果是这样的：

$ go run -ldflags '-X "github.com/go-mysql-org/go-mysql/driver.driverName=gomysql"' main.go 

2024/12/06 10:26:45 /tmp/issue957/main.go:46 sql: Scan error on column index 1, name "CreatedAt": unsupported Scan, storing driver.Value type []uint8 into type *time.Time
[1.591ms] [rows:1] SELECT * FROM `user_info` WHERE id = 1 ORDER BY `user_info`.`id` LIMIT 1
2024/12/06 10:26:45 sql: Scan error on column index 1, name "CreatedAt": unsupported Scan, storing driver.Value type []uint8 into type *time.Time

看起来这并没有按预期进行。

Thank you for your reply

I created a sample library, you can have a look at this.
Maybe you need to change and replace the references in the go mod, see the readme for details

https://github.com/inoth/go-mysql-example

dveeden · 2024-12-06T10:24:36Z

Isn't this PR working on the server part of your example code instead of the client side?

inoth · 2024-12-06T10:29:21Z

Isn't this PR working on the server part of your example code instead of the client side?

I use replace to point to the local test after I clone the project locally

lance6716 · 2024-12-09T03:14:09Z

Hi, please fix linter error

  Running [/home/runner/golangci-lint-1.62.2-linux-amd64/golangci-lint run  --timeout=3m] in [/home/runner/work/go-mysql/go-mysql] ...
  mysql/resultset_helper.go:27:15: Error return value of `binary.Write` is not checked (errcheck)
  		binary.Write(&buf, binary.LittleEndian, uint16(year))
  		            ^
  mysql/resultset_helper.go:33:15: Error return value of `binary.Write` is not checked (errcheck)
  		binary.Write(&buf, binary.LittleEndian, uint32(nanosec/1000))
  		            ^
  mysql/resultset_helper.go:36:15: Error return value of `binary.Write` is not checked (errcheck)
  		binary.Write(&buf, binary.LittleEndian, uint16(year))

inoth · 2024-12-12T07:10:52Z

Hi, please fix linter error

  Running [/home/runner/golangci-lint-1.62.2-linux-amd64/golangci-lint run  --timeout=3m] in [/home/runner/work/go-mysql/go-mysql] ...
  mysql/resultset_helper.go:27:15: Error return value of `binary.Write` is not checked (errcheck)
  		binary.Write(&buf, binary.LittleEndian, uint16(year))
  		            ^
  mysql/resultset_helper.go:33:15: Error return value of `binary.Write` is not checked (errcheck)
  		binary.Write(&buf, binary.LittleEndian, uint32(nanosec/1000))
  		            ^
  mysql/resultset_helper.go:36:15: Error return value of `binary.Write` is not checked (errcheck)
  		binary.Write(&buf, binary.LittleEndian, uint16(year))

Okay, I took care of it

lance6716

rest lgtm. Thanks!

lance6716 · 2024-12-15T13:11:36Z

mysql/resultset_helper.go


 	"github.com/pingcap/errors"
 	"github.com/siddontang/go/hack"
 )

+func toBinaryDateTime(t time.Time) ([]byte, error) {


There's FormatBinaryDateTime under mysql/util.go. Can you move this function adjacent to FormatBinaryDateTime? So we can compare their logic easier.

Also, please add a unit test for FormatBinaryDateTime and ToBinaryDateTime, covert some value back and forth

dveeden · 2024-12-15T15:53:30Z

Please resolve the conflict

feat: Added support for datetime format

fe07a17

dveeden reviewed Dec 6, 2024

View reviewed changes

dveeden requested a review from lance6716 December 6, 2024 09:27

fix: handler "Error return value of binary.Write is not checked"

aa48110

Merge branch 'master' into master

dd2c4b1

lance6716 reviewed Dec 15, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Added support for datetime format #957

feat: Added support for datetime format #957

inoth commented Dec 6, 2024

dveeden left a comment •

edited

Loading

dveeden commented Dec 6, 2024 •

edited

Loading

dveeden commented Dec 6, 2024

inoth commented Dec 6, 2024 •

edited

Loading

dveeden commented Dec 6, 2024

inoth commented Dec 6, 2024

lance6716 commented Dec 9, 2024

inoth commented Dec 12, 2024

lance6716 left a comment

lance6716 Dec 15, 2024 •

edited

Loading

dveeden commented Dec 15, 2024

feat: Added support for datetime format #957

Are you sure you want to change the base?

feat: Added support for datetime format #957

Conversation

inoth commented Dec 6, 2024

dveeden left a comment • edited Loading

Choose a reason for hiding this comment

dveeden commented Dec 6, 2024 • edited Loading

dveeden commented Dec 6, 2024

inoth commented Dec 6, 2024 • edited Loading

dveeden commented Dec 6, 2024

inoth commented Dec 6, 2024

lance6716 commented Dec 9, 2024

inoth commented Dec 12, 2024

lance6716 left a comment

Choose a reason for hiding this comment

lance6716 Dec 15, 2024 • edited Loading

Choose a reason for hiding this comment

dveeden commented Dec 15, 2024

dveeden left a comment •

edited

Loading

dveeden commented Dec 6, 2024 •

edited

Loading

inoth commented Dec 6, 2024 •

edited

Loading

lance6716 Dec 15, 2024 •

edited

Loading