From 0ce1d79a6a771f7449ec493b993ed2a720917870 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 11 Oct 2016 08:34:58 -0700 Subject: database/sql: accept nil pointers to Valuers implemented on value receivers The driver.Valuer interface lets types map their Go representation to a suitable database/sql/driver.Value. If a user defines the Value method with a value receiver, such as: type MyStr string func (s MyStr) Value() (driver.Value, error) { return strings.ToUpper(string(s)), nil } Then they can't use (*MyStr)(nil) as an argument to an SQL call via database/sql, because *MyStr also implements driver.Value, but via a compiler-generated wrapper which checks whether the pointer is nil and panics if so. We now accept (*MyStr)(nil) and map it to "nil" (an SQL "NULL") if the Valuer method is implemented on MyStr instead of *MyStr. If a user implements the driver.Value interface with a pointer receiver, they retain full control of what nil means: type MyStr string func (s *MyStr) Value() (driver.Value, error) { if s == nil { return "missing MyStr", nil } return strings.ToUpper(string(*s)), nil } Adds tests for both cases. Fixes #8415 Change-Id: I897d609d80d46e2354d2669a8a3e090688eee3ad Reviewed-on: https://go-review.googlesource.com/31259 Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot Reviewed-by: Daniel Theophanes Reviewed-by: Brad Fitzpatrick --- src/database/sql/driver/types.go | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) (limited to 'src/database/sql/driver') diff --git a/src/database/sql/driver/types.go b/src/database/sql/driver/types.go index e480e701a4..c93c97a392 100644 --- a/src/database/sql/driver/types.go +++ b/src/database/sql/driver/types.go @@ -208,13 +208,35 @@ type defaultConverter struct{} var _ ValueConverter = defaultConverter{} +var valuerReflectType = reflect.TypeOf((*Valuer)(nil)).Elem() + +// 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 mirrored in the database/sql package. +func callValuerValue(vr Valuer) (v Value, err error) { + if rv := reflect.ValueOf(vr); rv.Kind() == reflect.Ptr && + rv.IsNil() && + rv.Type().Elem().Implements(valuerReflectType) { + return nil, nil + } + return vr.Value() +} + func (defaultConverter) ConvertValue(v interface{}) (Value, error) { if IsValue(v) { return v, nil } - if svi, ok := v.(Valuer); ok { - sv, err := svi.Value() + if vr, ok := v.(Valuer); ok { + sv, err := callValuerValue(vr) if err != nil { return nil, err } -- cgit v1.3