aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord/test
diff options
context:
space:
mode:
authorSean Griffin <sean@seantheprogrammer.com>2016-01-12 14:34:42 -0700
committerSean Griffin <sean@seantheprogrammer.com>2016-01-12 14:38:25 -0700
commit5d41cb3bfd6b19833261622ce5d339b1e580bd8b (patch)
tree46ed63b4dd8f07e9b13abb0987eb3afe41962b1d /activerecord/test
parent858b9652ced5438127e6c6404bf3f9cae9699985 (diff)
downloadrails-5d41cb3bfd6b19833261622ce5d339b1e580bd8b.tar.gz
rails-5d41cb3bfd6b19833261622ce5d339b1e580bd8b.tar.bz2
rails-5d41cb3bfd6b19833261622ce5d339b1e580bd8b.zip
Change `WhereClause#merge` to same named columns on diff tables
While the predicates are an arel equality node where the left side is a full arel attribute, the binds just have the name of the column and nothing else. This means that while splitting the predicates can include the table as a factor, the binds cannot. It's entirely possible that we might be able to have the bind params carry a bit more information (I don't believe the name is used for anything but logging), and that is probably a worthwhile change to make in the future. However the simplest (and likely slightly faster) solution is to simply use the indices of the conflicts in both cases. This means that we only have to compute the collision space once, instead of twice even though we're doing an additional array iteration. Regardless, this method isn't a performance hotspot. Close #22823. [Ben Woosley & Sean Griffin]
Diffstat (limited to 'activerecord/test')
-rw-r--r--activerecord/test/cases/relation/where_clause_test.rb17
1 files changed, 15 insertions, 2 deletions
diff --git a/activerecord/test/cases/relation/where_clause_test.rb b/activerecord/test/cases/relation/where_clause_test.rb
index c20ed94d90..c3296b28fd 100644
--- a/activerecord/test/cases/relation/where_clause_test.rb
+++ b/activerecord/test/cases/relation/where_clause_test.rb
@@ -62,8 +62,21 @@ class ActiveRecord::Relation
end
test "merge allows for columns with the same name from different tables" do
- 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
+ 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)
end
test "a clause knows if it is empty" do