code review 5674084: database/sql/driver: API cleanups

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 issue 2842

[bradfitz, 2012-02-17 05:00:25]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/

[r, 2012-02-17 05:06:41]

LGTM but wait for others

http://codereview.appspot.com/5674084/diff/3001/src/pkg/database/sql/driver/d...
File src/pkg/database/sql/driver/driver.go (right):

http://codereview.appspot.com/5674084/diff/3001/src/pkg/database/sql/driver/d...
src/pkg/database/sql/driver/driver.go:13: // A driver Value is a value that
drivers must be able to handle.
s/driver //

[rsc, 2012-02-17 16:28:38]

LGTM

http://codereview.appspot.com/5674084/diff/3001/src/pkg/database/sql/driver/d...
File src/pkg/database/sql/driver/driver.go (right):

http://codereview.appspot.com/5674084/diff/3001/src/pkg/database/sql/driver/d...
src/pkg/database/sql/driver/driver.go:189: // DDLSuccess is a pre-defined Result
for drivers to return when a DDL
What is DDL?

http://codereview.appspot.com/5674084/diff/3001/src/pkg/database/sql/driver/t...
File src/pkg/database/sql/driver/types.go (right):

http://codereview.appspot.com/5674084/diff/3001/src/pkg/database/sql/driver/t...
src/pkg/database/sql/driver/types.go:30: type ValueConverter interface {
Assume func conversion is later.

http://codereview.appspot.com/5674084/diff/3001/src/pkg/database/sql/driver/t...
src/pkg/database/sql/driver/types.go:38: // themselves to one of the driver's
allowed subset values.
subset

http://codereview.appspot.com/5674084/diff/3001/src/pkg/database/sql/driver/t...
src/pkg/database/sql/driver/types.go:40: // Value returns a driver parameter
subset value.
subset

http://codereview.appspot.com/5674084/diff/3001/src/pkg/database/sql/driver/t...
src/pkg/database/sql/driver/types.go:41: Value() (interface{}, error)
s/interface{}/Value/

http://codereview.appspot.com/5674084/diff/3001/src/pkg/database/sql/driver/t...
src/pkg/database/sql/driver/types.go:185: // This is the same list as
IsScanValue, with the addition of
Is this the same list as on the type Value doc?  If so, delete?

http://codereview.appspot.com/5674084/diff/3001/src/pkg/database/sql/driver/t...
src/pkg/database/sql/driver/types.go:187: func IsValue(v Value) bool {
s/Value/interface{}/

Presumably something with static type Value should already be known to be a
Value.  I realize the compiler's not enforcing that, but even so, you're passing
an arbitrary thing here, not a driver.Value.

http://codereview.appspot.com/5674084/diff/3001/src/pkg/database/sql/driver/t...
src/pkg/database/sql/driver/types.go:208: func IsScanValue(v Value) bool {
s/Value/interface{}/

http://codereview.appspot.com/5674084/diff/3001/src/pkg/database/sql/driver/t...
src/pkg/database/sql/driver/types.go:244: return nil, fmt.Errorf("non-subset
type %T returned from Value", sv)
subset

[bradfitz, 2012-02-20 03:03:17]

Hello golang-dev@googlegroups.com, r@golang.org, rsc@golang.org (cc:
golang-dev@googlegroups.com),

Please take another look.

[bradfitz, 2012-02-20 03:05:24]

I think this is good now.  Before I submit, though, I've changed the
disliked DDLSuccess to ResultNoRows, as follows:  (is this cool?)


// 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 noRows struct{}

var _ Result = noRows{}

func (noRows) LastInsertId() (int64, error) {
        return 0, errors.New("no LastInsertId available after DDL
statement")
}

func (noRows) RowsAffected() (int64, error) {
        return 0, errors.New("no RowsAffected available after DDL
statement")
}


On Mon, Feb 20, 2012 at 2:03 PM, <bradfitz@golang.org> wrote:

> Hello golang-dev@googlegroups.com, r@golang.org, rsc@golang.org (cc:
> golang-dev@googlegroups.com),
>
> Please take another look.
>
>
>
http://codereview.appspot.com/**5674084/<http://codereview.appspot.com/5674084/>
>

[bradfitz, 2012-02-20 03:12:50]

Russ LGTM'd on chat.

On Mon, Feb 20, 2012 at 2:05 PM, Brad Fitzpatrick <bradfitz@golang.org>wrote:

> I think this is good now.  Before I submit, though, I've changed the
> disliked DDLSuccess to ResultNoRows, as follows:  (is this cool?)
>
>
> // 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 noRows struct{}
>
> var _ Result = noRows{}
>
> func (noRows) LastInsertId() (int64, error) {
>         return 0, errors.New("no LastInsertId available after DDL
> statement")
> }
>
> func (noRows) RowsAffected() (int64, error) {
>         return 0, errors.New("no RowsAffected available after DDL
> statement")
> }
>
>
> On Mon, Feb 20, 2012 at 2:03 PM, <bradfitz@golang.org> wrote:
>
>> Hello golang-dev@googlegroups.com, r@golang.org, rsc@golang.org (cc:
>> golang-dev@googlegroups.com),
>>
>> Please take another look.
>>
>>
>>
http://codereview.appspot.com/**5674084/<http://codereview.appspot.com/5674084/>
>>
>
>

[bradfitz, 2012-02-20 03:25:35]

*** Submitted as http://code.google.com/p/go/source/detail?r=2c15f43065c1 ***

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 issue 2842

R=golang-dev, r, rsc
CC=golang-dev
http://codereview.appspot.com/5674084