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

Pointer in inline structs returns error when reading from database #210

Open
bojangavrovski opened this issue Jul 8, 2018 · 5 comments
Open
Labels

Comments

@bojangavrovski
Copy link

bojangavrovski commented Jul 8, 2018

Hi
I encountered an issue while populating a struct with db data. My structs are as follows:

type Product struct {
    ID bson.ObjectId `json:"id" bson:"_id"`
    Name string `json:"name" bson:"name"`
    Meta `json:"meta" bson:"meta"`
}

type Meta {
    *Printer `json:",inline" bson:",inline"`
    *Monitor `json:",inline" bson:",inline"`
}

type Monitor struct {
    ScreenSize float64 `json:"screen_size" bson:"screen_size"`
    ScreenType string `json:"screen_type" bson:"screen_type"`
}

...

p := Product{}
if err := db.C("products").Find(nil).One(&p); err != nil {
    return err
}

the error that I get from this action is

reflect: indirection through nil pointer to embedded struct

I should note that this only happens while reading from the db, and not when inserting into.

I am aware that this issue has been raised couple of times before: #146, go-mgo#346, and although a fix has been submitted, it still seems that it doesn't work properly (at least for reading data from the db).

The version of go that I'm using is

go version go1.10.3 darwin/amd64

and the globalsign/mgo version is the last one available

@larrycinnabar
Copy link

larrycinnabar commented Jul 13, 2018

Yeap, unfortunately, the inline-pointer feature was not fully implemented (the encoding is ok, but decoding doesn't work).

@bojangavrovski, I have a fix, but it not fully tested yet. Will make a PR with a fix this weekend.

@domodwyer
Copy link

Hi @bojangavrovski / @larrycinnabar

Sorry for the delay - it was a busy week!

Thanks so much @larrycinnabar for triaging and developing a fix - it's truly appreciated - it's nice to see the open source community in action! Let me know if I can help with the fix PR at all.

Thanks again!

Dom

@domodwyer domodwyer added the bug label Jul 16, 2018
@larrycinnabar
Copy link

larrycinnabar commented Jul 16, 2018

@domodwyer
bug appeared as a result of my feature, so yeap, it's my responsibility to fix it now.
I'm almost done, (all old test cases pass), but a couple of new test cases fail still.

I'm working on the fix during off-hours, so it will wait a little bit more ;)

@larrycinnabar
Copy link

@bojangavrovski
If you're in a hurry, you can just copy-n-paste my wip PR's code (not so many changes), and insert in your local branch. Will work as a temporary workaround

@msolimans
Copy link

@larrycinnabar thanks! seems like it is not merged yet!, any updates or plans for this issue?

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

Successfully merging a pull request may close this issue.

4 participants