Arel: only wrap SELECT statements in UNION if they involve ORDER BY, LIMIT or OFFSET

This change was not compatible with the SQLite3 adapter, so revert it, except for
the specific cases where this actually fixed an issue.

Author: ClearlyClaire

activerecord/lib/arel/visitors/to_sql.rb

@@ -965,21 +965,21 @@ def infix_value_with_paren(o, collector, value, suppress_parens = false)
           collector = if o.left.class == o.class
             infix_value_with_paren(o.left, collector, value, true)
           else
-            grouping_parentheses o.left, collector
+            grouping_parentheses o.left, collector, false
           end
           collector << value
           collector = if o.right.class == o.class
             infix_value_with_paren(o.right, collector, value, true)
           else
-            grouping_parentheses o.right, collector
+            grouping_parentheses o.right, collector, false
           end
           collector << " )" unless suppress_parens
           collector
         end
 
         # Used by some visitors to enclose select queries in parentheses
-        def grouping_parentheses(o, collector)
-          if o.is_a?(Nodes::SelectStatement)
+        def grouping_parentheses(o, collector, always_wrap_selects = true)
+          if o.is_a?(Nodes::SelectStatement) && (always_wrap_selects || o.orders.present? || o.limit.present? || o.offset.present?)
             collector << "("
             visit o, collector
             collector << ")"

activerecord/test/cases/arel/attributes/attribute_test.rb

@@ -963,7 +963,7 @@ class AttributeTest < Arel::Spec
           union = mgr1.union(mgr2)
           node = relation[:id].in(union)
           _(node.to_sql).must_be_like %{
-            "users"."id" IN (( (SELECT "users"."id" FROM "users") UNION (SELECT "users"."id" FROM "users") ))
+            "users"."id" IN (( SELECT "users"."id" FROM "users" UNION SELECT "users"."id" FROM "users" ))
           }
         end
 

activerecord/test/cases/arel/select_manager_test.rb

@@ -253,15 +253,15 @@ def test_join_sources
 
         # maybe FIXME: decide when wrapper parens are needed
         _(node.to_sql).must_be_like %{
-          ( (SELECT * FROM "users"  WHERE "users"."age" < 18) UNION (SELECT * FROM "users"  WHERE "users"."age" > 99) )
+          ( SELECT * FROM "users"  WHERE "users"."age" < 18 UNION SELECT * FROM "users"  WHERE "users"."age" > 99 )
         }
       end
 
       it "should union all" do
         node = @m1.union :all, @m2
 
         _(node.to_sql).must_be_like %{
-          ( (SELECT * FROM "users"  WHERE "users"."age" < 18) UNION ALL (SELECT * FROM "users"  WHERE "users"."age" > 99) )
+          ( SELECT * FROM "users"  WHERE "users"."age" < 18 UNION ALL SELECT * FROM "users"  WHERE "users"."age" > 99 )
         }
       end
     end
@@ -354,9 +354,9 @@ def test_join_sources
         sql = manager.to_sql
         _(sql).must_be_like %{
           WITH RECURSIVE "replies" AS (
-              (SELECT "comments"."id", "comments"."parent_id" FROM "comments" WHERE "comments"."id" = 42)
+              SELECT "comments"."id", "comments"."parent_id" FROM "comments" WHERE "comments"."id" = 42
             UNION
-              (SELECT "comments"."id", "comments"."parent_id" FROM "comments" INNER JOIN "replies" ON "comments"."parent_id" = "replies"."id")
+              SELECT "comments"."id", "comments"."parent_id" FROM "comments" INNER JOIN "replies" ON "comments"."parent_id" = "replies"."id"
           )
           SELECT * FROM "replies"
         }