From 96bc0549b47a2cc28f6847705195061659982b84 Mon Sep 17 00:00:00 2001 From: Claire Date: Fri, 19 Apr 2024 12:44:04 +0200 Subject: [PATCH] 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. --- activerecord/lib/arel/visitors/to_sql.rb | 8 ++++---- activerecord/test/cases/arel/attributes/attribute_test.rb | 2 +- activerecord/test/cases/arel/select_manager_test.rb | 8 ++++---- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/activerecord/lib/arel/visitors/to_sql.rb b/activerecord/lib/arel/visitors/to_sql.rb index a521acf1353..71edff4bfc6 100644 --- a/activerecord/lib/arel/visitors/to_sql.rb +++ b/activerecord/lib/arel/visitors/to_sql.rb @@ -965,21 +965,21 @@ module Arel # :nodoc: all 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 << ")" diff --git a/activerecord/test/cases/arel/attributes/attribute_test.rb b/activerecord/test/cases/arel/attributes/attribute_test.rb index d18d5859e60..0f3d78c459e 100644 --- a/activerecord/test/cases/arel/attributes/attribute_test.rb +++ b/activerecord/test/cases/arel/attributes/attribute_test.rb @@ -963,7 +963,7 @@ module Arel 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 diff --git a/activerecord/test/cases/arel/select_manager_test.rb b/activerecord/test/cases/arel/select_manager_test.rb index 0297564da13..a07d0b9d699 100644 --- a/activerecord/test/cases/arel/select_manager_test.rb +++ b/activerecord/test/cases/arel/select_manager_test.rb @@ -253,7 +253,7 @@ module Arel # 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 @@ -261,7 +261,7 @@ module Arel 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 @@ module Arel 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" }