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

StructScan doesn't work with embedded pointer fields. #35

Closed
kisielk opened this issue Dec 10, 2013 · 5 comments
Closed

StructScan doesn't work with embedded pointer fields. #35

kisielk opened this issue Dec 10, 2013 · 5 comments
Labels

Comments

@kisielk
Copy link
Contributor

kisielk commented Dec 10, 2013

sqlx supports embedded structs, but pointers to structs are not followed. This is a bug, because it's contrary to reasonable expectations, ie:

type Foo struct {
    Name string
}

// can set 'Name' on this struct
type Place struct {
    Foo
}

// cannot set 'Name' on this struct
type Person struct {
    *Foo
}

Currently, when you scan into a nil pointer, database/sql allocates the space for you. This is how []byte and string work as well; a new one will always be allocated. I'm going to copy this behavior, even though it's not well known, because I don't want StructScan to behave differently and because I think it could end up creating incredibly difficult to figure out bugs.

The setValues function can allocate these using reflect.

Note that all this per-row allocation is going to cost a little something, so if speed is a priority I recommend against pointer embeds.

@teburd
Copy link

teburd commented Dec 10, 2013

Specifically structscan doesn't work with embedded struct pointer fields?

Thats at least the issue I'm having

@kisielk
Copy link
Contributor Author

kisielk commented Dec 10, 2013

Sure, I was just noting from the conversation. If you can post an example here that would be helpful.

@simonklee
Copy link

After this patch I receive sql: Scan error on column index 6: destination pointer is nil upon calling StructScan. Just to notify that this brakes some working code. I'll see if I get time for a full bug report later.

@jmoiron
Copy link
Owner

jmoiron commented Jan 4, 2014

All embedded pointers should be getting allocated but if you can amend the test case for them to fail that'd help a lot.

@simonklee
Copy link

0d135e4 fixed the issue for me.

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

4 participants