diff options
| author | Sean Griffin <sean@seantheprogrammer.com> | 2016-01-12 15:47:05 -0700 |
|---|---|---|
| committer | Sean Griffin <sean@seantheprogrammer.com> | 2016-01-12 15:48:00 -0700 |
| commit | b64b7545106db0744c054ef615a952fa1e962e60 (patch) | |
| tree | 07e5949c034e812a1c0a95eaea1a3664881c1cbb /activerecord | |
| parent | dd731446587d6b97fbe0bcd73e226952d549f5ba (diff) | |
| download | rails-b64b7545106db0744c054ef615a952fa1e962e60.tar.gz rails-b64b7545106db0744c054ef615a952fa1e962e60.tar.bz2 rails-b64b7545106db0744c054ef615a952fa1e962e60.zip | |
Revert "Change `WhereClause#merge` to same named columns on diff tables"
This reverts commit 5d41cb3bfd6b19833261622ce5d339b1e580bd8b.
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.
Diffstat (limited to 'activerecord')
| -rw-r--r-- | activerecord/lib/active_record/relation/where_clause.rb | 21 | ||||
| -rw-r--r-- | activerecord/test/cases/relation/where_clause_test.rb | 17 |
2 files changed, 13 insertions, 25 deletions
diff --git a/activerecord/lib/active_record/relation/where_clause.rb b/activerecord/lib/active_record/relation/where_clause.rb index e39dbac0d5..2c2d6cfa47 100644 --- a/activerecord/lib/active_record/relation/where_clause.rb +++ b/activerecord/lib/active_record/relation/where_clause.rb @@ -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 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 diff --git a/activerecord/test/cases/relation/where_clause_test.rb b/activerecord/test/cases/relation/where_clause_test.rb index c3296b28fd..c20ed94d90 100644 --- a/activerecord/test/cases/relation/where_clause_test.rb +++ b/activerecord/test/cases/relation/where_clause_test.rb @@ -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 |