Merge pull request #34437 from kbrock/union_all_paren

Fix: Arel now emits a single pair of parens for UNION and UNION ALL

2 files changed, 22 insertions, 10 deletions

diff --git a/activerecord/test/cases/arel/visitors/mysql_test.rb b/activerecord/test/cases/arel/visitors/mysql_test.rb
index 9d3bad8516..dae98b0583 100644
--- a/activerecord/test/cases/arel/visitors/mysql_test.rb
+++ b/activerecord/test/cases/arel/visitors/mysql_test.rb
@@ -13,16 +13,6 @@ module Arel
         @visitor.accept(node, Collectors::SQLString.new).value
       end
 
-      it "squashes parenthesis on multiple unions" do
-        subnode = Nodes::Union.new Arel.sql("left"), Arel.sql("right")
-        node    = Nodes::Union.new subnode, Arel.sql("topright")
-        assert_equal 1, compile(node).scan("(").length
-
-        subnode = Nodes::Union.new Arel.sql("left"), Arel.sql("right")
-        node    = Nodes::Union.new Arel.sql("topleft"), subnode
-        assert_equal 1, compile(node).scan("(").length
-      end
-
       ###
       # :'(
       # http://dev.mysql.com/doc/refman/5.0/en/select.html#id3482214
diff --git a/activerecord/test/cases/arel/visitors/to_sql_test.rb b/activerecord/test/cases/arel/visitors/to_sql_test.rb
index b6426a211e..680c26a9ab 100644
--- a/activerecord/test/cases/arel/visitors/to_sql_test.rb
+++ b/activerecord/test/cases/arel/visitors/to_sql_test.rb
@@ -480,6 +480,28 @@ module Arel
         end
       end
 
+      describe "Nodes::Union" do
+        it "squashes parenthesis on multiple unions" do
+          subnode = Nodes::Union.new Arel.sql("left"), Arel.sql("right")
+          node = Nodes::Union.new subnode, Arel.sql("topright")
+          assert_equal("( left UNION right UNION topright )", compile(node))
+          subnode = Nodes::Union.new Arel.sql("left"), Arel.sql("right")
+          node = Nodes::Union.new Arel.sql("topleft"), subnode
+          assert_equal("( topleft UNION left UNION right )", compile(node))
+        end
+      end
+
+      describe "Nodes::UnionAll" do
+        it "squashes parenthesis on multiple union alls" do
+          subnode = Nodes::UnionAll.new Arel.sql("left"), Arel.sql("right")
+          node = Nodes::UnionAll.new subnode, Arel.sql("topright")
+          assert_equal("( left UNION ALL right UNION ALL topright )", compile(node))
+          subnode = Nodes::UnionAll.new Arel.sql("left"), Arel.sql("right")
+          node = Nodes::UnionAll.new Arel.sql("topleft"), subnode
+          assert_equal("( topleft UNION ALL left UNION ALL right )", compile(node))
+        end
+      end
+
       describe "Nodes::NotIn" do
         it "should know how to visit" do
           node = @attr.not_in [1, 2, 3]