uptrace / bun

SQL-first Golang ORM
https://bun.uptrace.dev
BSD 2-Clause "Simplified" License
3.65k stars 221 forks source link

Add option to initialize MySQLDialect with server timezone #1021

Closed m4ex closed 1 week ago

m4ex commented 3 weeks ago

This merge request adds the ability to initialize MySQLDialect with the server's timezone. This ensures that timestamp fields are set correctly when the server's timezone differs from UTC-0. The default behavior remains unchanged, as this change only introduces a new constructor for MySQLDialect.

The standard MySQL driver automatically converts a time.Time object to the server's timezone (source). However, in Bun, the default driver receives a byte string instead of the original time.Time object, which makes it impossible to set the correct time. While it is possible to manually convert the time.Time object to the appropriate timezone, this becomes cumbersome when dealing with a large number of models.

With the proposed solution, we can explicitly specify the server's timezone, allowing for correct timestamp entries. Reading timestamps remains unaffected, as this is already handled correctly by the driver.

The example above demonstrates how to open a MySQL connection using the server's timezone (Asia/Tokyo) and initialize MySQLDialect with the same timezone, ensuring that all timestamp fields are handled correctly:

driverDB, err := sql.Open("mysql", "root:test@tcp(127.0.0.1:3306)/test?parseTime=true&loc=Asia%2FTokyo")
if err != nil {
    panic(err)
}

bunDB := bun.NewDB(driverDB, mysqldialect.NewWithLocation("Asia/Tokyo"))
vmihailenco commented 3 weeks ago

I like the idea, but let's add an option WithTimeLocation like this:

driverDB, err := sql.Open("mysql", "root:test@tcp(127.0.0.1:3306)/test?parseTime=true&loc=Asia%2FTokyo")
if err != nil {
    panic(err)
}

bunDB := bun.NewDB(driverDB, mysqldialect.New(mysqldialect.WithTimeLocation("Asia/Tokyo")))
m4ex commented 3 weeks ago

This would be a breaking change since the mysqldialect.New() method previously didn't accept any parameters. If you'd rather not introduce a new constructor, an alternative approach would be to use a setter in the method chaining style, like this:

bunDB := bun.NewDB(driverDB, mysqldialect.New().WithTimeLocation("Asia/Tokyo"))

If this option works for you, I'd be happy to update the code accordingly.

vmihailenco commented 2 weeks ago

mysqldialect.New() should accept options ...Option which is a common pattern, for example, opts ...DBOption

m4ex commented 1 week ago

I've reworked the code using the functional options pattern. Sorry for the long delay in getting back to you. Let me know if this approach works for you, and I'm happy to make any changes if needed.