Use nested queries when doing UPDATE in myslq and GROUP_BY and HAVING clauses are present.

MySQL does not support GROUP_BY and HAVING on UPDATE, we need to use a Subquery.

activerecord/CHANGELOG.md

@@ -1,3 +1,24 @@
+*   Use subquery for UPDATE with GROUP_BY and HAVING clauses.
+
+    Prior to this change, updates with GROUP_BY and HAVING were being ignored, generating a SQL like this:
+
+    ```sql
+    UPDATE "posts" SET "flagged" = ? WHERE "posts"."id" IN (
+        SELECT "posts"."id" FROM "posts" INNER JOIN "comments" ON "comments"."post_id" = "posts"."id"
+    )  [["flagged", "t"]]
+    ```
+
+    After this change, GROUP_BY and HAVING clauses are used as a subquery in updates, like this:
+
+    ```sql
+    UPDATE "posts" SET "flagged" = ? WHERE "posts"."id" IN (
+        SELECT "posts"."id" FROM "posts" INNER JOIN "comments" ON "comments"."post_id" = "posts"."id" 
+        GROUP BY posts.id HAVING (count(comments.id) >= 2)
+    )  [["flagged", "t"]]
+    ```
+
+    *Ignacio Chiazzo Cardarello*
+
 *   Add support for setting the filename of the schema or structure dump in the database config.
 
     Applications may now set their the filename or path of the schema / structure dump file in their database configuration.

activerecord/lib/active_record/relation.rb

@@ -485,8 +485,9 @@ module ActiveRecord
       arel = eager_loading? ? apply_join_dependency.arel : build_arel
       arel.source.left = table
 
-      stmt = arel.compile_update(values, table[primary_key])
-
+      group_values_arel_columns = arel_columns(group_values.uniq)
+      having_clause_ast = having_clause.ast unless having_clause.empty?
+      stmt = arel.compile_update(values, table[primary_key], having_clause_ast, group_values_arel_columns)
       klass.connection.update(stmt, "#{klass} Update All").tap { reset }
     end
 

activerecord/lib/arel/crud.rb

@@ -14,7 +14,12 @@ module Arel # :nodoc: all
       InsertManager.new
     end
 
-    def compile_update(values, key = nil)
+    def compile_update(
+      values,
+      key = nil,
+      having_clause = nil,
+      group_values_columns = []
+    )
       um = UpdateManager.new(source)
       um.set(values)
       um.take(limit)
@@ -22,6 +27,9 @@ module Arel # :nodoc: all
       um.order(*orders)
       um.wheres = constraints
       um.key = key
+
+      um.group(group_values_columns) unless group_values_columns.empty?
+      um.having(having_clause) unless having_clause.nil?
       um
     end
 

activerecord/lib/arel/nodes/delete_statement.rb

@@ -3,12 +3,14 @@
 module Arel # :nodoc: all
   module Nodes
     class DeleteStatement < Arel::Nodes::Node
-      attr_accessor :relation, :wheres, :orders, :limit, :offset, :key
+      attr_accessor :relation, :wheres, :groups, :havings, :orders, :limit, :offset, :key
 
       def initialize(relation = nil, wheres = [])
         super()
         @relation = relation
         @wheres = wheres
+        @groups = []
+        @havings = []
         @orders = []
         @limit = nil
         @offset = nil
@@ -30,6 +32,8 @@ module Arel # :nodoc: all
           self.relation == other.relation &&
           self.wheres == other.wheres &&
           self.orders == other.orders &&
+          self.groups == other.groups &&
+          self.havings == other.havings &&
           self.limit == other.limit &&
           self.offset == other.offset &&
           self.key == other.key

activerecord/lib/arel/nodes/update_statement.rb

@@ -3,13 +3,15 @@
 module Arel # :nodoc: all
   module Nodes
     class UpdateStatement < Arel::Nodes::Node
-      attr_accessor :relation, :wheres, :values, :orders, :limit, :offset, :key
+      attr_accessor :relation, :wheres, :values, :groups, :havings, :orders, :limit, :offset, :key
 
       def initialize(relation = nil)
         super()
         @relation = relation
         @wheres   = []
         @values   = []
+        @groups   = []
+        @havings  = []
         @orders   = []
         @limit    = nil
         @offset   = nil
@@ -31,6 +33,8 @@ module Arel # :nodoc: all
           self.relation == other.relation &&
           self.wheres == other.wheres &&
           self.values == other.values &&
+          self.groups == other.groups &&
+          self.havings == other.havings &&
           self.orders == other.orders &&
           self.limit == other.limit &&
           self.offset == other.offset &&

activerecord/lib/arel/update_manager.rb

@@ -28,5 +28,21 @@ module Arel # :nodoc: all
       end
       self
     end
+
+    def group(columns)
+      columns.each do |column|
+        column = Nodes::SqlLiteral.new(column) if String === column
+        column = Nodes::SqlLiteral.new(column.to_s) if Symbol === column
+
+        @ast.groups.push Nodes::Group.new column
+      end
+
+      self
+    end
+
+    def having(expr)
+      @ast.havings << expr
+      self
+    end
   end
 end

activerecord/lib/arel/visitors/mysql.rb

@@ -67,7 +67,8 @@ module Arel # :nodoc: all
         # query. However, this does not allow for LIMIT, OFFSET and ORDER. To support
         # these, we must use a subquery.
         def prepare_update_statement(o)
-          if o.offset || has_join_sources?(o) && has_limit_or_offset_or_orders?(o)
+          if o.offset || has_group_by_and_having?(o) ||
+            has_join_sources?(o) && has_limit_or_offset_or_orders?(o)
             super
           else
             o

activerecord/lib/arel/visitors/to_sql.rb

