fix: Implement sql.Valuer with pointer receiver#108
Conversation
This allows interpreting `*uuid.UUID(nil)` values as `NULL` on SQL databases. Closes: gofrs#107
|
This is a breaking change since |
How so? What needs to be changed by the users? package main
import (
"database/sql/driver"
"fmt"
)
type UUIDP struct {
}
func (u *UUIDP) Value() (driver.Value, error) {
return "pointer", nil
}
type UUIDNP struct {
}
func (u UUIDNP) Value() (driver.Value, error) {
return "non pointer", nil
}
func main() {
// regular
var u UUIDP
fmt.Println(u.Value())
var up *UUIDP
// calling .Value on pointer implementation works ok.
fmt.Println(up.Value()) // works
if _, ok := ((any)(up)).(driver.Valuer); ok { // implements
fmt.Printf("%T implements driver.Valuer\n", up)
}
// non-pointer
var np UUIDNP
fmt.Println(np.Value())
var npp *UUIDNP
if _, ok := ((any)(npp)).(driver.Valuer); ok { // implements
fmt.Printf("%T implements driver.Valuer\n", npp)
}
// calling the implementation on a non-pointer method panics
fmt.Println(npp.Value())
}run it on playground: https://go.dev/play/p/6B2INaIbb0h |
Codecov ReportBase: 100.00% // Head: 100.00% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## master #108 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 4 4
Lines 411 414 +3
=========================================
+ Hits 411 414 +3
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
This will break all code bases that expect a UUID value (not pointer) to implement Additionally, you can check this using the actual package main
import (
"database/sql"
"github.com/gofrs/uuid"
_ "github.com/mattn/go-sqlite3"
)
func main() {
db, err := sql.Open("sqlite3", ":memory:")
if err != nil {
panic(err)
}
defer db.Close()
_, err = db.Exec(`CREATE TABLE uuids (id INTEGER PRIMARY KEY, uuid TEXT NOT NULL);`)
if err != nil {
panic(err)
}
u, err := uuid.NewV4()
if err != nil {
panic(err)
}
_, err = db.Exec(`INSERT INTO uuids (uuid) VALUES (?);`, u)
if err != nil {
panic(err)
}
var found uuid.UUID
err = db.QueryRow(`SELECT (uuid) FROM uuids WHERE uuid = ?`, u).Scan(&found)
if err != nil {
panic(err)
}
if found != u {
panic("WAT")
}
println("Ok")
}With your proposed change the program will error with: |
|
In addition to this being a breaking change as @charlievieth called out, |
|
Hi @abdusco really appreciate that you identified this gap and took the time to submit a fix. As others have stated, this is a breaking change to the library and as is I don't believe we can accept this donation. There's another PR that address a similar need (#113). I'll give that PR a couple of days before I merge it, would love to hear your thoughts over there. Thanks again! |
This allows interpreting
*uuid.UUID(nil)values asNULLon SQL databases.Closes: #107