Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

make it possible to disable implicit join references. #9733

Merged
merged 1 commit into from

4 participants

@senny
Owner

Closes #9712.

The feature can be disabled completely.

# in your configuration
config.active_record.disable_implicit_join_references = true

# or directly
ActiveRecord::Base.disable_implicit_join_references = true
@senny
Owner

@jonleighton what do you think?

@jonleighton
Collaborator

@senny seems good, but please update the deprecation warning so that people know they can use this option to quiet the deprecation.

@senny
Owner

@jonleighton thanks, very good idea. I pushed an updated version.

@jonleighton jonleighton merged commit 5558bb0 into rails:master
@sadfuzzy

Great commit!

@senny senny deleted the senny:9712_option_to_turn_references_deprecation_off branch
@mitio mitio commented on the diff
activerecord/test/cases/relations_test.rb
@@ -1224,6 +1224,15 @@ def test_eager_loading_with_conditions_on_joins
end
end
+ def test_turn_off_eager_loading_with_conditions_on_joins
+ original_value = ActiveRecord::Base.disable_implicit_join_references
+ ActiveRecord::Base.disable_implicit_join_references = true
+ scope = Topic.where(author_email_address: 'my.example@gmail.com').includes(:replies)
+ assert_not scope.eager_loading?
+ ensure
+ ActiveRecord::Base.time_zone_aware_attributes = original_value
@mitio
mitio added a note

Wait, @senny, shouldn't this be ActiveRecord::Base.disable_implicit_join_references = original_value?

@senny Owner
senny added a note

damn copy&paste mistakes :cold_sweat:. Thanks @mitio for reporting. Gonna fix ASAP.

@senny Owner
senny added a note

@carlosantoniodasilva already cleaned up after me :grin: a662a78

@mitio
mitio added a note

Ah, didn't see that... :+1: Next time I'll check before raising an alarm :smile:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 15, 2013
  1. @senny
This page is out of date. Refresh to see the latest.
View
15 activerecord/CHANGELOG.md
@@ -1,5 +1,20 @@
## Rails 4.0.0 (unreleased) ##
+* Referencing join tables implicitly was deprecated. There is a
+ possibility that these deprecation warnings are shown even if you
+ don't make use of that feature. You can now disable the feature entirely.
+ Fixes #9712.
+
+ Example:
+
+ # in your configuration
+ config.active_record.disable_implicit_join_references = true
+
+ # or directly
+ ActiveRecord::Base.disable_implicit_join_references = true
+
+ *Yves Senn*
+
* The `:distinct` option for `Relation#count` is deprecated. You
should use `Relation#distinct` instead.
View
8 activerecord/lib/active_record/core.rb
@@ -69,6 +69,14 @@ module Core
mattr_accessor :timestamped_migrations, instance_writer: false
self.timestamped_migrations = true
+ ##
+ # :singleton-method:
+ # Disable implicit join references. This feature was deprecated with Rails 4.
+ # If you don't make use of implicit references but still see deprecation warnings
+ # you can disable the feature entirely. This will be the default with Rails 4.1.
+ mattr_accessor :disable_implicit_join_references, instance_writer: false
+ self.disable_implicit_join_references = false
+
class_attribute :connection_handler, instance_writer: false
self.connection_handler = ConnectionAdapters::ConnectionHandler.new
end
View
8 activerecord/lib/active_record/relation.rb
@@ -595,7 +595,8 @@ def references_eager_loaded_tables?
if (references_values - joined_tables).any?
true
- elsif (string_tables - joined_tables).any?
+ elsif !ActiveRecord::Base.disable_implicit_join_references &&
+ (string_tables - joined_tables).any?
ActiveSupport::Deprecation.warn(
"It looks like you are eager loading table(s) (one of: #{string_tables.join(', ')}) " \
"that are referenced in a string SQL snippet. For example: \n" \
@@ -609,7 +610,10 @@ def references_eager_loaded_tables?
"From now on, you must explicitly tell Active Record when you are referencing a table " \
"from a string:\n" \
"\n" \
- " Post.includes(:comments).where(\"comments.title = 'foo'\").references(:comments)\n\n"
+ " Post.includes(:comments).where(\"comments.title = 'foo'\").references(:comments)\n" \
+ "\n" \
+ "If you don't rely on implicit join references you can disable the feature entirely" \
+ "by setting `config.active_record.disable_implicit_join_references = true`."
)
true
else
View
9 activerecord/test/cases/relations_test.rb
@@ -1224,6 +1224,15 @@ def test_eager_loading_with_conditions_on_joins
end
end
+ def test_turn_off_eager_loading_with_conditions_on_joins
+ original_value = ActiveRecord::Base.disable_implicit_join_references
+ ActiveRecord::Base.disable_implicit_join_references = true
+ scope = Topic.where(author_email_address: 'my.example@gmail.com').includes(:replies)
+ assert_not scope.eager_loading?
+ ensure
+ ActiveRecord::Base.time_zone_aware_attributes = original_value
@mitio
mitio added a note

Wait, @senny, shouldn't this be ActiveRecord::Base.disable_implicit_join_references = original_value?

@senny Owner
senny added a note

damn copy&paste mistakes :cold_sweat:. Thanks @mitio for reporting. Gonna fix ASAP.

@senny Owner
senny added a note

@carlosantoniodasilva already cleaned up after me :grin: a662a78

@mitio
mitio added a note

Ah, didn't see that... :+1: Next time I'll check before raising an alarm :smile:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ end
+
def test_ordering_with_extra_spaces
assert_equal authors(:david), Author.order('id DESC , name DESC').last
end
Something went wrong with that request. Please try again.