-
Notifications
You must be signed in to change notification settings - Fork 18k
proposal: database/sql: expose convertAssign functionality in an Any type #35697
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
/cc @kardianos |
@kardianos, any opinions on this one way or another? |
The converAssign function is certainly useful for other purposes in drivers, or possibly libraries. For instance, the MS SQL driver has a copy of the convert assign function here. It is used to scan Output variables back out from the result. @tgulacsi, are you aware of other use cases for someone copying convertAssign? To date, I've classified this as "a little copying is fine", especially as the code doesn't change often. I think if we gathered sufficient real world examples of this copied in the wild, we should consider exporting it in some way. |
No, I don't use a copy of convertAssign, but it may be useful, as it does some nice tricks. But I don't understand what is this Any type for - you do know the type of the requested column, don't you? If not, use and interface{}... Maybe a stripped-down Convert, which can convert between various string, []byte and numeric types? |
@dolmen Can you point me to code that currently copies and pastes this function today? |
Unless I can see a few more places this is already used by copy/paste, I'm inclined to reject this. This code can generally be safely copy and pasted. Setting to likely decline to declined if no additional examples are found. |
@kardianos I wrote this import (
"database/sql"
"fmt"
"log"
"reflect"
)
type appendScanner struct {
elemZero reflect.Value
scan func(target reflect.Value, v interface{}) error
slice reflect.Value
}
func (as *appendScanner) Scan(v interface{}) error {
newTarget := reflect.Append(as.slice.Elem(), as.elemZero)
newElem := newTarget.Index(newTarget.Len() - 1).Addr()
if err := as.scan(newElem.Elem(), v); err != nil {
return err
}
as.slice.Elem().Set(newTarget)
return nil
}
// AppendScanner returns an sql.Scanner that appends the value from the database to the slice.
func AppendScanner(slice interface{}) sql.Scanner {
t := reflect.TypeOf(slice)
if t.Kind() != reflect.Ptr || t.Elem().Kind() != reflect.Slice || reflect.ValueOf(slice).IsNil() {
log.Println(t.Kind())
log.Println(t.Elem().Kind())
panic("target should be a pointer to a slice variable")
}
elemType := t.Elem().Elem()
var scan func(target reflect.Value, v interface{}) error
if reflect.PtrTo(elemType).Implements(scannerType) || reflect.PtrTo(reflect.PtrTo(elemType)).Implements(scannerType) {
scan = func(target reflect.Value, v interface{}) error {
return target.Interface().(sql.Scanner).Scan(v)
}
} else {
switch reflect.Zero(elemType).Interface().(type) {
case int, *int, int64, *int64, int32, *int32:
scan = func(target reflect.Value, v interface{}) error {
target.SetInt(v.(int64))
return nil
}
case uint, *uint, uint64, *uint64, uint32, *uint32:
scan = func(target reflect.Value, v interface{}) error {
target.SetUint(uint64(v.(int64)))
return nil
}
case string, *string:
scan = func(target reflect.Value, v interface{}) error {
switch v := v.(type) {
case string:
target.SetString(v)
case []byte:
target.SetString(string(v))
default:
return fmt.Errorf("unexpected type %T to scan into %s", v, target.Type())
}
return nil
}
case []byte:
scan = func(target reflect.Value, v interface{}) error {
target.SetBytes(v.([]byte))
return nil
}
default:
// TODO add support for any driver.Value type
panic("slice elements must implement sql.Scanner")
}
}
// If pointer, wrap the scan to allocate the target value if not NULL
if reflect.Zero(elemType).Kind() == reflect.Ptr {
innerScan := scan
scan = func(target reflect.Value, v interface{}) error {
if v == nil {
return nil
}
vv := reflect.New(target.Type().Elem())
if err := innerScan(vv.Elem(), v); err != nil {
return err
}
target.Set(vv)
return nil
}
}
return &appendScanner{
elemZero: reflect.Zero(elemType),
// isElemPtr: elemType.Kind() == reflect.Ptr,
scan: scan,
slice: reflect.ValueOf(slice),
}
} And here are two testcases: func ExampleAppendScanner() {
ctx := context.Background()
db, err := sql.Open("sqlite3", ":memory:")
if err != nil {
log.Printf("Open: %v", err)
return
}
defer db.Close()
// Use AppendScanner to scan multiple columns values into a slice
var v1 []int64
appender := sqlutil.AppendScanner(&v1)
if err := db.QueryRowContext(ctx, `SELECT 1, 2, 3`).
Scan(appender, appender, appender); err != nil {
log.Fatal(err)
}
fmt.Println(v1)
// Use AppendScanner to scan value from multiple into a slice
var v2 []string
appender = sqlutil.AppendScanner(&v2)
rows, err := db.QueryContext(ctx, ``+
` SELECT 'a'`+
` UNION ALL`+
` SELECT 'b'`+
` UNION ALL`+
` SELECT 'c'`,
)
if err != nil {
log.Fatal(err)
}
defer rows.Close()
for rows.Next() {
if err = rows.Scan(appender); err != nil {
log.Fatal(err)
}
}
if err = rows.Err(); err != nil {
log.Fatal(err)
}
fmt.Println(v2)
// Output:
// [1 2 3]
// [a b c]
} |
@kardianos said:
The reply from @dolmen shows one copy, but I don't think we have evidence that the copying is very common. Are there more examples? (@kardianos, I moved the project classification back to Active because we only mark Likely Decline when we publish proposal minutes.) |
Based on the lack of evidence that copying is very common, this seems like a likely decline. |
No change in consensus, so declined. |
The internal
convertAssignRows
function (from convert.go) does much magic. Access to this magic is useful for libraries that want to expose utilities (types, functions) that wrap any destination value accepted byRows.Scan
and expose ansql.Scanner
by usingsql.Scanner
s as russian dolls. This could be used for example to implement struct scanning.However that magic is hard to reproduce and maintain with full compatibility on the long term for external users of
database/sql
.I propose to add a function
sql.Any
that exposes most of theconvertAssignRows
magic by allowing to wrap anyRows.Scan
-compatible value as ansql.Scanner
.Notes:
Any
is a bad name (not enough explicit)convertAssignRows
magic and what conversions would be not be possible if the converter doesn't have access to both the source and destinations types at the same time. I want to be lightened about those edge cases.The text was updated successfully, but these errors were encountered: