Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Relation::Merger should not fill values with empty values #29914

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 12 additions & 10 deletions activerecord/lib/active_record/relation/merger.rb
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -901,16 +901,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
Original file line number Diff line number Diff line change
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