aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/CHANGELOG.md5
-rw-r--r--activerecord/lib/active_record/base.rb2
-rw-r--r--activerecord/lib/active_record/no_touching.rb4
-rw-r--r--activerecord/lib/active_record/relation/calculations.rb10
-rw-r--r--activerecord/lib/active_record/relation/predicate_builder.rb4
-rw-r--r--activerecord/lib/active_record/sanitization.rb9
-rw-r--r--activerecord/lib/active_record/table_metadata.rb4
-rw-r--r--activerecord/test/cases/associations/left_outer_join_association_test.rb25
-rw-r--r--activerecord/test/cases/finder_test.rb9
-rw-r--r--activerecord/test/cases/relation/where_test.rb2
-rw-r--r--activerecord/test/cases/relation_test.rb4
-rw-r--r--activerecord/test/cases/serialized_attribute_test.rb7
-rw-r--r--activerecord/test/cases/touch_later_test.rb9
13 files changed, 59 insertions, 35 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index bf81c23036..6b996fd6bd 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,8 @@
+* Doing count on relations that contain LEFT OUTER JOIN Arel node no longer
+ force a DISTINCT. This solves issues when using count after a left_joins.
+
+ *Maxime Handfield Lapointe*
+
* RecordNotFound raised by association.find exposes `id`, `primary_key` and
`model` methods to be consistent with RecordNotFound raised by Record.find.
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 01d7886406..1e7e939097 100644
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -312,8 +312,8 @@ module ActiveRecord #:nodoc:
include NestedAttributes
include Aggregations
include Transactions
- include NoTouching
include TouchLater
+ include NoTouching
include Reflection
include Serialization
include Store
diff --git a/activerecord/lib/active_record/no_touching.rb b/activerecord/lib/active_record/no_touching.rb
index edb5066fa0..4059020e25 100644
--- a/activerecord/lib/active_record/no_touching.rb
+++ b/activerecord/lib/active_record/no_touching.rb
@@ -45,6 +45,10 @@ module ActiveRecord
NoTouching.applied_to?(self.class)
end
+ def touch_later(*) # :nodoc:
+ super unless no_touching?
+ end
+
def touch(*) # :nodoc:
super unless no_touching?
end
diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb
index ecf3700aab..a4962879ab 100644
--- a/activerecord/lib/active_record/relation/calculations.rb
+++ b/activerecord/lib/active_record/relation/calculations.rb
@@ -117,7 +117,10 @@ module ActiveRecord
end
if has_include?(column_name)
- construct_relation_for_association_calculations.calculate(operation, column_name)
+ relation = construct_relation_for_association_calculations
+ relation = relation.distinct if operation.to_s.downcase == "count"
+
+ relation.calculate(operation, column_name)
else
perform_calculation(operation, column_name)
end
@@ -198,11 +201,6 @@ module ActiveRecord
if operation == "count"
column_name ||= select_for_count
-
- unless arel.ast.grep(Arel::Nodes::OuterJoin).empty?
- distinct = true
- end
-
column_name = primary_key if column_name == :all && distinct
distinct = nil if column_name =~ /\s*DISTINCT[\s(]+/i
end
diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb
index e874fab4bb..29422bf131 100644
--- a/activerecord/lib/active_record/relation/predicate_builder.rb
+++ b/activerecord/lib/active_record/relation/predicate_builder.rb
@@ -76,7 +76,7 @@ module ActiveRecord
return ["1=0"] if attributes.empty?
attributes.flat_map do |key, value|
- if value.is_a?(Hash)
+ if value.is_a?(Hash) && !table.has_column?(key)
associated_predicate_builder(key).expand_from_hash(value)
else
build(table.arel_attribute(key), value)
@@ -90,7 +90,7 @@ module ActiveRecord
attributes.each do |column_name, value|
case
- when value.is_a?(Hash)
+ when value.is_a?(Hash) && !table.has_column?(column_name)
attrs, bvs = associated_predicate_builder(column_name).create_binds_for_hash(value, options)
result[column_name] = attrs
binds += bvs
diff --git a/activerecord/lib/active_record/sanitization.rb b/activerecord/lib/active_record/sanitization.rb
index 6617008344..c40e98715e 100644
--- a/activerecord/lib/active_record/sanitization.rb
+++ b/activerecord/lib/active_record/sanitization.rb
@@ -5,13 +5,6 @@ module ActiveRecord
extend ActiveSupport::Concern
module ClassMethods
- # Used to sanitize objects before they're used in an SQL SELECT statement.
- # Delegates to {connection.quote}[rdoc-ref:ConnectionAdapters::Quoting#quote].
- def sanitize(object) # :nodoc:
- connection.quote(object)
- end
- alias_method :quote_value, :sanitize
-
protected
# Accepts an array or string of SQL conditions and sanitizes
@@ -216,7 +209,7 @@ module ActiveRecord
# TODO: Deprecate this
def quoted_id # :nodoc:
- self.class.quote_value(@attributes[self.class.primary_key].value_for_database)
+ self.class.connection.quote(@attributes[self.class.primary_key].value_for_database)
end
end
end
diff --git a/activerecord/lib/active_record/table_metadata.rb b/activerecord/lib/active_record/table_metadata.rb
index e8d6a144f9..0ca880e635 100644
--- a/activerecord/lib/active_record/table_metadata.rb
+++ b/activerecord/lib/active_record/table_metadata.rb
@@ -37,6 +37,10 @@ module ActiveRecord
end
end
+ def has_column?(column_name)
+ klass && klass.columns_hash.key?(column_name.to_s)
+ end
+
def associated_with?(association_name)
klass && klass._reflect_on_association(association_name)
end
diff --git a/activerecord/test/cases/associations/left_outer_join_association_test.rb b/activerecord/test/cases/associations/left_outer_join_association_test.rb
index d814229f64..2cc6468827 100644
--- a/activerecord/test/cases/associations/left_outer_join_association_test.rb
+++ b/activerecord/test/cases/associations/left_outer_join_association_test.rb
@@ -25,23 +25,32 @@ class LeftOuterJoinAssociationTest < ActiveRecord::TestCase
end
end
- def test_construct_finder_sql_executes_a_left_outer_join
- assert_not_equal Author.count, Author.joins(:posts).count
- assert_equal Author.count, Author.left_outer_joins(:posts).count
+ def test_left_outer_joins_count_is_same_as_size_of_loaded_results
+ assert_equal 17, Post.left_outer_joins(:comments).to_a.size
+ assert_equal 17, Post.left_outer_joins(:comments).count
end
- def test_left_outer_join_by_left_joins
- assert_not_equal Author.count, Author.joins(:posts).count
- assert_equal Author.count, Author.left_joins(:posts).count
+ def test_left_joins_aliases_left_outer_joins
+ assert_equal Post.left_outer_joins(:comments).to_sql, Post.left_joins(:comments).to_sql
+ end
+
+ def test_left_outer_joins_return_has_value_for_every_comment
+ all_post_ids = Post.pluck(:id)
+ assert_equal all_post_ids, all_post_ids & Post.left_outer_joins(:comments).pluck(:id)
+ end
+
+ def test_left_outer_joins_actually_does_a_left_outer_join
+ queries = capture_sql { Author.left_outer_joins(:posts).to_a }
+ assert queries.any? { |sql| /LEFT OUTER JOIN/i.match?(sql) }
end
def test_construct_finder_sql_ignores_empty_left_outer_joins_hash
- queries = capture_sql { Author.left_outer_joins({}) }
+ queries = capture_sql { Author.left_outer_joins({}).to_a }
assert queries.none? { |sql| /LEFT OUTER JOIN/i.match?(sql) }
end
def test_construct_finder_sql_ignores_empty_left_outer_joins_array
- queries = capture_sql { Author.left_outer_joins([]) }
+ queries = capture_sql { Author.left_outer_joins([]).to_a }
assert queries.none? { |sql| /LEFT OUTER JOIN/i.match?(sql) }
end
diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb
index f8f9f2d383..31bc4fa1f2 100644
--- a/activerecord/test/cases/finder_test.rb
+++ b/activerecord/test/cases/finder_test.rb
@@ -22,13 +22,13 @@ class FinderTest < ActiveRecord::TestCase
fixtures :companies, :topics, :entrants, :developers, :developers_projects, :posts, :comments, :accounts, :authors, :author_addresses, :customers, :categories, :categorizations, :cars
def test_find_by_id_with_hash
- assert_raises(ActiveRecord::StatementInvalid) do
+ assert_nothing_raised do
Post.find_by_id(limit: 1)
end
end
def test_find_by_title_and_id_with_hash
- assert_raises(ActiveRecord::StatementInvalid) do
+ assert_nothing_raised do
Post.find_by_title_and_id("foo", limit: 1)
end
end
@@ -874,11 +874,6 @@ class FinderTest < ActiveRecord::TestCase
assert_kind_of Time, Topic.where(["id = :id", { id: 1 }]).first.written_on
end
- def test_string_sanitation
- assert_not_equal "'something ' 1=1'", ActiveRecord::Base.sanitize("something ' 1=1")
- assert_equal "'something; select table'", ActiveRecord::Base.sanitize("something; select table")
- end
-
def test_count_by_sql
assert_equal(0, Entrant.count_by_sql("SELECT COUNT(*) FROM entrants WHERE id > 3"))
assert_equal(1, Entrant.count_by_sql(["SELECT COUNT(*) FROM entrants WHERE id > ?", 2]))
diff --git a/activerecord/test/cases/relation/where_test.rb b/activerecord/test/cases/relation/where_test.rb
index ad9008ea4d..ce4e041793 100644
--- a/activerecord/test/cases/relation/where_test.rb
+++ b/activerecord/test/cases/relation/where_test.rb
@@ -196,7 +196,7 @@ module ActiveRecord
end
def test_where_error
- assert_raises(ActiveRecord::StatementInvalid) do
+ assert_nothing_raised do
Post.where(id: { "posts.author_id" => 10 }).first
end
end
diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb
index 951b83e87b..103075d09c 100644
--- a/activerecord/test/cases/relation_test.rb
+++ b/activerecord/test/cases/relation_test.rb
@@ -223,7 +223,7 @@ module ActiveRecord
def test_relation_merging_with_merged_joins_as_symbols
special_comments_with_ratings = SpecialComment.joins(:ratings)
posts_with_special_comments_with_ratings = Post.group("posts.id").joins(:special_comments).merge(special_comments_with_ratings)
- assert_equal 3, authors(:david).posts.merge(posts_with_special_comments_with_ratings).count.length
+ assert_equal({ 2=>1, 4=>3, 5=>1 }, authors(:david).posts.merge(posts_with_special_comments_with_ratings).count)
end
def test_relation_merging_with_joins_as_join_dependency_pick_proper_parent
@@ -273,7 +273,7 @@ module ActiveRecord
join_string = "LEFT OUTER JOIN #{Rating.quoted_table_name} ON #{SpecialComment.quoted_table_name}.id = #{Rating.quoted_table_name}.comment_id"
special_comments_with_ratings = SpecialComment.joins join_string
posts_with_special_comments_with_ratings = Post.group("posts.id").joins(:special_comments).merge(special_comments_with_ratings)
- assert_equal 3, authors(:david).posts.merge(posts_with_special_comments_with_ratings).count.length
+ assert_equal({ 2=>1, 4=>3, 5=>1 }, authors(:david).posts.merge(posts_with_special_comments_with_ratings).count)
end
class EnsureRoundTripTypeCasting < ActiveRecord::Type::Value
diff --git a/activerecord/test/cases/serialized_attribute_test.rb b/activerecord/test/cases/serialized_attribute_test.rb
index ceb0d5441b..bebd856faf 100644
--- a/activerecord/test/cases/serialized_attribute_test.rb
+++ b/activerecord/test/cases/serialized_attribute_test.rb
@@ -157,6 +157,13 @@ class SerializedAttributeTest < ActiveRecord::TestCase
assert_equal(settings, Topic.find(topic.id).content)
end
+ def test_where_by_serialized_attribute_with_hash
+ settings = { "color" => "green" }
+ Topic.serialize(:content, Hash)
+ topic = Topic.create!(content: settings)
+ assert_equal topic, Topic.where(content: settings).take
+ end
+
def test_serialized_default_class
Topic.serialize(:content, Hash)
topic = Topic.new
diff --git a/activerecord/test/cases/touch_later_test.rb b/activerecord/test/cases/touch_later_test.rb
index 697447a4f3..d1e8c649d9 100644
--- a/activerecord/test/cases/touch_later_test.rb
+++ b/activerecord/test/cases/touch_later_test.rb
@@ -24,6 +24,15 @@ class TouchLaterTest < ActiveRecord::TestCase
assert_not invoice.changed?
end
+ def test_touch_later_respects_no_touching_policy
+ time = Time.now.utc - 25.days
+ topic = Topic.create!(updated_at: time, created_at: time)
+ Topic.no_touching do
+ topic.touch_later
+ end
+ assert_equal time.to_i, topic.updated_at.to_i
+ end
+
def test_touch_later_update_the_attributes
time = Time.now.utc - 25.days
topic = Topic.create!(updated_at: time, created_at: time)