Skip to content

Commit

Permalink
provide reflectx.FieldByIndexesReadOnly which does not allocate nil p…
Browse files Browse the repository at this point in the history
…ointers, which fixes two bugs with named queries: the first is unaddressable values passed to Named exec/query/etc that had nil pointers attempted an illegal set and paniced, and the second is described in jmoiron#68
  • Loading branch information
jmoiron committed Jun 28, 2014
1 parent 65a9444 commit 5cb0c84
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 1 deletion.
2 changes: 1 addition & 1 deletion named.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ func bindArgs(names []string, arg interface{}) ([]interface{}, error) {
if len(t) == 0 {
return arglist, fmt.Errorf("could not find name %s in %v", names[i], arg)
}
val := reflectx.FieldByIndexes(v, t)
val := reflectx.FieldByIndexesReadOnly(v, t)
arglist = append(arglist, val.Interface())
}

Expand Down
10 changes: 10 additions & 0 deletions reflectx/reflect.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,16 @@ func FieldByIndexes(v reflect.Value, indexes []int) reflect.Value {
return v
}

// FieldByIndexesReadOnly returns a value for a particular struct traversal,
// but is not concerned with allocating nil pointers because the value is
// going to be used for reading and not setting.
func FieldByIndexesReadOnly(v reflect.Value, indexes []int) reflect.Value {
for _, i := range indexes {
v = reflect.Indirect(v).Field(i)
}
return v
}

// Deref is Indirect for reflect.Types
func Deref(t reflect.Type) reflect.Type {
if t.Kind() == reflect.Ptr {
Expand Down
45 changes: 45 additions & 0 deletions sqlx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,51 @@ func TestNamedQuery(t *testing.T) {
})
}

func TestNilInserts(t *testing.T) {
var schema = Schema{
create: `
CREATE TABLE tt (
id integer,
value text NULL DEFAULT NULL
);`,
drop: "drop table tt;",
}

RunWithSchema(schema, t, func(db *DB, t *testing.T) {
type TT struct {
Id int
Value *string
}
var v, v2 TT
r := db.Rebind

db.MustExec(r(`INSERT INTO tt (id) VALUES (1)`))
db.Get(&v, r(`SELECT * FROM tt`))
if v.Id != 1 {
t.Errorf("Expecting id of 1, got %v", v.Id)
}
if v.Value != nil {
t.Errorf("Expecting NULL to map to nil, got %s", v.Value)
}

v.Id = 2
// NOTE: this incidentally uncovered a bug which was that named queries with
// pointer destinations would not work if the passed value here was not addressable,
// as reflectx.FieldByIndexes attempts to allocate nil pointer receivers for
// writing. This was fixed by creating & using the reflectx.FieldByIndexesReadOnly
// function. This next line is important as it provides the only coverage for this.
db.NamedExec(`INSERT INTO tt (id, value) VALUES (:id, :value)`, v)

db.Get(&v2, r(`SELECT * FROM tt WHERE id=2`))
if v.Id != v2.Id {
t.Errorf("%v != %v", v.Id, v2.Id)
}
if v2.Value != nil {
t.Errorf("Expecting NULL to map to nil, got %s", v.Value)
}
})
}

func TestScanError(t *testing.T) {
var schema = Schema{
create: `
Expand Down

0 comments on commit 5cb0c84

Please sign in to comment.