Skip to content
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

NamedQuery causes panic for embedded struct #24

Closed
e-dard opened this issue Nov 10, 2013 · 3 comments
Closed

NamedQuery causes panic for embedded struct #24

e-dard opened this issue Nov 10, 2013 · 3 comments
Labels

Comments

@e-dard
Copy link

e-dard commented Nov 10, 2013

I spoke to @jmoiron briefly about this on Friday, and have put together a simple example to show problem.

When using NamedQuery with an embedded struct, Go panics:

go run main.go
classic OK - id = 16
panic: reflect: Field index out of range

goroutine 1 [running]:
reflect.Value.Field(0x1cef60, 0xc2000796c0, 0x192, 0x1, 0x1, ...)
    /usr/local/share/go/src/pkg/reflect/value.go:785 +0x164
github.com/jmoiron/sqlx.BindStruct(0x2, 0x25f770, 0x56, 0x1cef60, 0xc2000796c0, ...)
    <snip>/src/github.com/jmoiron/sqlx/bind.go:84 +0x4d9
github.com/jmoiron/sqlx.(*Tx).BindStruct(0xc200095700, 0x25f770, 0x56, 0x1cef60, 0xc2000796c0, ...)
    <snip>/src/github.com/jmoiron/sqlx/sqlx.go:298 +0x71
github.com/jmoiron/sqlx.NamedQuery(0xc2000b5300, 0xc200095700, 0x25f770, 0x56, 0x1cef60, ...)
    <snip>/src/github.com/jmoiron/sqlx/sqlx.go:1021 +0x61
github.com/jmoiron/sqlx.(*Tx).NamedQuery(0xc200095700, 0x25f770, 0x56, 0x1cef60, 0xc2000796c0, ...)
    <snip>/src/github.com/jmoiron/sqlx/sqlx.go:303 +0x67
main.InsertNamed(0x0, 0x16, 0xc2000b3150, 0x1)
    <snip>/src/scratch/sqlx_bug/main.go:49 +0xfc
main.main()
    <snip>/src/scratch/sqlx_bug/main.go:78 +0x16d

goroutine 2 [syscall]:
exit status 2

Reproduce Issue

I can reproduce the issue by creating a PostgreSQL table according to the following schema:

CREATE TABLE "public"."table" (
    "id" SERIAL PRIMARY KEY,
    "field" int4
);

And then using the following Go program:

package main

import (
    "fmt"
    "github.com/jmoiron/sqlx"
    _ "github.com/lib/pq"
)

type T struct {
    Id    int
    Field int `db:"field"`
}

type Container struct {
    T
}

func InsertClassic(c Container, db *sqlx.DB) (id int) {
    stmt := `INSERT into "table" ("field")
             VALUES ($1)
             RETURNING "id"`

    tx, err := db.Begin()
    if err != nil {
        panic(err)
    }

    if err = tx.QueryRow(stmt, c.Field).Scan(&id); err != nil {
        panic(err)
    }

    if err = tx.Commit(); err != nil {
        panic(err)
    }
    return
}

func InsertNamed(c Container, db *sqlx.DB) (id int) {
    stmt := `INSERT into "table" ("field")
             VALUES (:field)
             RETURNING "id"`

    tx, err := db.Beginx()
    if err != nil {
        panic(err)
    }

    var rows *sqlx.Rows
    if rows, err = tx.NamedQuery(stmt, c); err != nil {
        panic(err)
    }
    defer rows.Close()

    for rows.Next() {
        if err = rows.Scan(&id); err != nil {
            panic(err)
        }
    }

    if err = rows.Err(); err != nil {
        panic(err)
    }

    if err = tx.Commit(); err != nil {
        panic(err)
    }
    return
}

func main() {
    db, err := sqlx.Connect("postgres", "dbname=postgres sslmode=disable port=5432")
    if err != nil {
        panic(err)
    }

    c := Container{T{Field: 22}}
    fmt.Printf("classic OK - id = %v\n", InsertClassic(c, db))
    fmt.Println("named OK - id = %v\n", InsertNamed(c, db))
}

The database/sql approach of using PG parameters works fine, but using sqlx's NamedQuery results in a panic.

I believe that the issue is stemming from the fact sqlx.getFieldMap maps the fields in the embedded struct T to indexes as follows:

map[string]int{
    "id": 0, 
    "field": 1,
}

, while the application of that mapping, via reflect.ValueOf(arg), where arg is the Container struct c, results in the first index of the map (which should map to c.T.Id) actually mapping to c.T. From there it's straightforward to see that there is no field available at the index 1 in the struct.

So, it appears that the implementation in sqlx.BindStruct does not make the right calls to the reflect package to get fields in the embedded field T, but instead gets the field(s) in the Container struct.

@chakrit
Copy link

chakrit commented Dec 21, 2013

+1 for this to gets fixed. Been trying to get sqlx to work these past few days and this prevents almost all use case of embedded structs for me.

@chakrit
Copy link

chakrit commented Dec 27, 2013

@jmoiron thanks for the fix!

@e-dard
Copy link
Author

e-dard commented Dec 27, 2013

Awesome. Thanks @jmoiron 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants