volatiletech / sqlboiler

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

nil values in eager loading nested models via many to many join table #154

Closed edoshor closed 7 years ago

edoshor commented 7 years ago

My Scenario: A and B are first class models joined by a many to many table C.

For simplicity, let's say I load all like this

models.As(db, qm.Load("C", "C.B")).All()

The relevant part of the generated code for my schema is:(https://github.com/vattle/sqlboiler/blob/master/templates/09_relationship_to_many_eager.tpl#L119-L129)

Let's say my tables have these rows: A [{1},{2},{3}] B [{1},{2},{3}] C [{1,1}, {2,1}, {3,3}]

Debug log shows, something like this:

select * from "As"
[]
select * from Cs where "id" in ($1,$2,$3)
[1,2,3]
select * from "Bs" where "id" in ($1,$2,$3)
[1,1,3]

The second C I get is missing the eager loaded B in it, C[1].R.B is nil. That's because the code above overrides the B in the first C (it puts it there twice...)

Update sorry, bad observation. That's all because the duplicate values in the IN clause. Need to delete the break statement on Line 127.

aarondl commented 7 years ago

Well look at this this weekend. Thanks for the report.

aarondl commented 7 years ago

@edoshor Can you clarify if you're using a many to many table as defined by sqlboiler, or your own? Because the part of the code that's being touched here and also the use case of eager loading this way doesn't seem correct.

A many-to-many table as defined by sqlboiler is:

Are you using a table that looks like this, or something else?

aarondl commented 7 years ago

So I'm going to need a little bit more information. Maybe even a snippet from your schema.

Using this:

create table videos (
  id serial not null primary key
);

create table tags (
  id serial not null primary key
);

create table tag_videos (
  video_id int not null,
  tag_id int not null,
  somemetadata text not null,
  primary key (video_id, tag_id),
  foreign key (video_id) references videos (id),
  foreign key (tag_id) references tags (id)
);

And this code:

    vids, err := models.Videos(db, qm.Load("TagVideos", "TagVideos.Tag")).All()
    if err != nil {
        panic(err)
    }

I can't really see anything wrong with the output?

SELECT * FROM "videos";
[]
select * from "tag_videos" where "video_id" in ($1,$2,$3)
[1 2 3]
select * from "tags" where "id" in ($1,$2,$3)
[1 1 3]
(models.VideoSlice) (len=3 cap=4) {
 (*models.Video)(0xc420118100)({
  ID: (int) 1,
  R: (*models.videoR)(0xc420118240)({
   TagVideos: (models.TagVideoSlice) (len=1 cap=1) {
    (*models.TagVideo)(0xc4200f9470)({
     VideoID: (int) 1,
     TagID: (int) 1,
     Somemetadata: (string) (len=4) "what",
     R: (*models.tagVideoR)(0xc4201087e0)({
      Video: (*models.Video)(<nil>),
      Tag: (*models.Tag)(0xc420118740)({
       ID: (int) 1,
       R: (*models.tagR)(<nil>),
       L: (models.tagL) {
       }
      })
     }),
     L: (models.tagVideoL) {
     }
    })
   }
  }),
  L: (models.videoL) {
  }
 }),
 (*models.Video)(0xc420118140)({
  ID: (int) 2,
  R: (*models.videoR)(0xc420118260)({
   TagVideos: (models.TagVideoSlice) (len=1 cap=1) {
    (*models.TagVideo)(0xc4200f94d0)({
     VideoID: (int) 2,
     TagID: (int) 1,
     Somemetadata: (string) (len=5) "there",
     R: (*models.tagVideoR)(0xc420108800)({
      Video: (*models.Video)(<nil>),
      Tag: (*models.Tag)(0xc420118740)({
       ID: (int) 1,
       R: (*models.tagR)(<nil>),
       L: (models.tagL) {
       }
      })
     }),
     L: (models.tagVideoL) {
     }
    })
   }
  }),
  L: (models.videoL) {
  }
 }),
 (*models.Video)(0xc420118180)({
  ID: (int) 3,
  R: (*models.videoR)(0xc420118280)({
   TagVideos: (models.TagVideoSlice) (len=1 cap=1) {
    (*models.TagVideo)(0xc4200f9530)({
     VideoID: (int) 3,
     TagID: (int) 3,
     Somemetadata: (string) (len=6) "friend",
     R: (*models.tagVideoR)(0xc420108810)({
      Video: (*models.Video)(<nil>),
      Tag: (*models.Tag)(0xc420118780)({
       ID: (int) 3,
       R: (*models.tagR)(<nil>),
       L: (models.tagL) {
       }
      })
     }),
     L: (models.tagVideoL) {
     }
    })
   }
  }),
  L: (models.videoL) {
  }
 })
}

Though I did fix a bug while I was looking at this that was preventing this code from working, so I assume we've got some differences going on here. Are you on the latest version? Can you maybe share the actual schema with names elided and non-primary-key columns removed (except in the case where you need it to be a not-pure join table as I described above)

edoshor commented 7 years ago

My join table satisfies the first two bullets in sqlboiler definition but not the third. It does have another column. Though I'm not sure if it matters or not.

The schema was generated using v2.1.7

Relevant part of the schema goes like this:


CREATE TABLE collections (
  id BIGSERIAL PRIMARY KEY,
);

CREATE TABLE content_units (
  id BIGSERIAL PRIMARY KEY,
);

CREATE TABLE collections_content_units (
  collection_id   BIGINT REFERENCES collections   NOT NULL,
  content_unit_id BIGINT REFERENCES content_units NOT NULL,
  name            VARCHAR(255)                    NOT NULL,
  PRIMARY KEY (collection_id, content_unit_id)
);

code snippet goes like this:

collections, err := models.Collections(db, 
    qm.Load("CollectionsContentUnits", "CollectionsContentUnits.ContentUnit"))).All()
