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.
>