globalsign / mgo

The MongoDB driver for Go
Other
1.97k stars 231 forks source link

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

Open bojangavrovski opened 6 years ago

bojangavrovski commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

larrycinnabar commented 6 years ago

@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 commented 6 years ago

@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 commented 6 years ago

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