if err != nil {
    panic(err)
}

The output you get looks good to me as well. I get nil on

(*models.TagVideo)(0xc4200f94d0) .R.Tag

I'll try to regenerate with the latest version and see if that helps.

aarondl commented 7 years ago

Have you had a chance to try with latest?

edoshor commented 7 years ago

Tried with the latest master (2.4.0) and nope, it doesn't seems to solve this bug.

I think the nested loop order should be as it is for a ToOne relationship

Anyway, using the latest version introduced a new issue causing the generated tests in another table to fail. I'll open another issue for that.

aarondl commented 7 years ago

Nice catch on the generated test failure. I've fixed that on the dev branch.

Also a different code path is used if it's a "pure" join table vs just a regular table. SQLBoiler doesn't generate a struct for the pure join tables and so things get queried and organized a little bit differently so it does matter if you have extra columns or not :)

I am still however having problems reproducing the issue you're seeing using the test data that you've supplied. Using the same schema as I posted above with some different output to better show what's going on:

Data used:

insert into videos default values;
insert into videos default values;
insert into videos default values;

insert into tags default values;
insert into tags default values;
insert into tags default values;

insert into tag_videos (video_id, tag_id, somemetadata) values (1, 1, 'what');
insert into tag_videos (video_id, tag_id, somemetadata) values (2, 1, 'what');
insert into tag_videos (video_id, tag_id, somemetadata) values (3, 3, 'what');
vids, err := models.Videos(db, qm.Load("TagVideos", "TagVideos.Tag")).All()
SELECT * FROM "videos";
[]
select * from "tag_videos" where "video_id" in ($1,$2,$3)
[1 2 3]
select * from "tags" where "id" in ($1,$2,$3)
[1 1 3]
Video: 1
  TagVideo: 1 1
    Tag: 1
Video: 2
  TagVideo: 2 1
    Tag: 1
Video: 3
  TagVideo: 3 3
    Tag: 3

The complete opposite also looks correct to me:

tags, err := models.Tags(db, qm.Load("TagVideos", "TagVideos.Video")).All()
[]
select * from "tag_videos" where "tag_id" in ($1,$2,$3)
[1 2 3]
select * from "videos" where "id" in ($1,$2,$3)
[1 2 3]
Tag: 1
  TagVideo: 1 1
    Video: 1
  TagVideo: 2 1
    Video: 2
Tag: 2
Tag: 3
  TagVideo: 3 3
    Video: 3

I attempted to reverse the data too. So that the inserts looked like this:

insert into tag_videos (video_id, tag_id, somemetadata) values (1, 1, 'what');
insert into tag_videos (video_id, tag_id, somemetadata) values (1, 2, 'what');
insert into tag_videos (video_id, tag_id, somemetadata) values (3, 3, 'what');
Video: 1
  TagVideo: 1 1
    Tag: 1
  TagVideo: 1 2
    Tag: 2
Video: 2
Video: 3
  TagVideo: 3 3
    Tag: 3
Tag: 1
  TagVideo: 1 1
    Video: 1
Tag: 2
  TagVideo: 1 2
    Video: 1
Tag: 3
  TagVideo: 3 3
    Video: 3

This all looks correct to me. Can you provide anything else to help me solve this? Please keep in mind that sqlboiler uses GOPATH to find it's templates. So if you simply compiled a different version of the binary but still have the old version checked out in your GOPATH it may be using the old templates and producing the same issue, despite the new binary. We're looking at fixing this with bindata but for now it may be causing issues as you try to debug.

edoshor commented 7 years ago

Well I can't seem to reproduce this with latest dev so I guess we better close this for now. If I'll run into it in the future I'll reopen this one.

Thanks a lot for all your efforts.