R being overwritten when doing multiple eager loads

[jseriff]

I believe this is related to #55 and #51 together. When doing multiple eager loads (as in #55), all of the queries are being run, but only the last eager on an object is being stored back to the R struct on the model.

This appears to be related to this block in the Load*** methods:

    if singular {
        object.R = &testR{}
        args[0] = object.ID
    } else {
        for i, obj := range slice {
            obj.R = &testR{}
            args[i] = obj.ID
        }
    }

Because each Load methods sets object.R, only the last Load method's instance will be used. At a minimum, I believe there should be a null check wrapping these (and anywhere an R gets set). I think a better solution might be to just have R not be a pointer, so it always exists - that will also save lots of null checking in the calling code.

I dont have a specific test case for this, although I think the case I outlined in #55 should show the issue.

[aarondl]

Hey @jseriff. Going to look at this tomorrow. The test cases I have should be sufficient for this thanks :)

I wanted to ask you a little about your use of SQLBoiler, have you been actively using it and seeing success? Hopefully these bugs haven't been completely stalling you. Can you tell us anything about your usage? Just keen to hear from people who are using it in the field.

[jseriff]

@aarondl - For usage - we have a medium size Go project that is currently using GORM to do database <-> struct mapping. The project has a large legacy schema that we are working with. We have been quite underwhelmed by GORM, and wanted to find an alternative. We had toyed around with the idea of building a code generation based database mapping tool when we found SQLBoiler.

As I have time, I have been playing around with migrating from GORM to SQLBoiler, which is how I have been coming across these various issues. Unfortunately, most of the issues have been blockers for us, so we really appreciate the speed at which you have been resolving them. I'm maintaining the SQLBoiler port in a branch, and will continue to work on it, hopefully being able to complete the port by end of year.

I am confident I will have some feature requests or pull requests for a few features that we think would be good additions as well - I'll get to those once I have a largely completed port.

[aarondl]

Nice to hear. I hope we can sort out all these bugs so that you can continue to use it.

I'm fixing the bug now, but as I was re-reading the issue I want to touch on why we decided R should be a pointer. If you have a struct with a lot of relationships currently, you're going to eat a potentially good chunk of allocation up front even if you never use that object to load any relationships. For example when you insert, or select one, or etc. It seemed like the use cases in which you interact with a single object outweigh the amount of times you want to use it with relationships. And therefore we deferred the memory allocation until use-time for relationships.

It's possible it's a premature optimization. But I thought that it should be okay so long as we got rid of the need to check for nil which I thought we had in the previous bugs. Sure they'll be in the generated code, but most people won't have to look there unless debugging.

Thoughts on this?

[jseriff]

I'm fine either way on the R issue - I definitely understand the allocation concerns - not sure how material they are without testing. The place where I most ran into it was setting up some of our integration tests, and needed to check to make sure R was non null. I dont know that it will be a common real world issue.

[aarondl]

Agreed that there's not much to it without solid testing and more usage. Your issues are fixed in 3046214fb657d1eceaa5f21093aadee553db726f (dev branch). Will release & merge once we solve the other two planned issues.

[aarondl]

https://github.com/vattle/sqlboiler/releases/tag/v2.1.0