mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Revert "Change WhereClause#merge
to same named columns on diff tables"
This reverts commit 5d41cb3bfd
.
This implementation does not properly handle cases involving predicates
which are not associated with a bind param. I have the fix in mind, but
don't have time to implement just yet. It will be more similar to #22823
than not.
This commit is contained in:
parent
dd73144658
commit
b64b754510
2 changed files with 13 additions and 25 deletions
|
@ -18,10 +18,9 @@ module ActiveRecord
|
|||
end
|
||||
|
||||
def merge(other)
|
||||
conflict_indices = indices_of_predicates_referenced_by(other).to_set
|
||||
WhereClause.new(
|
||||
non_conflicting(predicates, conflict_indices) + other.predicates,
|
||||
non_conflicting(binds, conflict_indices) + other.binds,
|
||||
predicates_unreferenced_by(other) + other.predicates,
|
||||
non_conflicting_binds(other) + other.binds,
|
||||
)
|
||||
end
|
||||
|
||||
|
@ -98,20 +97,22 @@ module ActiveRecord
|
|||
|
||||
private
|
||||
|
||||
def indices_of_predicates_referenced_by(other)
|
||||
predicates.each_with_index.select do |(n, _)|
|
||||
def predicates_unreferenced_by(other)
|
||||
predicates.reject do |n|
|
||||
equality_node?(n) && other.referenced_columns.include?(n.left)
|
||||
end.map(&:last)
|
||||
end
|
||||
|
||||
def non_conflicting(values, conflict_indices)
|
||||
values.reject.with_index { |_, i| conflict_indices.include?(i) }
|
||||
end
|
||||
|
||||
def equality_node?(node)
|
||||
node.respond_to?(:operator) && node.operator == :==
|
||||
end
|
||||
|
||||
def non_conflicting_binds(other)
|
||||
conflicts = referenced_columns & other.referenced_columns
|
||||
conflicts.map! { |node| node.name.to_s }
|
||||
binds.reject { |attr| conflicts.include?(attr.name) }
|
||||
end
|
||||
|
||||
def inverted_predicates
|
||||
predicates.map { |node| invert_predicate(node) }
|
||||
end
|
||||
|
|
|
@ -62,21 +62,8 @@ class ActiveRecord::Relation
|
|||
end
|
||||
|
||||
test "merge allows for columns with the same name from different tables" do
|
||||
table2 = Arel::Table.new("table2")
|
||||
a = WhereClause.new(
|
||||
[table["id"].eq(bind_param), table2["id"].eq(bind_param), table["name"].eq(bind_param)],
|
||||
[attribute("id", 3), attribute("id", 2), attribute("name", "Jim")]
|
||||
)
|
||||
b = WhereClause.new(
|
||||
[table["id"].eq(bind_param), table["name"].eq(bind_param)],
|
||||
[attribute("id", 1), attribute("name", "Sean")],
|
||||
)
|
||||
expected = WhereClause.new(
|
||||
[table2["id"].eq(bind_param), table["id"].eq(bind_param), table["name"].eq(bind_param)],
|
||||
[attribute("id", 2), attribute("id", 1), attribute("name", "Sean")],
|
||||
)
|
||||
|
||||
assert_equal expected, a.merge(b)
|
||||
skip "This is not possible as of 4.2, and the binds do not yet contain sufficient information for this to happen"
|
||||
# We might be able to change the implementation to remove conflicts by index, rather than column name
|
||||
end
|
||||
|
||||
test "a clause knows if it is empty" do
|
||||
|
|
Loading…
Reference in a new issue