From cd60828b591b50b456df562786e484ca5f6841bd Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Fri, 19 Jan 2024 14:42:19 -0800 Subject: [PATCH] Warn by default if trying to eager_graph/association_join an association 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. --- CHANGELOG | 2 ++ lib/sequel/model/associations.rb | 11 +++++++++-- spec/model/eager_loading_spec.rb | 8 ++++++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 2db0322a06..77bf103c0e 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -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) diff --git a/lib/sequel/model/associations.rb b/lib/sequel/model/associations.rb index 3bee2b791f..3be56e2fad 100644 --- a/lib/sequel/model/associations.rb +++ b/lib/sequel/model/associations.rb @@ -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]) diff --git a/spec/model/eager_loading_spec.rb b/spec/model/eager_loading_spec.rb index 654e65322b..15d894d99c 100644 --- a/spec/model/eager_loading_spec.rb +++ b/spec/model/eager_loading_spec.rb @@ -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