Skip to content

Commit

Permalink
Merge pull request #29914 from kamipo/relation_merger_should_not_fill…
Browse files Browse the repository at this point in the history
…_empty_values

`Relation::Merger` should not fill `values` with empty values
  • Loading branch information
matthewd committed Aug 2, 2017
2 parents 75436a4 + f0e6ecc commit 686261f
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 19 deletions.
22 changes: 12 additions & 10 deletions activerecord/lib/active_record/relation/merger.rb
Expand Up @@ -135,31 +135,33 @@ def merge_multi_values
if other.reordering_value
# override any order specified in the original relation
relation.reorder! other.order_values
elsif other.order_values
elsif other.order_values.any?
# merge in order_values from relation
relation.order! other.order_values
end

relation.extend(*other.extending_values) unless other.extending_values.blank?
extensions = other.extensions - relation.extensions
relation.extending!(*extensions) if extensions.any?
end

def merge_single_values
if relation.from_clause.empty?
relation.from_clause = other.from_clause
end
relation.lock_value ||= other.lock_value
relation.lock_value ||= other.lock_value if other.lock_value

unless other.create_with_value.blank?
relation.create_with_value = (relation.create_with_value || {}).merge(other.create_with_value)
end
end

def merge_clauses
CLAUSE_METHODS.each do |method|
clause = relation.get_value(method)
other_clause = other.get_value(method)
relation.set_value(method, clause.merge(other_clause))
if relation.from_clause.empty? && !other.from_clause.empty?
relation.from_clause = other.from_clause
end

where_clause = relation.where_clause.merge(other.where_clause)
relation.where_clause = where_clause unless where_clause.empty?

having_clause = relation.having_clause.merge(other.having_clause)
relation.having_clause = having_clause unless having_clause.empty?
end
end
end
Expand Down
19 changes: 10 additions & 9 deletions activerecord/lib/active_record/relation/query_methods.rb
Expand Up @@ -902,16 +902,17 @@ def arel # :nodoc:
@arel ||= build_arel
end

# Returns a relation value with a given name
def get_value(name) # :nodoc:
@values[name] || default_value_for(name)
end
protected
# Returns a relation value with a given name
def get_value(name) # :nodoc:
@values[name] || default_value_for(name)
end

# Sets the relation value with the given name
def set_value(name, value) # :nodoc:
assert_mutability!
@values[name] = value
end
# Sets the relation value with the given name
def set_value(name, value) # :nodoc:
assert_mutability!
@values[name] = value
end

private

Expand Down
8 changes: 8 additions & 0 deletions activerecord/test/cases/relation_test.rb
Expand Up @@ -100,6 +100,14 @@ def test_create_with_value_with_wheres
assert_equal({ "hello" => "world", "id" => 10 }, relation.scope_for_create)
end

def test_empty_scope
relation = Relation.new(Post, Post.arel_table, Post.predicate_builder)
assert relation.empty_scope?

relation.merge!(relation)
assert relation.empty_scope?
end

def test_bad_constants_raise_errors
assert_raises(NameError) do
ActiveRecord::Relation::HelloWorld
Expand Down

0 comments on commit 686261f

Please sign in to comment.