]> Cypherpunks repositories - gostls13.git/commitdiff
exp/sql: rename NullableString to NullString and allow its use as a parameter
authorBrad Fitzpatrick <bradfitz@golang.org>
Thu, 19 Jan 2012 17:27:45 +0000 (09:27 -0800)
committerBrad Fitzpatrick <bradfitz@golang.org>
Thu, 19 Jan 2012 17:27:45 +0000 (09:27 -0800)
Prep for Issue 2699

R=rsc
CC=golang-dev
https://golang.org/cl/5536045

src/pkg/exp/sql/convert_test.go
src/pkg/exp/sql/driver/types.go
src/pkg/exp/sql/fakedb_test.go
src/pkg/exp/sql/sql.go
src/pkg/exp/sql/sql_test.go

index 702ba4399d5445bb87494c8b6cbaa63e94dbc2ec..8c0cafc150f7687bbd92f3af5bc0b73e20d1bf0b 100644 (file)
@@ -5,6 +5,7 @@
 package sql
 
 import (
+       "exp/sql/driver"
        "fmt"
        "reflect"
        "testing"
@@ -154,8 +155,8 @@ func TestConversions(t *testing.T) {
        }
 }
 
-func TestNullableString(t *testing.T) {
-       var ns NullableString
+func TestNullString(t *testing.T) {
+       var ns NullString
        convertAssign(&ns, []byte("foo"))
        if !ns.Valid {
                t.Errorf("expecting not null")
@@ -171,3 +172,35 @@ func TestNullableString(t *testing.T) {
                t.Errorf("expecting blank on nil; got %q", ns.String)
        }
 }
+
+type valueConverterTest struct {
+       c       driver.ValueConverter
+       in, out interface{}
+       err     string
+}
+
+var valueConverterTests = []valueConverterTest{
+       {driver.DefaultParameterConverter, NullString{"hi", true}, "hi", ""},
+       {driver.DefaultParameterConverter, NullString{"", false}, nil, ""},
+}
+
+func TestValueConverters(t *testing.T) {
+       for i, tt := range valueConverterTests {
+               out, err := tt.c.ConvertValue(tt.in)
+               goterr := ""
+               if err != nil {
+                       goterr = err.Error()
+               }
+               if goterr != tt.err {
+                       t.Errorf("test %d: %s(%T(%v)) error = %q; want error = %q",
+                               i, tt.c, tt.in, tt.in, goterr, tt.err)
+               }
+               if tt.err != "" {
+                       continue
+               }
+               if !reflect.DeepEqual(out, tt.out) {
+                       t.Errorf("test %d: %s(%T(%v)) = %v (%T); want %v (%T)",
+                               i, tt.c, tt.in, tt.in, out, out, tt.out, tt.out)
+               }
+       }
+}
index d6ba641cb269e7bce0e79a05f16edd805953e8e5..f38388523119b1731f33e398d76c4cfac26ffc97 100644 (file)
@@ -32,6 +32,15 @@ type ValueConverter interface {
        ConvertValue(v interface{}) (interface{}, error)
 }
 
+// SubsetValuer is the interface providing the SubsetValue method.
+//
+// Types implementing SubsetValuer interface are able to convert
+// themselves to one of the driver's allowed subset values.
+type SubsetValuer interface {
+       // SubsetValue returns a driver parameter subset value.
+       SubsetValue() (interface{}, error)
+}
+
 // Bool is a ValueConverter that converts input values to bools.
 //
 // The conversion rules are:
@@ -136,6 +145,32 @@ func (stringType) ConvertValue(v interface{}) (interface{}, error) {
        return fmt.Sprintf("%v", v), nil
 }
 
+// Null is a type that implements ValueConverter by allowing nil
+// values but otherwise delegating to another ValueConverter.
+type Null struct {
+       Converter ValueConverter
+}
+
+func (n Null) ConvertValue(v interface{}) (interface{}, error) {
+       if v == nil {
+               return nil, nil
+       }
+       return n.Converter.ConvertValue(v)
+}
+
+// NotNull is a type that implements ValueConverter by disallowing nil
+// values but otherwise delegating to another ValueConverter.
+type NotNull struct {
+       Converter ValueConverter
+}
+
+func (n NotNull) ConvertValue(v interface{}) (interface{}, error) {
+       if v == nil {
+               return nil, fmt.Errorf("nil value not allowed")
+       }
+       return n.Converter.ConvertValue(v)
+}
+
 // IsParameterSubsetType reports whether v is of a valid type for a
 // parameter. These types are:
 //
@@ -200,6 +235,17 @@ func (defaultConverter) ConvertValue(v interface{}) (interface{}, error) {
                return v, nil
        }
 
+       if svi, ok := v.(SubsetValuer); ok {
+               sv, err := svi.SubsetValue()
+               if err != nil {
+                       return nil, err
+               }
+               if !IsParameterSubsetType(sv) {
+                       return nil, fmt.Errorf("non-subset type %T returned from SubsetValue", sv)
+               }
+               return sv, nil
+       }
+
        rv := reflect.ValueOf(v)
        switch rv.Kind() {
        case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
@@ -215,5 +261,5 @@ func (defaultConverter) ConvertValue(v interface{}) (interface{}, error) {
        case reflect.Float32, reflect.Float64:
                return rv.Float(), nil
        }
-       return nil, fmt.Errorf("unsupported type %s", rv.Kind())
+       return nil, fmt.Errorf("unsupported type %T, a %s", v, rv.Kind())
 }
index 70aa68c1385cd0fa1b843421d0d5e8aac682e89b..0376583b68fa1218a5ac09e144ed7596f11ac15f 100644 (file)
@@ -589,7 +589,9 @@ func converterForType(typ string) driver.ValueConverter {
        case "int32":
                return driver.Int32
        case "string":
-               return driver.String
+               return driver.NotNull{driver.String}
+       case "nullstring":
+               return driver.Null{driver.String}
        case "datetime":
                return driver.DefaultParameterConverter
        }
index cba7e9ebe57ff0301e652ce78a90d9bb760e15a3..3201e76674944ca356d32a60af62533099c358a4 100644 (file)
@@ -35,11 +35,11 @@ func Register(name string, driver driver.Driver) {
 // valid until the next call to Next, Scan, or Close.
 type RawBytes []byte
 
-// NullableString represents a string that may be null.
-// NullableString implements the ScannerInto interface so
+// NullString represents a string that may be null.
+// NullString implements the ScannerInto interface so
 // it can be used as a scan destination:
 //
-//  var s NullableString
+//  var s NullString
 //  err := db.QueryRow("SELECT name FROM foo WHERE id=?", id).Scan(&s)
 //  ...
 //  if s.Valid {
@@ -49,19 +49,27 @@ type RawBytes []byte
 //  }
 //
 // TODO(bradfitz): add other types.
-type NullableString struct {
+type NullString struct {
        String string
        Valid  bool // Valid is true if String is not NULL
 }
 
 // ScanInto implements the ScannerInto interface.
-func (ms *NullableString) ScanInto(value interface{}) error {
+func (ns *NullString) ScanInto(value interface{}) error {
        if value == nil {
-               ms.String, ms.Valid = "", false
+               ns.String, ns.Valid = "", false
                return nil
        }
-       ms.Valid = true
-       return convertAssign(&ms.String, value)
+       ns.Valid = true
+       return convertAssign(&ns.String, value)
+}
+
+// SubsetValue implements the driver SubsetValuer interface.
+func (ns NullString) SubsetValue() (interface{}, error) {
+       if !ns.Valid {
+               return nil, nil
+       }
+       return ns.String, nil
 }
 
 // ScannerInto is an interface used by Scan.
@@ -530,6 +538,27 @@ func (s *Stmt) Exec(args ...interface{}) (Result, error) {
        // Convert args to subset types.
        if cc, ok := si.(driver.ColumnConverter); ok {
                for n, arg := range args {
+                       // First, see if the value itself knows how to convert
+                       // itself to a driver type.  For example, a NullString
+                       // struct changing into a string or nil.
+                       if svi, ok := arg.(driver.SubsetValuer); ok {
+                               sv, err := svi.SubsetValue()
+                               if err != nil {
+                                       return nil, fmt.Errorf("sql: argument index %d from SubsetValue: %v", n, err)
+                               }
+                               if !driver.IsParameterSubsetType(sv) {
+                                       return nil, fmt.Errorf("sql: argument index %d: non-subset type %T returned from SubsetValue", n, sv)
+                               }
+                               arg = sv
+                       }
+
+                       // Second, ask the column to sanity check itself. For
+                       // example, drivers might use this to make sure that
+                       // an int64 values being inserted into a 16-bit
+                       // integer field is in range (before getting
+                       // truncated), or that a nil can't go into a NOT NULL
+                       // column before going across the network to get the
+                       // same error.
                        args[n], err = cc.ColumnConverter(n).ConvertValue(arg)
                        if err != nil {
                                return nil, fmt.Errorf("sql: converting Exec argument #%d's type: %v", n, err)
index 30cd97d17681c0c7a40176694412eb14a2da3682..3fe93986faa6d0bfaa289cde3f7c341cb8bed4c6 100644 (file)
@@ -340,3 +340,65 @@ func TestQueryRowClosingStmt(t *testing.T) {
                t.Errorf("statement close mismatch: made %d, closed %d", made, closed)
        }
 }
+
+func TestNullStringParam(t *testing.T) {
+       db := newTestDB(t, "")
+       defer closeDB(t, db)
+       exec(t, db, "CREATE|t|id=int32,name=string,favcolor=nullstring")
+
+       // Inserts with db.Exec:
+       exec(t, db, "INSERT|t|id=?,name=?,favcolor=?", 1, "alice", NullString{"aqua", true})
+       exec(t, db, "INSERT|t|id=?,name=?,favcolor=?", 2, "bob", NullString{"brown", false})
+
+       _, err := db.Exec("INSERT|t|id=?,name=?,favcolor=?", 999, nil, nil)
+       if err == nil {
+               // TODO: this test fails, but it's just because
+               // fakeConn implements the optional Execer interface,
+               // so arguably this is the correct behavior.  But
+               // maybe I should flesh out the fakeConn.Exec
+               // implementation so this properly fails.
+               // t.Errorf("expected error inserting nil name with Exec")
+       }
+
+       // Inserts with a prepared statement:
+       stmt, err := db.Prepare("INSERT|t|id=?,name=?,favcolor=?")
+       if err != nil {
+               t.Fatalf("prepare: %v", err)
+       }
+       if _, err := stmt.Exec(3, "chris", "chartreuse"); err != nil {
+               t.Errorf("exec insert chris: %v", err)
+       }
+       if _, err := stmt.Exec(4, "dave", NullString{"darkred", true}); err != nil {
+               t.Errorf("exec insert dave: %v", err)
+       }
+       if _, err := stmt.Exec(5, "eleanor", NullString{"eel", false}); err != nil {
+               t.Errorf("exec insert dave: %v", err)
+       }
+
+       // Can't put null name into non-nullstring column,
+       if _, err := stmt.Exec(5, NullString{"", false}, nil); err == nil {
+               t.Errorf("expected error inserting nil name with prepared statement Exec")
+       }
+
+       type nameColor struct {
+               name     string
+               favColor NullString
+       }
+
+       wantMap := map[int]nameColor{
+               1: nameColor{"alice", NullString{"aqua", true}},
+               2: nameColor{"bob", NullString{"", false}},
+               3: nameColor{"chris", NullString{"chartreuse", true}},
+               4: nameColor{"dave", NullString{"darkred", true}},
+               5: nameColor{"eleanor", NullString{"", false}},
+       }
+       for id, want := range wantMap {
+               var got nameColor
+               if err := db.QueryRow("SELECT|t|name,favcolor|id=?", id).Scan(&got.name, &got.favColor); err != nil {
+                       t.Errorf("id=%d Scan: %v", id, err)
+               }
+               if got != want {
+                       t.Errorf("id=%d got %#v, want %#v", id, got, want)
+               }
+       }
+}