Permalink
Browse files

Fix a few corner cases in eager_graph

This commit fixes two separate problems with eager graph. The first
is that cascading *_to_many associations could cause an exception to
be raised, because eager_graph_make_associations_unique wasn't
recursing properly.

The second is that *_to_many associations that were cascaded after
a many_to_one association weren't recursed into to eliminate
duplicates caused by cartesian products.

Thanks a lot to jarredh on IRC for bringing the second bug to my
attention, which led me to fix the first bug (even before I fixed the
second).
  • Loading branch information...
1 parent b9eeb3b commit 022d4dd290a2c460e9b8ae9fd1a26780f7767bcd @jeremyevans committed Sep 17, 2008
Showing with 35 additions and 2 deletions.
  1. +2 −0 CHANGELOG
  2. +2 −2 lib/sequel_model/eager_loading.rb
  3. +31 −0 spec/sequel_model/eager_loading_spec.rb
View
@@ -1,5 +1,7 @@
=== HEAD
+* Fix a few corner cases in eager_graph (jeremyevans)
+
* Support transactions on MSSQL (jeremyevans)
* Use string literals in AS clauses on SQLite (jeremyevans) (#241)
@@ -323,9 +323,9 @@ def eager_graph_make_associations_unique(records, dependency_map, alias_map, typ
else
list = record.send(alias_map[ta])
list.uniq!
- # Recurse into dependencies
- list.each{|rec| eager_graph_make_associations_unique(rec, deps, alias_map, type_map)}
end
+ # Recurse into dependencies
+ eager_graph_make_associations_unique(list, deps, alias_map, type_map) if list
end
end
end
@@ -638,6 +638,37 @@ def ds.fetch_rows(sql, &block)
a.album.band.members.first.values.should == {:id => 5}
end
+ it "should allow cascading of eager loading for multiple *_to_many associations, eliminating duplicates caused by cartesian products" do
+ ds = GraphBand.eager_graph({:albums=>:tracks}, :members)
+ ds.sql.should == 'SELECT bands.id, bands.vocalist_id, albums.id AS albums_id, albums.band_id, tracks.id AS tracks_id, tracks.album_id, members.id AS members_id FROM bands LEFT OUTER JOIN albums ON (albums.band_id = bands.id) LEFT OUTER JOIN tracks ON (tracks.album_id = albums.id) LEFT OUTER JOIN bm ON (bm.band_id = bands.id) LEFT OUTER JOIN members ON (members.id = bm.member_id)'
+ def ds.fetch_rows(sql, &block)
+ yield({:id=>1, :vocalist_id=>2, :albums_id=>3, :band_id=>1, :tracks_id=>4, :album_id=>3, :members_id=>5})
+ yield({:id=>1, :vocalist_id=>2, :albums_id=>3, :band_id=>1, :tracks_id=>4, :album_id=>3, :members_id=>6})
+ yield({:id=>1, :vocalist_id=>2, :albums_id=>3, :band_id=>1, :tracks_id=>5, :album_id=>3, :members_id=>5})
+ yield({:id=>1, :vocalist_id=>2, :albums_id=>3, :band_id=>1, :tracks_id=>5, :album_id=>3, :members_id=>6})
+ yield({:id=>1, :vocalist_id=>2, :albums_id=>4, :band_id=>1, :tracks_id=>6, :album_id=>4, :members_id=>5})
+ yield({:id=>1, :vocalist_id=>2, :albums_id=>4, :band_id=>1, :tracks_id=>6, :album_id=>4, :members_id=>6})
+ yield({:id=>1, :vocalist_id=>2, :albums_id=>4, :band_id=>1, :tracks_id=>7, :album_id=>4, :members_id=>5})
+ yield({:id=>1, :vocalist_id=>2, :albums_id=>4, :band_id=>1, :tracks_id=>7, :album_id=>4, :members_id=>6})
+ yield({:id=>2, :vocalist_id=>2, :albums_id=>5, :band_id=>2, :tracks_id=>8, :album_id=>5, :members_id=>5})
+ yield({:id=>2, :vocalist_id=>2, :albums_id=>5, :band_id=>2, :tracks_id=>8, :album_id=>5, :members_id=>6})
+ yield({:id=>2, :vocalist_id=>2, :albums_id=>5, :band_id=>2, :tracks_id=>9, :album_id=>5, :members_id=>5})
+ yield({:id=>2, :vocalist_id=>2, :albums_id=>5, :band_id=>2, :tracks_id=>9, :album_id=>5, :members_id=>6})
+ yield({:id=>2, :vocalist_id=>2, :albums_id=>6, :band_id=>2, :tracks_id=>1, :album_id=>6, :members_id=>5})
+ yield({:id=>2, :vocalist_id=>2, :albums_id=>6, :band_id=>2, :tracks_id=>1, :album_id=>6, :members_id=>6})
+ yield({:id=>2, :vocalist_id=>2, :albums_id=>6, :band_id=>2, :tracks_id=>2, :album_id=>6, :members_id=>5})
+ yield({:id=>2, :vocalist_id=>2, :albums_id=>6, :band_id=>2, :tracks_id=>2, :album_id=>6, :members_id=>6})
+ end
+ a = ds.all
+ a.should == [GraphBand.load(:id=>1, :vocalist_id=>2), GraphBand.load(:id=>2, :vocalist_id=>2)]
+ members = a.map{|x| x.members}
+ members.should == [[GraphBandMember.load(:id=>5), GraphBandMember.load(:id=>6)], [GraphBandMember.load(:id=>5), GraphBandMember.load(:id=>6)]]
+ albums = a.map{|x| x.albums}
+ albums.should == [[GraphAlbum.load(:id=>3, :band_id=>1), GraphAlbum.load(:id=>4, :band_id=>1)], [GraphAlbum.load(:id=>5, :band_id=>2), GraphAlbum.load(:id=>6, :band_id=>2)]]
+ tracks = albums.map{|x| x.map{|y| y.tracks}}
+ tracks.should == [[[GraphTrack.load(:id=>4, :album_id=>3), GraphTrack.load(:id=>5, :album_id=>3)], [GraphTrack.load(:id=>6, :album_id=>4), GraphTrack.load(:id=>7, :album_id=>4)]], [[GraphTrack.load(:id=>8, :album_id=>5), GraphTrack.load(:id=>9, :album_id=>5)], [GraphTrack.load(:id=>1, :album_id=>6), GraphTrack.load(:id=>2, :album_id=>6)]]]
+ end
+
it "should populate the reciprocal many_to_one association when eagerly loading the one_to_many association" do
MODEL_DB.reset
ds = GraphAlbum.eager_graph(:tracks)

0 comments on commit 022d4dd

Please sign in to comment.