Remove comma from weight system notes more efficiently

1. There is no index on system_note_metadata's action column, so add a partial
   one on 'weight'.
2. update_column_in_batches expects a single column to be updated, but we need
   to null out note_html too. Using an SQL literal lets us get around this.
3. Arel's UpdateManager doesn't support joins, so use a subquery instead.
4. MySQL can't update from the same table referenced in the subquery. We use an
   alias to handle this case.

ee/db/migrate/20180530201303_remove_commas_from_weight_note.rb

-# See http://doc.gitlab.com/ce/development/migration_style_guide.html
-# for more information on how to write migrations for GitLab.
-
 class RemoveCommasFromWeightNote < ActiveRecord::Migration
   include Gitlab::Database::MigrationHelpers
+
   DOWNTIME = false
 
+  disable_ddl_transaction!
+
   def up
-    ActiveRecord::Base.connection.execute(<<-EOQ)
-      UPDATE notes
-      SET notes.note = SUBSTRING(notes.note, 1, LEN(notes.note) - 1)
-      FROM system_note_metadata 
-      WHERE system_note_metadata.note_id = notes.id AND notes.system AND system_note_metadata.action = 'weight' AND notes.note LIKE '%,'
-    EOQ
-  end
+    add_concurrent_index(:system_note_metadata, :action, where: "action = 'weight'")
 
-  def down
-    old_notes = Note.where("note LIKE 'changed weight to%'")
+    notes_table = Arel::Table.new(:notes)
+    metadata_table = Arel::Table.new(:system_note_metadata)
+    update_value = Arel.sql("TRIM(TRAILING ',' FROM note), note_html = NULL")
+
+    weight_system_notes =
+      notes_table
+        .join(metadata_table).on(notes_table[:id].eq(metadata_table[:note_id]))
+        .where(metadata_table[:action].eq('weight'))
+        .where(notes_table[:system].eq(true))
+        .where(notes_table[:note].matches('%,'))
+        .project(notes_table[:id])
 
-    old_notes.find_each do |note|
-      note.update_column(:note, note.note.concat(','))
+    if Gitlab::Database.mysql?
+      weight_system_notes = weight_system_notes.from(
+        notes_table.project([notes_table[:id], notes_table[:system]]).as('notes')
+      )
     end
+
+    update_column_in_batches(:notes, :note, update_value) do |table, query|
+      query.where(table[:id].in(weight_system_notes))
+    end
+
+    remove_concurrent_index(:system_note_metadata, :action)
+  end
+
+  def down
+    # We can't reliably find notes that would need a comma, so do nothing
   end
 end

ee/spec/migrations/remove_commas_from_weight_note_spec.rb

@@ -2,27 +2,30 @@ require 'spec_helper'
 require Rails.root.join('ee', 'db', 'migrate', '../../db/migrate/20180530201303_remove_commas_from_weight_note.rb')
 
 describe RemoveCommasFromWeightNote, :migration do
-  let(:migration) { described_class.new }
+  RSpec::Matchers.define_negated_matcher :not_change, :change
 
   describe '#up' do
-    let(:notes) {table(:notes)}
+    let(:notes) { table(:notes) }
+    let(:system_note_metadata) { table(:system_note_metadata) }
 
-    let!(:note_1) {notes.create(note: 'changed weight to 5,')}
-    let!(:note_2) {notes.create(note: 'removed the weight')}
-
-    it 'removes all trailing commas' do
-      expect { migrate! }.to change { Note.where("note LIKE '%,'").count }.from(1).to(0)
+    def create_system_note(note, metadata_action)
+      notes.create(note: note, system: true).tap do |system_note|
+        system_note_metadata.create(note_id: system_note.id, action: metadata_action)
+      end
     end
-  end
-
-  describe '#down' do
-    let(:notes) {table(:notes)}
 
-    let!(:note_1) {notes.create(note: 'changed weight to 5')}
-    let!(:note_2) {notes.create(note: 'removed the weight')}
+    let(:weight_note_with_comma) { create_system_note('changed weight to 5,', 'weight') }
+    let(:weight_note_without_comma) { create_system_note('removed the weight', 'weight') }
+    let(:title_note) { create_system_note('changed title from 5, to 5,', 'title') }
+    let(:user_note) { notes.create(note: 'changed weight to 5,') }
 
-    it 'adds trailing commas' do
-      expect { migration.down }.to change { Note.where("note LIKE '%,'").count }.from(0).to(1)
+    it 'removes all trailing commas from weight system notes' do
+      expect { migrate! }
+        .to change { Note.where("note LIKE '%,'").count }.from(3).to(2)
+        .and change { weight_note_with_comma.reload.note }
+        .and not_change { weight_note_without_comma.reload.note }
+        .and not_change { title_note.reload.note }
+        .and not_change { user_note.reload.note }
     end
   end
 end