From 943f6cc837f4513a8cae7df199d14c5a38cf7677 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 20 Feb 2012 14:25:28 +1100 Subject: [PATCH] database/sql/driver: API cleanups -- add driver.Value type and documentation, convert from interface{} to Value where appropriate. -- don't say "subset" anywhere, -- SubsetValuer -> Valuer -- SubsetValue -> Value -- IsParameterSubsetType -> IsValue -- IsScanSubsetType -> IsScanValue Fixes #2842 R=golang-dev, r, rsc CC=golang-dev https://golang.org/cl/5674084 --- src/pkg/database/sql/convert.go | 4 +- src/pkg/database/sql/driver/driver.go | 51 ++++++++--------- src/pkg/database/sql/driver/types.go | 80 ++++++++++----------------- src/pkg/database/sql/fakedb_test.go | 16 +++--- src/pkg/database/sql/sql.go | 54 +++++++++--------- 5 files changed, 92 insertions(+), 113 deletions(-) diff --git a/src/pkg/database/sql/convert.go b/src/pkg/database/sql/convert.go index 4afa2bef753..bfcb03ccf8d 100644 --- a/src/pkg/database/sql/convert.go +++ b/src/pkg/database/sql/convert.go @@ -17,8 +17,8 @@ import ( // subsetTypeArgs takes a slice of arguments from callers of the sql // package and converts them into a slice of the driver package's // "subset types". -func subsetTypeArgs(args []interface{}) ([]interface{}, error) { - out := make([]interface{}, len(args)) +func subsetTypeArgs(args []interface{}) ([]driver.Value, error) { + out := make([]driver.Value, len(args)) for n, arg := range args { var err error out[n], err = driver.DefaultParameterConverter.ConvertValue(arg) diff --git a/src/pkg/database/sql/driver/driver.go b/src/pkg/database/sql/driver/driver.go index b9300776050..7f986b80f2c 100644 --- a/src/pkg/database/sql/driver/driver.go +++ b/src/pkg/database/sql/driver/driver.go @@ -6,21 +6,20 @@ // drivers as used by package sql. // // Most code should use package sql. -// -// Drivers only need to be aware of a subset of Go's types. The sql package -// will convert all types into one of the following: +package driver + +import "errors" + +// A driver Value is a value that drivers must be able to handle. +// A Value is either nil or an instance of one of these types: // // int64 // float64 // bool -// nil // []byte // string [*] everywhere except from Rows.Next. // time.Time -// -package driver - -import "errors" +type Value interface{} // Driver is the interface that must be implemented by a database // driver. @@ -50,11 +49,9 @@ var ErrSkip = errors.New("driver: skip fast-path; continue as if unimplemented") // first prepare a query, execute the statement, and then close the // statement. // -// All arguments are of a subset type as defined in the package docs. -// // Exec may return ErrSkip. type Execer interface { - Exec(query string, args []interface{}) (Result, error) + Exec(query string, args []Value) (Result, error) } // Conn is a connection to a database. It is not used concurrently @@ -127,18 +124,17 @@ type Stmt interface { NumInput() int // Exec executes a query that doesn't return rows, such - // as an INSERT or UPDATE. The args are all of a subset - // type as defined above. - Exec(args []interface{}) (Result, error) + // as an INSERT or UPDATE. + Exec(args []Value) (Result, error) // Exec executes a query that may return rows, such as a - // SELECT. The args of all of a subset type as defined above. - Query(args []interface{}) (Rows, error) + // SELECT. + Query(args []Value) (Rows, error) } // ColumnConverter may be optionally implemented by Stmt if the // the statement is aware of its own columns' types and can -// convert from any type to a driver subset type. +// convert from any type to a driver Value. type ColumnConverter interface { // ColumnConverter returns a ValueConverter for the provided // column index. If the type of a specific column isn't known @@ -162,12 +158,12 @@ type Rows interface { // the provided slice. The provided slice will be the same // size as the Columns() are wide. // - // The dest slice may be populated with only with values - // of subset types defined above, but excluding string. + // The dest slice may be populated only with + // a driver Value type, but excluding string. // All string values must be converted to []byte. // // Next should return io.EOF when there are no more rows. - Next(dest []interface{}) error + Next(dest []Value) error } // Tx is a transaction. @@ -190,18 +186,19 @@ func (v RowsAffected) RowsAffected() (int64, error) { return int64(v), nil } -// DDLSuccess is a pre-defined Result for drivers to return when a DDL -// command succeeds. -var DDLSuccess ddlSuccess +// ResultNoRows is a pre-defined Result for drivers to return when a DDL +// command (such as a CREATE TABLE) succeeds. It returns an error for both +// LastInsertId and RowsAffected. +var ResultNoRows noRows -type ddlSuccess struct{} +type noRows struct{} -var _ Result = ddlSuccess{} +var _ Result = noRows{} -func (ddlSuccess) LastInsertId() (int64, error) { +func (noRows) LastInsertId() (int64, error) { return 0, errors.New("no LastInsertId available after DDL statement") } -func (ddlSuccess) RowsAffected() (int64, error) { +func (noRows) RowsAffected() (int64, error) { return 0, errors.New("no RowsAffected available after DDL statement") } diff --git a/src/pkg/database/sql/driver/types.go b/src/pkg/database/sql/driver/types.go index ce3c943ead2..3305354dfd0 100644 --- a/src/pkg/database/sql/driver/types.go +++ b/src/pkg/database/sql/driver/types.go @@ -17,28 +17,28 @@ import ( // driver package to provide consistent implementations of conversions // between drivers. The ValueConverters have several uses: // -// * converting from the subset types as provided by the sql package +// * converting from the Value types as provided by the sql package // into a database table's specific column type and making sure it // fits, such as making sure a particular int64 fits in a // table's uint16 column. // // * converting a value as given from the database into one of the -// subset types. +// driver Value types. // -// * by the sql package, for converting from a driver's subset type +// * by the sql package, for converting from a driver's Value type // to a user's type in a scan. type ValueConverter interface { - // ConvertValue converts a value to a restricted subset type. - ConvertValue(v interface{}) (interface{}, error) + // ConvertValue converts a value to a driver Value. + ConvertValue(v interface{}) (Value, error) } -// SubsetValuer is the interface providing the SubsetValue method. +// Valuer is the interface providing the Value 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) +// Types implementing Valuer interface are able to convert +// themselves to a driver Value. +type Valuer interface { + // Value returns a driver Value. + Value() (Value, error) } // Bool is a ValueConverter that converts input values to bools. @@ -59,7 +59,7 @@ var _ ValueConverter = boolType{} func (boolType) String() string { return "Bool" } -func (boolType) ConvertValue(src interface{}) (interface{}, error) { +func (boolType) ConvertValue(src interface{}) (Value, error) { switch s := src.(type) { case bool: return s, nil @@ -104,7 +104,7 @@ type int32Type struct{} var _ ValueConverter = int32Type{} -func (int32Type) ConvertValue(v interface{}) (interface{}, error) { +func (int32Type) ConvertValue(v interface{}) (Value, error) { rv := reflect.ValueOf(v) switch rv.Kind() { case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: @@ -137,7 +137,7 @@ var String stringType type stringType struct{} -func (stringType) ConvertValue(v interface{}) (interface{}, error) { +func (stringType) ConvertValue(v interface{}) (Value, error) { switch v.(type) { case string, []byte: return v, nil @@ -151,7 +151,7 @@ type Null struct { Converter ValueConverter } -func (n Null) ConvertValue(v interface{}) (interface{}, error) { +func (n Null) ConvertValue(v interface{}) (Value, error) { if v == nil { return nil, nil } @@ -164,28 +164,17 @@ type NotNull struct { Converter ValueConverter } -func (n NotNull) ConvertValue(v interface{}) (interface{}, error) { +func (n NotNull) ConvertValue(v interface{}) (Value, 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: -// -// int64 -// float64 -// bool -// nil -// []byte -// time.Time -// string -// -// This is the same list as IsScanSubsetType, with the addition of -// string. -func IsParameterSubsetType(v interface{}) bool { - if IsScanSubsetType(v) { +// IsValue reports whether v is a valid Value parameter type. +// Unlike IsScanValue, IsValue permits the string type. +func IsValue(v interface{}) bool { + if IsScanValue(v) { return true } if _, ok := v.(string); ok { @@ -194,18 +183,9 @@ func IsParameterSubsetType(v interface{}) bool { return false } -// IsScanSubsetType reports whether v is of a valid type for a -// value populated by Rows.Next. These types are: -// -// int64 -// float64 -// bool -// nil -// []byte -// time.Time -// -// This is the same list as IsParameterSubsetType, without string. -func IsScanSubsetType(v interface{}) bool { +// IsScanValue reports whether v is a valid Value scan type. +// Unlike IsValue, IsScanValue does not permit the string type. +func IsScanValue(v interface{}) bool { if v == nil { return true } @@ -221,7 +201,7 @@ func IsScanSubsetType(v interface{}) bool { // ColumnConverter. // // DefaultParameterConverter returns the given value directly if -// IsSubsetType(value). Otherwise integer type are converted to +// IsValue(value). Otherwise integer type are converted to // int64, floats to float64, and strings to []byte. Other types are // an error. var DefaultParameterConverter defaultConverter @@ -230,18 +210,18 @@ type defaultConverter struct{} var _ ValueConverter = defaultConverter{} -func (defaultConverter) ConvertValue(v interface{}) (interface{}, error) { - if IsParameterSubsetType(v) { +func (defaultConverter) ConvertValue(v interface{}) (Value, error) { + if IsValue(v) { return v, nil } - if svi, ok := v.(SubsetValuer); ok { - sv, err := svi.SubsetValue() + if svi, ok := v.(Valuer); ok { + sv, err := svi.Value() if err != nil { return nil, err } - if !IsParameterSubsetType(sv) { - return nil, fmt.Errorf("non-subset type %T returned from SubsetValue", sv) + if !IsValue(sv) { + return nil, fmt.Errorf("non-Value type %T returned from Value", sv) } return sv, nil } diff --git a/src/pkg/database/sql/fakedb_test.go b/src/pkg/database/sql/fakedb_test.go index 889e2a25232..fc63f03740a 100644 --- a/src/pkg/database/sql/fakedb_test.go +++ b/src/pkg/database/sql/fakedb_test.go @@ -217,7 +217,7 @@ func (c *fakeConn) Close() error { return nil } -func checkSubsetTypes(args []interface{}) error { +func checkSubsetTypes(args []driver.Value) error { for n, arg := range args { switch arg.(type) { case int64, float64, bool, nil, []byte, string, time.Time: @@ -228,7 +228,7 @@ func checkSubsetTypes(args []interface{}) error { return nil } -func (c *fakeConn) Exec(query string, args []interface{}) (driver.Result, error) { +func (c *fakeConn) Exec(query string, args []driver.Value) (driver.Result, error) { // This is an optional interface, but it's implemented here // just to check that all the args of of the proper types. // ErrSkip is returned so the caller acts as if we didn't @@ -379,7 +379,7 @@ func (s *fakeStmt) Close() error { var errClosed = errors.New("fakedb: statement has been closed") -func (s *fakeStmt) Exec(args []interface{}) (driver.Result, error) { +func (s *fakeStmt) Exec(args []driver.Value) (driver.Result, error) { if s.closed { return nil, errClosed } @@ -392,12 +392,12 @@ func (s *fakeStmt) Exec(args []interface{}) (driver.Result, error) { switch s.cmd { case "WIPE": db.wipe() - return driver.DDLSuccess, nil + return driver.ResultNoRows, nil case "CREATE": if err := db.createTable(s.table, s.colName, s.colType); err != nil { return nil, err } - return driver.DDLSuccess, nil + return driver.ResultNoRows, nil case "INSERT": return s.execInsert(args) } @@ -405,7 +405,7 @@ func (s *fakeStmt) Exec(args []interface{}) (driver.Result, error) { return nil, fmt.Errorf("unimplemented statement Exec command type of %q", s.cmd) } -func (s *fakeStmt) execInsert(args []interface{}) (driver.Result, error) { +func (s *fakeStmt) execInsert(args []driver.Value) (driver.Result, error) { db := s.c.db if len(args) != s.placeholders { panic("error in pkg db; should only get here if size is correct") @@ -441,7 +441,7 @@ func (s *fakeStmt) execInsert(args []interface{}) (driver.Result, error) { return driver.RowsAffected(1), nil } -func (s *fakeStmt) Query(args []interface{}) (driver.Rows, error) { +func (s *fakeStmt) Query(args []driver.Value) (driver.Rows, error) { if s.closed { return nil, errClosed } @@ -548,7 +548,7 @@ func (rc *rowsCursor) Columns() []string { return rc.cols } -func (rc *rowsCursor) Next(dest []interface{}) error { +func (rc *rowsCursor) Next(dest []driver.Value) error { if rc.closed { return errors.New("fakedb: cursor is closed") } diff --git a/src/pkg/database/sql/sql.go b/src/pkg/database/sql/sql.go index f14a98c3cf2..62b551d89b5 100644 --- a/src/pkg/database/sql/sql.go +++ b/src/pkg/database/sql/sql.go @@ -62,8 +62,8 @@ func (ns *NullString) Scan(value interface{}) error { return convertAssign(&ns.String, value) } -// SubsetValue implements the driver SubsetValuer interface. -func (ns NullString) SubsetValue() (interface{}, error) { +// Value implements the driver Valuer interface. +func (ns NullString) Value() (driver.Value, error) { if !ns.Valid { return nil, nil } @@ -88,8 +88,8 @@ func (n *NullInt64) Scan(value interface{}) error { return convertAssign(&n.Int64, value) } -// SubsetValue implements the driver SubsetValuer interface. -func (n NullInt64) SubsetValue() (interface{}, error) { +// Value implements the driver Valuer interface. +func (n NullInt64) Value() (driver.Value, error) { if !n.Valid { return nil, nil } @@ -114,8 +114,8 @@ func (n *NullFloat64) Scan(value interface{}) error { return convertAssign(&n.Float64, value) } -// SubsetValue implements the driver SubsetValuer interface. -func (n NullFloat64) SubsetValue() (interface{}, error) { +// Value implements the driver Valuer interface. +func (n NullFloat64) Value() (driver.Value, error) { if !n.Valid { return nil, nil } @@ -140,8 +140,8 @@ func (n *NullBool) Scan(value interface{}) error { return convertAssign(&n.Bool, value) } -// SubsetValue implements the driver SubsetValuer interface. -func (n NullBool) SubsetValue() (interface{}, error) { +// Value implements the driver Valuer interface. +func (n NullBool) Value() (driver.Value, error) { if !n.Valid { return nil, nil } @@ -523,8 +523,13 @@ func (tx *Tx) Exec(query string, args ...interface{}) (Result, error) { } defer tx.releaseConn() + sargs, err := subsetTypeArgs(args) + if err != nil { + return nil, err + } + if execer, ok := ci.(driver.Execer); ok { - resi, err := execer.Exec(query, args) + resi, err := execer.Exec(query, sargs) if err == nil { return result{resi}, nil } @@ -539,11 +544,6 @@ func (tx *Tx) Exec(query string, args ...interface{}) (Result, error) { } defer sti.Close() - sargs, err := subsetTypeArgs(args) - if err != nil { - return nil, err - } - resi, err := sti.Exec(sargs) if err != nil { return nil, err @@ -618,19 +618,21 @@ func (s *Stmt) Exec(args ...interface{}) (Result, error) { return nil, fmt.Errorf("sql: expected %d arguments, got %d", want, len(args)) } + sargs := make([]driver.Value, len(args)) + // 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 svi, ok := arg.(driver.Valuer); ok { + sv, err := svi.Value() if err != nil { - return nil, fmt.Errorf("sql: argument index %d from SubsetValue: %v", n, err) + return nil, fmt.Errorf("sql: argument index %d from Value: %v", n, err) } - if !driver.IsParameterSubsetType(sv) { - return nil, fmt.Errorf("sql: argument index %d: non-subset type %T returned from SubsetValue", n, sv) + if !driver.IsValue(sv) { + return nil, fmt.Errorf("sql: argument index %d: non-subset type %T returned from Value", n, sv) } arg = sv } @@ -642,25 +644,25 @@ func (s *Stmt) Exec(args ...interface{}) (Result, error) { // 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) + sargs[n], err = cc.ColumnConverter(n).ConvertValue(arg) if err != nil { return nil, fmt.Errorf("sql: converting Exec argument #%d's type: %v", n, err) } - if !driver.IsParameterSubsetType(args[n]) { + if !driver.IsValue(sargs[n]) { return nil, fmt.Errorf("sql: driver ColumnConverter error converted %T to unsupported type %T", - arg, args[n]) + arg, sargs[n]) } } } else { for n, arg := range args { - args[n], err = driver.DefaultParameterConverter.ConvertValue(arg) + sargs[n], err = driver.DefaultParameterConverter.ConvertValue(arg) if err != nil { return nil, fmt.Errorf("sql: converting Exec argument #%d's type: %v", n, err) } } } - resi, err := si.Exec(args) + resi, err := si.Exec(sargs) if err != nil { return nil, err } @@ -829,7 +831,7 @@ type Rows struct { rowsi driver.Rows closed bool - lastcols []interface{} + lastcols []driver.Value lasterr error closeStmt *Stmt // if non-nil, statement to Close on close } @@ -846,7 +848,7 @@ func (rs *Rows) Next() bool { return false } if rs.lastcols == nil { - rs.lastcols = make([]interface{}, len(rs.rowsi.Columns())) + rs.lastcols = make([]driver.Value, len(rs.rowsi.Columns())) } rs.lasterr = rs.rowsi.Next(rs.lastcols) if rs.lasterr == io.EOF {