code review 43510044: database/sql: let drivers access their own driver.Rows.

database/sql: let drivers access their own driver.Rows.

When a driver implementing driver.Inspector is registered, it receives
a function wich unwrapps the drivers native Rows type from the internal database/sql
representation.
For some database systems, this enables access to richer metadata like e.g. column types,originating tables of a column or a columns charset.

Fixes issue 5606.

[ahormann, 2013-12-18 08:06:34]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[bradfitz, 2013-12-18 11:47:07]

The name and all the empty interfaces in the signature don't look great.

The docs also don't say why I'd care to extract a driver.Rows, or what I
should return, or when this would even be called.



On Wed, Dec 18, 2013 at 12:06 AM, <arnehormann@gmail.com> wrote:

> Reviewers: golang-dev1,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> database/sql: let drivers access their own driver.Rows.
>
> When a driver implementing driver.Inspector is registered, it receives
> a function wich unwrapps the drivers native Rows type from the internal
> database/sql
> representation.
> For some database systems, this enables access to richer metadata like
> e.g. column types,originating tables of a column or a columns charset.
>
> Fixes issue 5606.
>
> Please review this at https://codereview.appspot.com/43510044/
>
> Affected files (+59, -5 lines):
>   M src/pkg/database/sql/driver/driver.go
>   M src/pkg/database/sql/fakedb_test.go
>   M src/pkg/database/sql/sql.go
>   M src/pkg/database/sql/sql_test.go
>
>
> Index: src/pkg/database/sql/driver/driver.go
> ===================================================================
> --- a/src/pkg/database/sql/driver/driver.go
> +++ b/src/pkg/database/sql/driver/driver.go
> @@ -54,6 +54,14 @@
>  // you shouldn't return ErrBadConn.
>  var ErrBadConn = errors.New("driver: bad connection")
>
> +// Inspector is an optional interface that may be implemented by a Driver.
> +type Inspector interface {
> +
> +       // SetInspect provides the Driver with a function to extract
> +       // driver.Rows from *sql.Rows or *sql.Row.
> +       SetInspect(inspect func(interface{}) (interface{}, error))
> +}
> +
>  // Execer is an optional interface that may be implemented by a Conn.
>  //
>  // If a Conn does not implement Execer, the sql package's DB.Exec will
> Index: src/pkg/database/sql/fakedb_test.go
> ===================================================================
> --- a/src/pkg/database/sql/fakedb_test.go
> +++ b/src/pkg/database/sql/fakedb_test.go
> @@ -120,7 +120,16 @@
>         placeholderConverter []driver.ValueConverter // used by INSERT
>  }
>
> -var fdriver driver.Driver = &fakeDriver{}
> +var inspectStruct func(interface{}) (interface{}, error)
> +
> +func (d *fakeDriver) SetInspect(inspect func(interface{}) (interface{},
> error)) {
> +       inspectStruct = inspect
> +}
> +
> +var fdriver = &fakeDriver{}
> +
> +var _ driver.Inspector = fdriver
> +var _ driver.Driver = fdriver
>
>  func init() {
>         Register("test", fdriver)
> @@ -259,7 +268,6 @@
>  }
>
>  func (c *fakeConn) Close() (err error) {
> -       drv := fdriver.(*fakeDriver)
>         defer func() {
>                 if err != nil && testStrictClose != nil {
>                         testStrictClose.Errorf("failed to close a test
> fakeConn: %v", err)
> @@ -271,9 +279,9 @@
>                         fn(c, err)
>                 }
>                 if err == nil {
> -                       drv.mu.Lock()
> -                       drv.closeCount++
> -                       drv.mu.Unlock()
> +                       fdriver.mu.Lock()
> +                       fdriver.closeCount++
> +                       fdriver.mu.Unlock()
>                 }
>         }()
>         if c.currTx != nil {
> Index: src/pkg/database/sql/sql.go
> ===================================================================
> --- a/src/pkg/database/sql/sql.go
> +++ b/src/pkg/database/sql/sql.go
> @@ -24,6 +24,10 @@
>
>  var drivers = make(map[string]driver.Driver)
>
> +// defined to avoid renaming the second parameter in Register
> +// which overlays the package name "driver"
> +type inspector driver.Inspector
> +
>  // Register makes a database driver available by the provided name.
>  // If Register is called twice with the same name or if driver is nil,
>  // it panics.
> @@ -35,6 +39,23 @@
>                 panic("sql: Register called twice for driver " + name)
>         }
>         drivers[name] = driver
> +       if insp, ok := driver.(inspector); ok {
> +               insp.SetInspect(inspect)
> +       }
> +}
> +
> +func inspect(sqlStruct interface{}) (interface{}, error) {
> +       switch unwrapped := sqlStruct.(type) {
> +       case *Row:
> +               rows := unwrapped.rows
> +               if rows == nil {
> +                       return nil, unwrapped.err
> +               }
> +               return rows.rowsi, nil
> +       case *Rows:
> +               return unwrapped.rowsi, nil
> +       }
> +       return nil, errors.New("sql: expected *Rows or *Row")
>  }
>
>  // RawBytes is a byte slice that holds a reference to memory owned by
> Index: src/pkg/database/sql/sql_test.go
> ===================================================================
> --- a/src/pkg/database/sql/sql_test.go
> +++ b/src/pkg/database/sql/sql_test.go
> @@ -1861,6 +1861,23 @@
>         wg.Wait()
>  }
>
> +func TestInspect(t *testing.T) {
> +       if inspectStruct == nil {
> +               t.Errorf("inspect was not set")
> +       }
> +       nativeRow := &rowsCursor{}
> +       sqlRows := &Rows{rowsi: nativeRow}
> +       sqlRow := &Row{rows: sqlRows}
> +       rows, err := inspectStruct(sqlRows)
> +       if err != nil || rows != nativeRow {
> +               t.Errorf("inspect failed for *Rows")
> +       }
> +       rows, err = inspectStruct(sqlRow)
> +       if err != nil || rows != nativeRow {
> +               t.Errorf("inspect failed for *Row")
> +       }
> +}
> +
>  func BenchmarkConcurrentDBExec(b *testing.B) {
>         b.ReportAllocs()
>         ct := new(concurrentDBExecTest)
>
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-dev+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>

[ahormann, 2013-12-18 12:02:17]

Am Mittwoch, 18. Dezember 2013 12:47:05 UTC+1 schrieb Brad Fitzpatrick:
>
> The name and all the empty interfaces in the signature don't look great.
>

I'm open to a better name, but I don't see a good way around the interfaces.
The parameter has to be interface because there's nothing else *Row and 
*Rows can be passed to.
The returned interface could be changed to driver.Rows, but the function 
could also be extended to unwrap statements and transactions later.
Even if driver.Rows would be returned, a type assertion to the drivers 
native type would still be needed, so there's not that much of a benefit.

 

> The docs also don't say why I'd care to extract a driver.Rows, or what I 
> should return, or when this would even be called.
>

I'll take a shot at better describing it, but I'd be really glad for 
pointers or noteworthy examples of how to do it right.

 

>
>
>
> On Wed, Dec 18, 2013 at 12:06 AM, <arneh...@gmail.com <javascript:>>wrote:
>
>> Reviewers: golang-dev1,
>>
>> Message:
>> Hello golan...@googlegroups.com <javascript:>,
>>
>> I'd like you to review this change to
>> https://code.google.com/p/go
>>
>>
>> Description:
>> database/sql: let drivers access their own driver.Rows.
>>
>> When a driver implementing driver.Inspector is registered, it receives
>> a function wich unwrapps the drivers native Rows type from the internal
>> database/sql
>> representation.
>> For some database systems, this enables access to richer metadata like
>> e.g. column types,originating tables of a column or a columns charset.
>>
>> Fixes issue 5606.
>>
>> Please review this at https://codereview.appspot.com/43510044/
>>
>> Affected files (+59, -5 lines):
>>   M src/pkg/database/sql/driver/driver.go
>>   M src/pkg/database/sql/fakedb_test.go
>>   M src/pkg/database/sql/sql.go
>>   M src/pkg/database/sql/sql_test.go
>>
>>
>> Index: src/pkg/database/sql/driver/driver.go
>> ===================================================================
>> --- a/src/pkg/database/sql/driver/driver.go
>> +++ b/src/pkg/database/sql/driver/driver.go
>> @@ -54,6 +54,14 @@
>>  // you shouldn't return ErrBadConn.
>>  var ErrBadConn = errors.New("driver: bad connection")
>>
>> +// Inspector is an optional interface that may be implemented by a 
>> Driver.
>> +type Inspector interface {
>> +
>> +       // SetInspect provides the Driver with a function to extract
>> +       // driver.Rows from *sql.Rows or *sql.Row.
>> +       SetInspect(inspect func(interface{}) (interface{}, error))
>> +}
>> +
>>  // Execer is an optional interface that may be implemented by a Conn.
>>  //
>>  // If a Conn does not implement Execer, the sql package's DB.Exec will
>> Index: src/pkg/database/sql/fakedb_test.go
>> ===================================================================
>> --- a/src/pkg/database/sql/fakedb_test.go
>> +++ b/src/pkg/database/sql/fakedb_test.go
>> @@ -120,7 +120,16 @@
>>         placeholderConverter []driver.ValueConverter // used by INSERT
>>  }
>>
>> -var fdriver driver.Driver = &fakeDriver{}
>> +var inspectStruct func(interface{}) (interface{}, error)
>> +
>> +func (d *fakeDriver) SetInspect(inspect func(interface{}) (interface{}, 
>> error)) {
>> +       inspectStruct = inspect
>> +}
>> +
>> +var fdriver = &fakeDriver{}
>> +
>> +var _ driver.Inspector = fdriver
>> +var _ driver.Driver = fdriver
>>
>>  func init() {
>>         Register("test", fdriver)
>> @@ -259,7 +268,6 @@
>>  }
>>
>>  func (c *fakeConn) Close() (err error) {
>> -       drv := fdriver.(*fakeDriver)
>>         defer func() {
>>                 if err != nil && testStrictClose != nil {
>>                         testStrictClose.Errorf("failed to close a test 
>> fakeConn: %v", err)
>> @@ -271,9 +279,9 @@
>>                         fn(c, err)
>>                 }
>>                 if err == nil {
>> -                       drv.mu.Lock()
>> -                       drv.closeCount++
>> -                       drv.mu.Unlock()
>> +                       fdriver.mu.Lock()
>> +                       fdriver.closeCount++
>> +                       fdriver.mu.Unlock()
>>                 }
>>         }()
>>         if c.currTx != nil {
>> Index: src/pkg/database/sql/sql.go
>> ===================================================================
>> --- a/src/pkg/database/sql/sql.go
>> +++ b/src/pkg/database/sql/sql.go
>> @@ -24,6 +24,10 @@
>>
>>  var drivers = make(map[string]driver.Driver)
>>
>> +// defined to avoid renaming the second parameter in Register
>> +// which overlays the package name "driver"
>> +type inspector driver.Inspector
>> +
>>  // Register makes a database driver available by the provided name.
>>  // If Register is called twice with the same name or if driver is nil,
>>  // it panics.
>> @@ -35,6 +39,23 @@
>>                 panic("sql: Register called twice for driver " + name)
>>         }
>>         drivers[name] = driver
>> +       if insp, ok := driver.(inspector); ok {
>> +               insp.SetInspect(inspect)
>> +       }
>> +}
>> +
>> +func inspect(sqlStruct interface{}) (interface{}, error) {
>> +       switch unwrapped := sqlStruct.(type) {
>> +       case *Row:
>> +               rows := unwrapped.rows
>> +               if rows == nil {
>> +                       return nil, unwrapped.err
>> +               }
>> +               return rows.rowsi, nil
>> +       case *Rows:
>> +               return unwrapped.rowsi, nil
>> +       }
>> +       return nil, errors.New("sql: expected *Rows or *Row")
>>  }
>>
>>  // RawBytes is a byte slice that holds a reference to memory owned by
>> Index: src/pkg/database/sql/sql_test.go
>> ===================================================================
>> --- a/src/pkg/database/sql/sql_test.go
>> +++ b/src/pkg/database/sql/sql_test.go
>> @@ -1861,6 +1861,23 @@
>>         wg.Wait()
>>  }
>>
>> +func TestInspect(t *testing.T) {
>> +       if inspectStruct == nil {
>> +               t.Errorf("inspect was not set")
>> +       }
>> +       nativeRow := &rowsCursor{}
>> +       sqlRows := &Rows{rowsi: nativeRow}
>> +       sqlRow := &Row{rows: sqlRows}
>> +       rows, err := inspectStruct(sqlRows)
>> +       if err != nil || rows != nativeRow {
>> +               t.Errorf("inspect failed for *Rows")
>> +       }
>> +       rows, err = inspectStruct(sqlRow)
>> +       if err != nil || rows != nativeRow {
>> +               t.Errorf("inspect failed for *Row")
>> +       }
>> +}
>> +
>>  func BenchmarkConcurrentDBExec(b *testing.B) {
>>         b.ReportAllocs()
>>         ct := new(concurrentDBExecTest)
>>
>>
>> -- 
>>
>> ---You received this message because you are subscribed to the Google 
>> Groups "golang-dev" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to golang-dev+...@googlegroups.com <javascript:>.
>> For more options, visit https://groups.google.com/groups/opt_out.
>>
>
>

[bradfitz, 2013-12-18 12:04:34]

On Wed, Dec 18, 2013 at 4:02 AM, <arnehormann@gmail.com> wrote:

>
> Am Mittwoch, 18. Dezember 2013 12:47:05 UTC+1 schrieb Brad Fitzpatrick:
>
>> The name and all the empty interfaces in the signature don't look great.
>>
>
> I'm open to a better name, but I don't see a good way around the
> interfaces.
> The parameter has to be interface because there's nothing else *Row and
> *Rows can be passed to.
> The returned interface could be changed to driver.Rows, but the function
> could also be extended to unwrap statements and transactions later.
> Even if driver.Rows would be returned, a type assertion to the drivers
> native type would still be needed, so there's not that much of a benefit.
>

The benefit is documentation.

Note how the driver package uses the Value type (
http://golang.org/pkg/database/sql/driver/#Value) everywhere in its API
even though it's "useless".



>
>
>
>> The docs also don't say why I'd care to extract a driver.Rows, or what I
>> should return, or when this would even be called.
>>
>
> I'll take a shot at better describing it, but I'd be really glad for
> pointers or noteworthy examples of how to do it right.
>
>
>
>>
>>
>>
>> On Wed, Dec 18, 2013 at 12:06 AM, <arneh...@gmail.com> wrote:
>>
>>> Reviewers: golang-dev1,
>>>
>>> Message:
>>> Hello golan...@googlegroups.com,
>>>
>>>
>>> I'd like you to review this change to
>>> https://code.google.com/p/go
>>>
>>>
>>> Description:
>>> database/sql: let drivers access their own driver.Rows.
>>>
>>> When a driver implementing driver.Inspector is registered, it receives
>>> a function wich unwrapps the drivers native Rows type from the internal
>>> database/sql
>>> representation.
>>> For some database systems, this enables access to richer metadata like
>>> e.g. column types,originating tables of a column or a columns charset.
>>>
>>> Fixes issue 5606.
>>>
>>> Please review this at https://codereview.appspot.com/43510044/
>>>
>>> Affected files (+59, -5 lines):
>>>   M src/pkg/database/sql/driver/driver.go
>>>   M src/pkg/database/sql/fakedb_test.go
>>>   M src/pkg/database/sql/sql.go
>>>   M src/pkg/database/sql/sql_test.go
>>>
>>>
>>> Index: src/pkg/database/sql/driver/driver.go
>>> ===================================================================
>>> --- a/src/pkg/database/sql/driver/driver.go
>>> +++ b/src/pkg/database/sql/driver/driver.go
>>> @@ -54,6 +54,14 @@
>>>  // you shouldn't return ErrBadConn.
>>>  var ErrBadConn = errors.New("driver: bad connection")
>>>
>>> +// Inspector is an optional interface that may be implemented by a
>>> Driver.
>>> +type Inspector interface {
>>> +
>>> +       // SetInspect provides the Driver with a function to extract
>>> +       // driver.Rows from *sql.Rows or *sql.Row.
>>> +       SetInspect(inspect func(interface{}) (interface{}, error))
>>> +}
>>> +
>>>  // Execer is an optional interface that may be implemented by a Conn.
>>>  //
>>>  // If a Conn does not implement Execer, the sql package's DB.Exec will
>>> Index: src/pkg/database/sql/fakedb_test.go
>>> ===================================================================
>>> --- a/src/pkg/database/sql/fakedb_test.go
>>> +++ b/src/pkg/database/sql/fakedb_test.go
>>> @@ -120,7 +120,16 @@
>>>         placeholderConverter []driver.ValueConverter // used by INSERT
>>>  }
>>>
>>> -var fdriver driver.Driver = &fakeDriver{}
>>> +var inspectStruct func(interface{}) (interface{}, error)
>>> +
>>> +func (d *fakeDriver) SetInspect(inspect func(interface{}) (interface{},
>>> error)) {
>>> +       inspectStruct = inspect
>>> +}
>>> +
>>> +var fdriver = &fakeDriver{}
>>> +
>>> +var _ driver.Inspector = fdriver
>>> +var _ driver.Driver = fdriver
>>>
>>>  func init() {
>>>         Register("test", fdriver)
>>> @@ -259,7 +268,6 @@
>>>  }
>>>
>>>  func (c *fakeConn) Close() (err error) {
>>> -       drv := fdriver.(*fakeDriver)
>>>         defer func() {
>>>                 if err != nil && testStrictClose != nil {
>>>                         testStrictClose.Errorf("failed to close a test
>>> fakeConn: %v", err)
>>> @@ -271,9 +279,9 @@
>>>                         fn(c, err)
>>>                 }
>>>                 if err == nil {
>>> -                       drv.mu.Lock()
>>> -                       drv.closeCount++
>>> -                       drv.mu.Unlock()
>>> +                       fdriver.mu.Lock()
>>> +                       fdriver.closeCount++
>>> +                       fdriver.mu.Unlock()
>>>                 }
>>>         }()
>>>         if c.currTx != nil {
>>> Index: src/pkg/database/sql/sql.go
>>> ===================================================================
>>> --- a/src/pkg/database/sql/sql.go
>>> +++ b/src/pkg/database/sql/sql.go
>>> @@ -24,6 +24,10 @@
>>>
>>>  var drivers = make(map[string]driver.Driver)
>>>
>>> +// defined to avoid renaming the second parameter in Register
>>> +// which overlays the package name "driver"
>>> +type inspector driver.Inspector
>>> +
>>>  // Register makes a database driver available by the provided name.
>>>  // If Register is called twice with the same name or if driver is nil,
>>>  // it panics.
>>> @@ -35,6 +39,23 @@
>>>                 panic("sql: Register called twice for driver " + name)
>>>         }
>>>         drivers[name] = driver
>>> +       if insp, ok := driver.(inspector); ok {
>>> +               insp.SetInspect(inspect)
>>> +       }
>>> +}
>>> +
>>> +func inspect(sqlStruct interface{}) (interface{}, error) {
>>> +       switch unwrapped := sqlStruct.(type) {
>>> +       case *Row:
>>> +               rows := unwrapped.rows
>>> +               if rows == nil {
>>> +                       return nil, unwrapped.err
>>> +               }
>>> +               return rows.rowsi, nil
>>> +       case *Rows:
>>> +               return unwrapped.rowsi, nil
>>> +       }
>>> +       return nil, errors.New("sql: expected *Rows or *Row")
>>>  }
>>>
>>>  // RawBytes is a byte slice that holds a reference to memory owned by
>>> Index: src/pkg/database/sql/sql_test.go
>>> ===================================================================
>>> --- a/src/pkg/database/sql/sql_test.go
>>> +++ b/src/pkg/database/sql/sql_test.go
>>> @@ -1861,6 +1861,23 @@
>>>         wg.Wait()
>>>  }
>>>
>>> +func TestInspect(t *testing.T) {
>>> +       if inspectStruct == nil {
>>> +               t.Errorf("inspect was not set")
>>> +       }
>>> +       nativeRow := &rowsCursor{}
>>> +       sqlRows := &Rows{rowsi: nativeRow}
>>> +       sqlRow := &Row{rows: sqlRows}
>>> +       rows, err := inspectStruct(sqlRows)
>>> +       if err != nil || rows != nativeRow {
>>> +               t.Errorf("inspect failed for *Rows")
>>> +       }
>>> +       rows, err = inspectStruct(sqlRow)
>>> +       if err != nil || rows != nativeRow {
>>> +               t.Errorf("inspect failed for *Row")
>>> +       }
>>> +}
>>> +
>>>  func BenchmarkConcurrentDBExec(b *testing.B) {
>>>         b.ReportAllocs()
>>>         ct := new(concurrentDBExecTest)
>>>
>>>
>>>
>>> --
>>>
>>> ---You received this message because you are subscribed to the Google
>>> Groups "golang-dev" group.
>>> To unsubscribe from this group and stop receiving emails from it, send
>>> an email to golang-dev+...@googlegroups.com.
>>>
>>> For more options, visit https://groups.google.com/groups/opt_out.
>>>
>>
>>  --
>
> ---
> You received this message because you are subscribed to the Google Groups
> "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-dev+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>

[ahormann, 2013-12-19 08:29:17]

PTAL

On 18.12 13:04, Brad Fitzpatrick wrote:
>
>
>
> On Wed, Dec 18, 2013 at 4:02 AM, <arnehormann@gmail.com 
> <mailto:arnehormann@gmail.com>> wrote:
>
>
>     Am Mittwoch, 18. Dezember 2013 12:47:05 UTC+1 schrieb Brad
>     Fitzpatrick:
>
>         The name and all the empty interfaces in the signature don't
>         look great.
>
>
>     I'm open to a better name, but I don't see a good way around the
>     interfaces.
>     The parameter has to be interface because there's nothing else
>     *Row and *Rows can be passed to.
>     The returned interface could be changed to driver.Rows, but the
>     function could also be extended to unwrap statements and
>     transactions later.
>     Even if driver.Rows would be returned, a type assertion to the
>     drivers native type would still be needed, so there's not that
>     much of a benefit.
>
>
> The benefit is documentation.
>
> Note how the driver package uses the Value type 
> (http://golang.org/pkg/database/sql/driver/#Value) everywhere in its 
> API even though it's "useless".
>
>
>         The docs also don't say why I'd care to extract a driver.Rows,
>         or what I should return, or when this would even be called.
>
>
>     I'll take a shot at better describing it, but I'd be really glad
>     for pointers or noteworthy examples of how to do it right.
>
>
>
>
>         On Wed, Dec 18, 2013 at 12:06 AM, <arneh...@gmail.com> wrote:
>
>             Reviewers: golang-dev1,
>
>             Message:
>             Hello golan...@googlegroups.com,
>
>
>             I'd like you to review this change to
>             https://code.google.com/p/go
>
>
>             Description:
>             database/sql: let drivers access their own driver.Rows.
>
>             When a driver implementing driver.Inspector is registered,
>             it receives
>             a function wich unwrapps the drivers native Rows type from
>             the internal
>             database/sql
>             representation.
>             For some database systems, this enables access to richer
>             metadata like
>             e.g. column types,originating tables of a column or a
>             columns charset.
>
>             Fixes issue 5606.
>
>             Please review this at https://codereview.appspot.com/43510044/
>
>             Affected files (+59, -5 lines):
>               M src/pkg/database/sql/driver/driver.go
>               M src/pkg/database/sql/fakedb_test.go
>               M src/pkg/database/sql/sql.go
>               M src/pkg/database/sql/sql_test.go
>
>
>             Index: src/pkg/database/sql/driver/driver.go
>            
===================================================================
>             --- a/src/pkg/database/sql/driver/driver.go
>             +++ b/src/pkg/database/sql/driver/driver.go
>             @@ -54,6 +54,14 @@
>              // you shouldn't return ErrBadConn.
>              var ErrBadConn = errors.New("driver: bad connection")
>
>             +// Inspector is an optional interface that may be
>             implemented by a Driver.
>             +type Inspector interface {
>             +
>             +       // SetInspect provides the Driver with a function
>             to extract
>             +       // driver.Rows from *sql.Rows or *sql.Row.
>             +       SetInspect(inspect func(interface{}) (interface{},
>             error))
>             +}
>             +
>              // Execer is an optional interface that may be
>             implemented by a Conn.
>              //
>              // If a Conn does not implement Execer, the sql package's
>             DB.Exec will
>             Index: src/pkg/database/sql/fakedb_test.go
>            
===================================================================
>             --- a/src/pkg/database/sql/fakedb_test.go
>             +++ b/src/pkg/database/sql/fakedb_test.go
>             @@ -120,7 +120,16 @@
>                     placeholderConverter []driver.ValueConverter //
>             used by INSERT
>              }
>
>             -var fdriver driver.Driver = &fakeDriver{}
>             +var inspectStruct func(interface{}) (interface{}, error)
>             +
>             +func (d *fakeDriver) SetInspect(inspect func(interface{})
>             (interface{}, error)) {
>             +       inspectStruct = inspect
>             +}
>             +
>             +var fdriver = &fakeDriver{}
>             +
>             +var _ driver.Inspector = fdriver
>             +var _ driver.Driver = fdriver
>
>              func init() {
>                     Register("test", fdriver)
>             @@ -259,7 +268,6 @@
>              }
>
>              func (c *fakeConn) Close() (err error) {
>             -       drv := fdriver.(*fakeDriver)
>                     defer func() {
>                             if err != nil && testStrictClose != nil {
>             testStrictClose.Errorf("failed to close a test fakeConn:
>             %v", err)
>             @@ -271,9 +279,9 @@
>                                     fn(c, err)
>                             }
>                             if err == nil {
>             -                       drv.mu.Lock()
>             -                       drv.closeCount++
>             -                       drv.mu.Unlock()
>             +                       fdriver.mu.Lock()
>             +                       fdriver.closeCount++
>             +                       fdriver.mu.Unlock()
>                             }
>                     }()
>                     if c.currTx != nil {
>             Index: src/pkg/database/sql/sql.go
>            
===================================================================
>             --- a/src/pkg/database/sql/sql.go
>             +++ b/src/pkg/database/sql/sql.go
>             @@ -24,6 +24,10 @@
>
>              var drivers = make(map[string]driver.Driver)
>
>             +// defined to avoid renaming the second parameter in Register
>             +// which overlays the package name "driver"
>             +type inspector driver.Inspector
>             +
>              // Register makes a database driver available by the
>             provided name.
>              // If Register is called twice with the same name or if
>             driver is nil,
>              // it panics.
>             @@ -35,6 +39,23 @@
>                             panic("sql: Register called twice for
>             driver " + name)
>                     }
>                     drivers[name] = driver
>             +       if insp, ok := driver.(inspector); ok {
>             +               insp.SetInspect(inspect)
>             +       }
>             +}
>             +
>             +func inspect(sqlStruct interface{}) (interface{}, error) {
>             +       switch unwrapped := sqlStruct.(type) {
>             +       case *Row:
>             +               rows := unwrapped.rows
>             +               if rows == nil {
>             +                       return nil, unwrapped.err
>             +               }
>             +               return rows.rowsi, nil
>             +       case *Rows:
>             +               return unwrapped.rowsi, nil
>             +       }
>             +       return nil, errors.New("sql: expected *Rows or *Row")
>              }
>
>              // RawBytes is a byte slice that holds a reference to
>             memory owned by
>             Index: src/pkg/database/sql/sql_test.go
>            
===================================================================
>             --- a/src/pkg/database/sql/sql_test.go
>             +++ b/src/pkg/database/sql/sql_test.go
>             @@ -1861,6 +1861,23 @@
>                     wg.Wait()
>              }
>
>             +func TestInspect(t *testing.T) {
>             +       if inspectStruct == nil {
>             +               t.Errorf("inspect was not set")
>             +       }
>             +       nativeRow := &rowsCursor{}
>             +       sqlRows := &Rows{rowsi: nativeRow}
>             +       sqlRow := &Row{rows: sqlRows}
>             +       rows, err := inspectStruct(sqlRows)
>             +       if err != nil || rows != nativeRow {
>             +               t.Errorf("inspect failed for *Rows")
>             +       }
>             +       rows, err = inspectStruct(sqlRow)
>             +       if err != nil || rows != nativeRow {
>             +               t.Errorf("inspect failed for *Row")
>             +       }
>             +}
>             +
>              func BenchmarkConcurrentDBExec(b *testing.B) {
>                     b.ReportAllocs()
>                     ct := new(concurrentDBExecTest)
>
>
>
>             -- 
>
>             ---You received this message because you are subscribed to
>             the Google Groups "golang-dev" group.
>             To unsubscribe from this group and stop receiving emails
>             from it, send an email to golang-dev+...@googlegroups.com.
>
>             For more options, visit
>             https://groups.google.com/groups/opt_out.
>
>
>     -- 
>
>     ---
>     You received this message because you are subscribed to the Google
>     Groups "golang-dev" group.
>     To unsubscribe from this group and stop receiving emails from it,
>     send an email to golang-dev+unsubscribe@googlegroups.com
>     <mailto:golang-dev%2Bunsubscribe@googlegroups.com>.
>     For more options, visit https://groups.google.com/groups/opt_out.
>
>

[gobot, 2013-12-20 16:26:16]

Replacing golang-dev with golang-codereviews.

[ahormann, 2014-04-03 08:33:00]

ping

A recap:

Currently, database/sql provides access to column names with Columns.
The drivers may have access to additional information, e.g. the column type,
creation comment, character set and collation, precision for decimal types, ...

After the driver's native row iterator (driver.Rows) is stored in sql.Rows or
sql.Row, the available information except for the names cannot be accessed any
longer.

It is still needed for tools working with tables or queries only known at
runtime.

Use cases:
 * derive the SQL needed for ALTER TABLE statements when a column should be
changed
 * use the closest matching type when data is serialized (e.g. numbers in JSON)
 * format output based on the type
 * generate code to efficiently scan into structs

Applying this CL allows the drivers to provide access to all available column
information.
The major change is that they have to be imported by name:

  import "github.com/go-sql-driver/mysql"
  // ...
  var cols []mysql.Column
  var err error
  cols, err = mysql.Columns(rows)

The drivers may add new capabilities on their own - but to their own interface.
database/sql will not need any changes to support that.

[bradfitz, 2014-04-10 16:01:57]

Sorry, we're too late in the Go 1.3 process for this. If you want to see this go
in, begin discussions early in the Go 1.4 cycle, once you see Go 1.3 released
around June 1st.

[ahormann, 2014-04-10 16:14:59]

On 2014/04/10 16:01:57, bradfitz wrote:
> Sorry, we're too late in the Go 1.3 process for this. If you want to see this
go
> in, begin discussions early in the Go 1.4 cycle, once you see Go 1.3 released
> around June 1st.

I discussed it - but on golang-nuts instead of golang-dev (newbie mistake). That
was inMay last year.
I also thought CLs submitted before the freeze still count - and it was early,
the current state went without review for over 3 months and you didn't reply to
my reply on the issue...

Still, I get that you are all pretty busy and I don't want anything rushed.
Can I just bump this in June or should I start a golang-dev thread?

[bradfitz, 2014-04-10 16:19:38]

Start a golang-dev thread and reference this and the bug.

It's true that CLs mailed before the cut are valid, but this CL could use
some design/naming work, which nobody has time for right now.



On Thu, Apr 10, 2014 at 9:15 AM, <arnehormann@gmail.com> wrote:

> On 2014/04/10 16:01:57, bradfitz wrote:
>
>> Sorry, we're too late in the Go 1.3 process for this. If you want to
>>
> see this go
>
>> in, begin discussions early in the Go 1.4 cycle, once you see Go 1.3
>>
> released
>
>> around June 1st.
>>
>
> I discussed it - but on golang-nuts instead of golang-dev (newbie
> mistake). That was inMay last year.
> I also thought CLs submitted before the freeze still count - and it was
> early, the current state went without review for over 3 months and you
> didn't reply to my reply on the issue...
>
> Still, I get that you are all pretty busy and I don't want anything
> rushed.
> Can I just bump this in June or should I start a golang-dev thread?
>
> https://codereview.appspot.com/43510044/
>

[bradfitz, 2014-04-14 22:28:57]

R=close

Until Go 1.4.