Skip to content

Commit

Permalink
Warn by default if trying to eager_graph/association_join an associat…
Browse files Browse the repository at this point in the history
…ion that uses a block, when the block would be ignored

The auto_restrict_eager_graph plugin raises an error in these
cases.  That will be the default behavior in Sequel 6, but until
then, warn in this case so users know they should fix this.
  • Loading branch information
jeremyevans committed Jan 19, 2024
1 parent e296f6e commit cd60828
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 2 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
=== master

* Warn by default if trying to eager_graph/association_join an association that uses a block, when the block would be ignored (jeremyevans)

* Speed up validates_unique in validation_helpers plugin by using empty? instead of count == 0 (numbata) (#2122)

* Speed up regexp matches in sqlite adapter on Ruby 2.4+ (jeremyevans)
Expand Down
11 changes: 9 additions & 2 deletions lib/sequel/model/associations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3387,8 +3387,15 @@ def eager_graph_association(ds, model, ta, requirements, r, *associations)
local_opts = ds.opts[:eager_graph][:local]
limit_strategy = r.eager_graph_limit_strategy(local_opts[:limit_strategy])

if r[:conditions] && !Sequel.condition_specifier?(r[:conditions]) && !r[:orig_opts].has_key?(:graph_conditions) && !r[:orig_opts].has_key?(:graph_only_conditions) && !r.has_key?(:graph_block)
raise Error, "Cannot eager_graph association when :conditions specified and not a hash or an array of pairs. Specify :graph_conditions, :graph_only_conditions, or :graph_block for the association. Model: #{r[:model]}, association: #{r[:name]}"
# SEQUEL6: remove and integrate the auto_restrict_eager_graph plugin
if !r[:orig_opts].has_key?(:graph_conditions) && !r[:orig_opts].has_key?(:graph_only_conditions) && !r.has_key?(:graph_block) && !r[:allow_eager_graph]
if r[:conditions] && !Sequel.condition_specifier?(r[:conditions])
raise Error, "Cannot eager_graph association when :conditions specified and not a hash or an array of pairs. Specify :graph_conditions, :graph_only_conditions, or :graph_block for the association. Model: #{r[:model]}, association: #{r[:name]}"
end

if r[:block] && !r[:graph_use_association_block]
warn "eager_graph used for association when association given a block without graph options. The block is ignored in this case. This will result in an exception starting in Sequel 6. Model: #{r[:model]}, association: #{r[:name]}"
end
end

ds = loader.call(:self=>ds, :table_alias=>assoc_table_alias, :implicit_qualifier=>(ta == ds.opts[:eager_graph][:master]) ? first_source : qualifier_from_alias_symbol(ta, first_source), :callback=>callback, :join_type=>join_type || local_opts[:join_type], :join_only=>local_opts[:join_only], :limit_strategy=>limit_strategy, :from_self_alias=>ds.opts[:eager_graph][:master])
Expand Down
8 changes: 8 additions & 0 deletions spec/model/eager_loading_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1863,6 +1863,14 @@ def columns; literal(opts[:select]) =~ /x_foreign_key_x/ ? [:id, :x_foreign_key_
proc{c.eager_graph(:genres)}.must_raise Sequel::Error
end

it "should warn when using eager_graph with association with block" do
c = Class.new(GraphAlbum)
c.many_to_many :genres, :clone=>:genres, :join_table=>Sequel[:ag].as(:ga) do |ds| ds end
m = nil
c.dataset.with_extend{define_method(:warn){|msg| m = msg}}.eager_graph(:genres)
m.must_include 'eager_graph used for association when association given a block without graph options. The block is ignored in this case. This will result in an exception starting in Sequel 6'
end

with_symbol_splitting "should correctly handle an aliased join table symbol in many_to_many and one_through_one" do
c = Class.new(GraphAlbum)
c.many_to_many :genres, :clone=>:genres, :join_table=>:ag___ga
Expand Down

0 comments on commit cd60828

Please sign in to comment.