Commit a8a45180 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Clean up `noteable_id` for notes on commits

This was incorrectly set by a bug in:
https://gitlab.com/gitlab-org/gitlab-ce/issues/54924

Also adds a `batch_size` option to `update_column_in_batches`
parent 8e33e7cf
# frozen_string_literal: true
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class CleanUpNoteableIdForNotesOnCommits < ActiveRecord::Migration[5.0]
include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
TEMP_INDEX_NAME = 'index_notes_on_commit_with_null_noteable_id'
disable_ddl_transaction!
def up
remove_concurrent_index_by_name(:notes, TEMP_INDEX_NAME)
add_concurrent_index(:notes, :id, where: "noteable_type = 'Commit' AND noteable_id IS NOT NULL", name: TEMP_INDEX_NAME)
# rubocop:disable Migration/UpdateLargeTable
update_column_in_batches(:notes, :noteable_id, nil, batch_size: 300) do |table, query|
query.where(
table[:noteable_type].eq('Commit').and(table[:noteable_id].not_eq(nil))
)
end
remove_concurrent_index_by_name(:notes, TEMP_INDEX_NAME)
end
def down
end
end
......@@ -282,6 +282,7 @@ module Gitlab
# Updates the value of a column in batches.
#
# This method updates the table in batches of 5% of the total row count.
# A `batch_size` option can also be passed to set this to a fixed number.
# This method will continue updating rows until no rows remain.
#
# When given a block this method will yield two values to the block:
......@@ -320,7 +321,7 @@ module Gitlab
# make things _more_ complex).
#
# rubocop: disable Metrics/AbcSize
def update_column_in_batches(table, column, value)
def update_column_in_batches(table, column, value, batch_size: nil)
if transaction_open?
raise 'update_column_in_batches can not be run inside a transaction, ' \
'you can disable transactions by calling disable_ddl_transaction! ' \
......@@ -336,6 +337,7 @@ module Gitlab
return if total == 0
if batch_size.nil?
# Update in batches of 5% until we run out of any rows to update.
batch_size = ((total / 100.0) * 5.0).ceil
max_size = 1000
......@@ -344,6 +346,7 @@ module Gitlab
# example, for "merge_requests" even 1% of the table is around 35 000
# rows for GitLab.com.
batch_size = max_size if batch_size > max_size
end
start_arel = table.project(table[:id]).order(table[:id].asc).take(1)
start_arel = yield table, start_arel if block_given?
......
# frozen_string_literal: true
require 'spec_helper'
require Rails.root.join('db', 'post_migrate', '20190313092516_clean_up_noteable_id_for_notes_on_commits.rb')
describe CleanUpNoteableIdForNotesOnCommits, :migration do
let(:notes) { table(:notes) }
before do
notes.create!(noteable_type: 'Commit', commit_id: '3d0a182204cece4857f81c6462720e0ad1af39c9', noteable_id: 3, note: 'Test')
notes.create!(noteable_type: 'Commit', commit_id: '3d0a182204cece4857f81c6462720e0ad1af39c9', noteable_id: 3, note: 'Test')
notes.create!(noteable_type: 'Commit', commit_id: '3d0a182204cece4857f81c6462720e0ad1af39c9', noteable_id: 3, note: 'Test')
notes.create!(noteable_type: 'Issue', noteable_id: 1, note: 'Test')
notes.create!(noteable_type: 'MergeRequest', noteable_id: 1, note: 'Test')
notes.create!(noteable_type: 'Snippet', noteable_id: 1, note: 'Test')
end
it 'clears noteable_id for notes on commits' do
expect { migrate! }.to change { dirty_notes_on_commits.count }.from(3).to(0)
end
it 'does not clear noteable_id for other notes' do
expect { migrate! }.not_to change { other_notes.count }
end
def dirty_notes_on_commits
notes.where(noteable_type: 'Commit').where('noteable_id IS NOT NULL')
end
def other_notes
notes.where("noteable_type != 'Commit' AND noteable_id IS NOT NULL")
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment