volatiletech / sqlboiler

Generate a Go ORM tailored to your database schema.
BSD 3-Clause "New" or "Revised" License
6.66k stars 539 forks source link

Only last qm.Load with Select clause is honorred #670

Open andradei opened 4 years ago

andradei commented 4 years ago

What version of SQLBoiler are you using (sqlboiler --version)?

SQLBoiler v3.6.1

What is your database and version (eg. Postgresql 10)

12.1

If this happened at runtime what code produced the issue? (if not applicable leave blank)

I tried something like:

s, err := models.Somethings(
  qm.Load(models.SomethingRels.Dad, qm.Select("name")),
  qm.Load(models.SomethingRels.Mom, qm.Select("name")),
  qm.Load(models.SomethingRels.Spouse, qm.Select("name")),
).All(c, exec)

Further information. What did you do, what did you expect?

Running the code above, only the last qm.Load Select was honored. So s.R.Spouse.Name was populated, but s.R.Dad/Mom were nil. Now, s.Dad/Mom (notice no .R.) are or type null.Int because they are foreign keys. This is important (and weird) because they are correctly set, and that's the same result I see when debug mode is on and the SELECT statement for eager-loading Dad/Mom is printed to stderr (it doesn't show the result of the query, but the struct s.Dad/Mom, like I said, weird).

I tried reading the eager-loading source, but that was too complex and I would need more time to be able to help.

aarondl commented 4 years ago

@andradei Any chance of getting a sample schema that comes with this? I'm not 100% sure when I'll get to this but I feel a big change in eager loading coming up soon (maybe a full rewrite for v4) and this sort of bug must be accounted for.

andradei commented 4 years ago

I'll try to get one to you. This might take a while because the code base I encountered this problem in is private, so I'll have to rename a few things and still make sure I have a reproducible example to give you.

Thanks.

muhlemmer commented 3 years ago

@andradei. It seems that using qm.Select confuses qm.Load if the relational id columns are omitted. So consider writing the above as:

s, err := models.Somethings(
  qm.Load(models.SomethingRels.Dad, qm.Select("id", "name")),
  qm.Load(models.SomethingRels.Mom, qm.Select("id", "name")),
  qm.Load(models.SomethingRels.Spouse, qm.Select("id", "name")),
).All(c, exec)
valzam commented 1 year ago

Just wanted to comment that I have been bitten my this as well. I think as a user it's quite unexpected that loading relations doesn't related in a SQL join and therefore the id column is needed explicitly so that sqlboiler can join in the application layer. I think it would be reasonable to automatically add id columns for qm.Select within a qm.Load since not including them is an easy footgun.