@@ -841,6 +841,10 @@ module Arel # :nodoc: all
           o.limit || o.offset || !o.orders.empty?
         end
 
+        def has_group_by_and_having?(o)
+          !o.groups.empty? && !o.havings.empty?
+        end
+
         # The default strategy for an UPDATE with joins is to use a subquery. This doesn't work
         # on MySQL (even when aliasing the tables), but MySQL allows using JOIN directly in
         # an UPDATE statement, so in the MySQL visitor we redefine this to do that.
@@ -852,6 +856,8 @@ module Arel # :nodoc: all
             stmt.orders = []
             stmt.wheres = [Nodes::In.new(o.key, [build_subselect(o.key, o)])]
             stmt.relation = o.relation.left if has_join_sources?(o)
+            stmt.groups = o.groups unless o.groups.empty?
+            stmt.havings = o.havings unless o.havings.empty?
             stmt
           else
             o
@@ -866,6 +872,8 @@ module Arel # :nodoc: all
           core.froms       = o.relation
           core.wheres      = o.wheres
           core.projections = [key]
+          core.groups      = o.groups unless o.groups.empty?
+          core.havings     = o.havings unless o.havings.empty?
           stmt.limit       = o.limit
           stmt.offset      = o.offset
           stmt.orders      = o.orders

activerecord/test/cases/arel/nodes/update_statement_test.rb

@@ -27,6 +27,8 @@ describe Arel::Nodes::UpdateStatement do
       statement1.orders   = %w[x y z]
       statement1.limit    = 42
       statement1.key      = "zomg"
+      statement1.groups   = ["foo"]
+      statement1.havings  = []
       statement2 = Arel::Nodes::UpdateStatement.new
       statement2.relation = "zomg"
       statement2.wheres   = 2
@@ -34,6 +36,8 @@ describe Arel::Nodes::UpdateStatement do
       statement2.orders   = %w[x y z]
       statement2.limit    = 42
       statement2.key      = "zomg"
+      statement2.groups   = ["foo"]
+      statement2.havings  = []
       array = [statement1, statement2]
       assert_equal 1, array.uniq.size
     end

activerecord/test/cases/arel/update_manager_test.rb

@@ -22,6 +22,57 @@ module Arel
       assert_match(/LIMIT 10/, um.to_sql)
     end
 
+    describe "having" do
+      it "sets having" do
+        users_table = Table.new(:users)
+        posts_table = Table.new(:posts)
+        join_source = Arel::Nodes::InnerJoin.new(users_table, posts_table)
+
+        update_manager = Arel::UpdateManager.new
+        update_manager.table(join_source)
+        update_manager.group(["posts.id"])
+        update_manager.having("count(posts.id) >= 2")
+
+        assert_equal(["count(posts.id) >= 2"], update_manager.ast.havings)
+      end
+    end
+
+    describe "group" do
+      it "adds columns to the AST when group value is a String" do
+        users_table = Table.new(:users)
+        posts_table = Table.new(:posts)
+        join_source = Arel::Nodes::InnerJoin.new(users_table, posts_table)
+
+        update_manager = Arel::UpdateManager.new
+        update_manager.table(join_source)
+        update_manager.group(["posts.id"])
+        update_manager.having("count(posts.id) >= 2")
+
+        assert_equal(1, update_manager.ast.groups.count)
+        group_ast = update_manager.ast.groups.first
+        _(group_ast).must_be_kind_of Nodes::Group
+        assert_equal("posts.id", group_ast.expr)
+        assert_equal(["count(posts.id) >= 2"], update_manager.ast.havings)
+      end
+
+      it "adds columns to the AST when group value is a Symbol" do
+        users_table = Table.new(:users)
+        posts_table = Table.new(:posts)
+        join_source = Arel::Nodes::InnerJoin.new(users_table, posts_table)
+
+        update_manager = Arel::UpdateManager.new
+        update_manager.table(join_source)
+        update_manager.group([:"posts.id"])
+        update_manager.having("count(posts.id) >= 2")
+
+        assert_equal(1, update_manager.ast.groups.count)
+        group_ast = update_manager.ast.groups.first
+        _(group_ast).must_be_kind_of Nodes::Group
+        assert_equal("posts.id", group_ast.expr)
+        assert_equal(["count(posts.id) >= 2"], update_manager.ast.havings)
+      end
+    end
+
     describe "set" do
       it "updates with null" do
         table = Table.new(:users)

activerecord/test/cases/relation/update_all_test.rb

@@ -44,6 +44,20 @@ class UpdateAllTest < ActiveRecord::TestCase
     assert_equal "Empty list of attributes to change", error.message
   end
 
+  def test_update_all_with_group_by
+    minimum_comments_count = 2
+
+    Post.most_commented(minimum_comments_count).update_all(title: "ig")
+    posts = Post.most_commented(minimum_comments_count).all.to_a
+
+    assert_operator posts.length, :>, 0
+    assert posts.all? { |post| post.comments.length >= minimum_comments_count }
+    assert posts.all? { |post| "ig" == post.title }
+
+    post = Post.joins(:comments).group("posts.id").having("count(comments.id) < #{minimum_comments_count}").first
+    assert_not_equal "ig", post.title
+  end
+
   def test_update_all_with_joins
     pets = Pet.joins(:toys).where(toys: { name: "Bone" })
 

activerecord/test/models/post.rb

@@ -34,6 +34,11 @@ class Post < ActiveRecord::Base
 
   scope :limit_by, lambda { |l| limit(l) }
   scope :locked, -> { lock }
+  scope :most_commented, lambda { |comments_count|
+    joins(:comments)
+    .group("posts.id")
+    .having("count(comments.id) >= #{comments_count}")
+  }
 
   belongs_to :author
   belongs_to :readonly_author, -> { readonly }, class_name: "Author", foreign_key: :author_id