From 0a64be94d3dd2f394f42b56a45779dfccdfe7bc6 Mon Sep 17 00:00:00 2001 From: Alexandre Thomas Date: Fri, 18 Oct 2024 15:07:25 +0200 Subject: [PATCH 1/2] Add failing test on call to sqlx.In with (driver.Valuer)(nil) --- sqlx_test.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/sqlx_test.go b/sqlx_test.go index 9fac2cd..e6bac6e 100644 --- a/sqlx_test.go +++ b/sqlx_test.go @@ -1504,6 +1504,14 @@ func TestIssue197(t *testing.T) { }) } +type Valuer struct { + Val string +} + +func (v Valuer) Value() (driver.Value, error) { + return v.Val, nil +} + func TestIn(t *testing.T) { // some quite normal situations type tr struct { @@ -1538,6 +1546,24 @@ func TestIn(t *testing.T) { } } + // nil driver.Valuer + t.Run("with nil driver.Valuer", func(t *testing.T) { + query := `SELECT * FROM foo WHERE x = ? or y IN (?)` + _, _, err := In(query, + (*Valuer)(nil), // a non-inited pointer to valuer + []interface{}{ + "a", // a usual value + nil, // a nil value + Valuer{Val: "foo"}, // a Valuer + &Valuer{Val: "foo"}, // a pointer to valuer + (*Valuer)(nil), // a non-inited pointer to valuer + }, + ) + if err != nil { + t.Error(err) + } + }) + // too many bindVars, but no slices, so short circuits parsing // i'm not sure if this is the right behavior; this query/arg combo // might not work, but we shouldn't parse if we don't need to From 402ed0c412400cff5bf4eb6eb0a23e07e00964f2 Mon Sep 17 00:00:00 2001 From: Alexandre Thomas Date: Fri, 18 Oct 2024 15:07:55 +0200 Subject: [PATCH 2/2] Fix failing test on call to sqlx.In with (driver.Valuer)(nil) --- bind.go | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/bind.go b/bind.go index e698039..4c5b3d4 100644 --- a/bind.go +++ b/bind.go @@ -163,7 +163,7 @@ func In(query string, args ...interface{}) (string, []interface{}, error) { for i, arg := range args { if a, ok := arg.(driver.Valuer); ok { var err error - arg, err = a.Value() + arg, err = callValuerValue(a) if err != nil { return "", nil, err } @@ -263,3 +263,24 @@ func appendReflectSlice(args []interface{}, v reflect.Value, vlen int) []interfa return args } + +// callValuerValue returns vr.Value(), with one exception: +// If vr.Value is an auto-generated method on a pointer type and the +// pointer is nil, it would panic at runtime in the panicwrap +// method. Treat it like nil instead. +// Issue 8415. +// +// This is so people can implement driver.Value on value types and +// still use nil pointers to those types to mean nil/NULL, just like +// string/*string. +// +// This function is copied from database/sql/driver package +// and mirrored in the database/sql package. +func callValuerValue(vr driver.Valuer) (v driver.Value, err error) { + if rv := reflect.ValueOf(vr); rv.Kind() == reflect.Pointer && + rv.IsNil() && + rv.Type().Elem().Implements(reflect.TypeOf((*driver.Valuer)(nil)).Elem()) { + return nil, nil + } + return vr.Value() +}