Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Make eager_graph respect associations' :order options (use :order_eag…

…er_graph=>false to disable)

This changes eager_graph so that it will add the columns specified in
each graphed association's :order option to the list of ordered
columns.  It qualifies the column with the table alias used for that
join.

This shouldn't cause problems unless:

1) You have an already qualified column in the :order option.

2) You have a column in the :order option that wasn't in the primary
   table (maybe because you were using the :eager_graph association
   option to include the table whose column you were using).

While remote, those problems can happen, and you can turn off the
including of the associations :order option by setting the
association's :order_eager_graph option to false.

This commit also fixes a bug in eager_graph when you were loading
an association with the same name as the base table in the FROM
clause.
  • Loading branch information...
commit be4d7161060fabbe41550e0e04f156cf212ca972 1 parent 70c9e90
@jeremyevans authored
View
2  CHANGELOG
@@ -1,5 +1,7 @@
=== HEAD
+* Make eager_graph respect associations' :order options (use :order_eager_graph=>false to disable) (jeremyevans)
+
* Cache negative lookup when eagerly loading many_to_one associations where no objects have an associated object (jeremyevans)
* Allow string keys to be used when using Dataset#multi_insert (jeremyevans)
View
3  lib/sequel_model/associations.rb
@@ -135,6 +135,8 @@ def all_association_reflections
# an array with two arguments for the value to specify a limit and offset.
# - :order - the column(s) by which to order the association dataset. Can be a
# singular column or an array.
+ # - :order_eager_graph - Whether to add the order to the dataset's order when graphing
+ # via eager graph. Defaults to true, so set to false to disable.
# - :read_only - Do not add a setter method (for many_to_one or one_to_many with :one_to_one),
# or add_/remove_/remove_all_ methods (for one_to_many, many_to_many)
# - :reciprocal - the symbol name of the reciprocal association,
@@ -192,6 +194,7 @@ def associate(type, name, opts = {}, &block)
opts = AssociationReflection.new.merge!(opts)
opts[:eager_block] = block unless opts.include?(:eager_block)
opts[:graph_join_type] ||= :left_outer
+ opts[:order_eager_graph] = true unless opts.include?(:order_eager_graph)
opts[:graph_conditions] = opts[:graph_conditions] ? opts[:graph_conditions].to_a : []
opts[:graph_select] = Array(opts[:graph_select]) if opts[:graph_select]
[:before_add, :before_remove, :after_add, :after_remove, :after_load, :extend].each do |cb_type|
View
17 lib/sequel_model/eager_loading.rb
@@ -87,10 +87,8 @@ def eager(*associations)
# create large cartesian products. If you must graph multiple *_to_many associations,
# make sure your filters are specific if you have a large database.
#
- # This does not respect each association's order, as all associations are loaded in
- # a single query. If you want to order the results, you must manually call .order.
- #
- # #eager_graph probably won't work correctly on a limited dataset, unless you are
+ # Each association's order, if definied, is respected. #eager_graph probably
+ # won't work correctly on a limited dataset, unless you are
# only graphing many_to_one associations.
#
# Does not use the block defined for the association, since it does a single query for
@@ -149,6 +147,7 @@ def eager_graph_association(ds, model, ta, requirements, r, *associations)
ds = ds.graph(r[:join_table], use_jt_only_conditions ? r[:graph_join_table_only_conditions] : [[r[:left_key], model.primary_key.qualify(ta)]] + r[:graph_join_table_conditions], :select=>false, :table_alias=>ds.eager_unique_table_alias(ds, r[:join_table]), :join_type=>r[:graph_join_table_join_type], &r[:graph_join_table_block])
ds.graph(klass, use_only_conditions ? only_conditions : [[klass.primary_key, r[:right_key]]] + conditions, :select=>select, :table_alias=>assoc_table_alias, :join_type=>join_type, &graph_block)
end
+ ds = ds.order_more(*(Array(r[:order]).map{|c| Sequel::SQL::QualifiedIdentifier.new(assoc_table_alias, c)})) if r[:order] and r[:order_eager_graph]
eager_graph = ds.opts[:eager_graph]
eager_graph[:requirements][assoc_table_alias] = requirements.dup
eager_graph[:alias_association_name_map][assoc_table_alias] = assoc_name
@@ -250,16 +249,18 @@ def eager_graph_build_associations(record_graphs)
# Will either be the table_alias itself or table_alias_N for some integer
# N (starting at 0 and increasing until an unused one is found).
def eager_unique_table_alias(ds, table_alias)
- if (graph = ds.opts[:graph]) && (table_aliases = graph[:table_aliases]) && (table_aliases.include?(table_alias))
+ used_aliases = ds.opts[:from]
+ graph = ds.opts[:graph]
+ used_aliases += graph[:table_aliases].keys if graph
+ if used_aliases.include?(table_alias)
i = 0
loop do
ta = :"#{table_alias}_#{i}"
- return ta unless table_aliases[ta]
+ return ta unless used_aliases.include?(ta)
i += 1
end
- else
- table_alias
end
+ table_alias
end
private
View
29 spec/sequel_model/eager_loading_spec.rb
@@ -992,4 +992,33 @@ def ds.fetch_rows(sql, &block)
it "should create unique table aliases for all associations" do
GraphAlbum.eager_graph(:previous_album=>{:previous_album=>:previous_album}).sql.should == "SELECT albums.id, albums.band_id, previous_album.id AS previous_album_id, previous_album.band_id AS previous_album_band_id, previous_album_0.id AS previous_album_0_id, previous_album_0.band_id AS previous_album_0_band_id, previous_album_1.id AS previous_album_1_id, previous_album_1.band_id AS previous_album_1_band_id FROM albums LEFT OUTER JOIN albums AS previous_album ON (previous_album.id = albums.previous_album_id) LEFT OUTER JOIN albums AS previous_album_0 ON (previous_album_0.id = previous_album.previous_album_id) LEFT OUTER JOIN albums AS previous_album_1 ON (previous_album_1.id = previous_album_0.previous_album_id)"
end
+
+ it "should respect the associations :order" do
+ GraphAlbum.one_to_many :right_tracks, :class=>'GraphTrack', :key=>:album_id, :order=>[:id, :album_id]
+ GraphAlbum.eager_graph(:right_tracks).sql.should == 'SELECT albums.id, albums.band_id, right_tracks.id AS right_tracks_id, right_tracks.album_id FROM albums LEFT OUTER JOIN tracks AS right_tracks ON (right_tracks.album_id = albums.id) ORDER BY right_tracks.id, right_tracks.album_id'
+ end
+
+ it "should not respect the associations :order if :order_eager_graph is false" do
+ GraphAlbum.one_to_many :right_tracks, :class=>'GraphTrack', :key=>:album_id, :order=>[:id, :album_id], :order_eager_graph=>false
+ GraphAlbum.eager_graph(:right_tracks).sql.should == 'SELECT albums.id, albums.band_id, right_tracks.id AS right_tracks_id, right_tracks.album_id FROM albums LEFT OUTER JOIN tracks AS right_tracks ON (right_tracks.album_id = albums.id)'
+ end
+
+ it "should add the associations :order to the existing order" do
+ GraphAlbum.one_to_many :right_tracks, :class=>'GraphTrack', :key=>:album_id, :order=>[:id, :album_id]
+ GraphAlbum.order(:band_id).eager_graph(:right_tracks).sql.should == 'SELECT albums.id, albums.band_id, right_tracks.id AS right_tracks_id, right_tracks.album_id FROM albums LEFT OUTER JOIN tracks AS right_tracks ON (right_tracks.album_id = albums.id) ORDER BY band_id, right_tracks.id, right_tracks.album_id'
+ end
+
+ it "should add the associations :order for cascading associations" do
+ GraphBand.one_to_many :a_albums, :class=>'GraphAlbum', :key=>:band_id, :order=>:name
+ GraphAlbum.one_to_many :b_tracks, :class=>'GraphTrack', :key=>:album_id, :order=>[:id, :album_id]
+ GraphBand.eager_graph(:a_albums=>:b_tracks).sql.should == 'SELECT bands.id, bands.vocalist_id, a_albums.id AS a_albums_id, a_albums.band_id, b_tracks.id AS b_tracks_id, b_tracks.album_id FROM bands LEFT OUTER JOIN albums AS a_albums ON (a_albums.band_id = bands.id) LEFT OUTER JOIN tracks AS b_tracks ON (b_tracks.album_id = a_albums.id) ORDER BY a_albums.name, b_tracks.id, b_tracks.album_id'
+ GraphAlbum.one_to_many :albums, :class=>'GraphAlbum', :key=>:band_id, :order=>[:band_id, :id]
+ GraphAlbum.eager_graph(:albums=>{:albums=>:albums}).sql.should == 'SELECT albums.id, albums.band_id, albums_0.id AS albums_0_id, albums_0.band_id AS albums_0_band_id, albums_1.id AS albums_1_id, albums_1.band_id AS albums_1_band_id, albums_2.id AS albums_2_id, albums_2.band_id AS albums_2_band_id FROM albums LEFT OUTER JOIN albums AS albums_0 ON (albums_0.band_id = albums.id) LEFT OUTER JOIN albums AS albums_1 ON (albums_1.band_id = albums_0.id) LEFT OUTER JOIN albums AS albums_2 ON (albums_2.band_id = albums_1.id) ORDER BY albums_0.band_id, albums_0.id, albums_1.band_id, albums_1.id, albums_2.band_id, albums_2.id'
+ end
+
+ it "should add the associations :order for multiple associations" do
+ GraphAlbum.many_to_many :a_genres, :class=>'GraphGenre', :left_key=>:album_id, :right_key=>:genre_id, :join_table=>:ag, :order=>:id
+ GraphAlbum.one_to_many :b_tracks, :class=>'GraphTrack', :key=>:album_id, :order=>[:id, :album_id]
+ GraphAlbum.eager_graph(:a_genres, :b_tracks).sql.should == 'SELECT albums.id, albums.band_id, a_genres.id AS a_genres_id, b_tracks.id AS b_tracks_id, b_tracks.album_id FROM albums LEFT OUTER JOIN ag ON (ag.album_id = albums.id) LEFT OUTER JOIN genres AS a_genres ON (a_genres.id = ag.genre_id) LEFT OUTER JOIN tracks AS b_tracks ON (b_tracks.album_id = albums.id) ORDER BY a_genres.id, b_tracks.id, b_tracks.album_id'
+ end
end
Please sign in to comment.
Something went wrong with that request. Please try again.