Commit 936e9e89 authored by Yorick Peterse's avatar Yorick Peterse

Clean up schema of the "merge_requests" table

This adds various foreign keys and indexes to the "merge_requests" table
as outlined in https://gitlab.com/gitlab-org/gitlab-ce/issues/31825.

Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/31825
parent 202ab628
---
title: Clean up schema of the "merge_requests" table
merge_request:
author:
type: other
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class MergeRequestsAuthorIdForeignKey < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
disable_ddl_transaction!
class MergeRequest < ActiveRecord::Base
include EachBatch
self.table_name = 'merge_requests'
def self.with_orphaned_authors
where('NOT EXISTS (SELECT true FROM users WHERE merge_requests.author_id = users.id)')
.where('author_id IS NOT NULL')
end
end
def up
# Replacing the ghost user ID logic would be too complex, hence we don't
# redefine the User model here.
ghost_id = User.select(:id).ghost.id
MergeRequest.with_orphaned_authors.each_batch(of: 100) do |batch|
batch.update_all(author_id: ghost_id)
end
add_concurrent_foreign_key(
:merge_requests,
:users,
column: :author_id,
on_delete: :nullify
)
end
def down
remove_foreign_key(:merge_requests, column: :author_id)
end
end
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class MergeRequestsAssigneeIdForeignKey < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
disable_ddl_transaction!
class MergeRequest < ActiveRecord::Base
include EachBatch
self.table_name = 'merge_requests'
def self.with_orphaned_assignees
where('NOT EXISTS (SELECT true FROM users WHERE merge_requests.assignee_id = users.id)')
.where('assignee_id IS NOT NULL')
end
end
def up
MergeRequest.with_orphaned_assignees.each_batch(of: 100) do |batch|
batch.update_all(assignee_id: nil)
end
add_concurrent_foreign_key(
:merge_requests,
:users,
column: :assignee_id,
on_delete: :nullify
)
end
def down
remove_foreign_key(:merge_requests, column: :assignee_id)
end
end
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class MergeRequestsUpdatedByIdForeignKey < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
disable_ddl_transaction!
class MergeRequest < ActiveRecord::Base
include EachBatch
self.table_name = 'merge_requests'
def self.with_orphaned_updaters
where('NOT EXISTS (SELECT true FROM users WHERE merge_requests.updated_by_id = users.id)')
.where('updated_by_id IS NOT NULL')
end
end
def up
MergeRequest.with_orphaned_updaters.each_batch(of: 100) do |batch|
batch.update_all(updated_by_id: nil)
end
add_concurrent_index(
:merge_requests,
:updated_by_id,
where: 'updated_by_id IS NOT NULL'
)
add_concurrent_foreign_key(
:merge_requests,
:users,
column: :updated_by_id,
on_delete: :nullify
)
end
def down
remove_foreign_key_without_error(:merge_requests, column: :updated_by_id)
remove_concurrent_index(:merge_requests, :updated_by_id)
end
end
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class MergeRequestsMergeUserIdForeignKey < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
disable_ddl_transaction!
class MergeRequest < ActiveRecord::Base
include EachBatch
self.table_name = 'merge_requests'
def self.with_orphaned_mergers
where('NOT EXISTS (SELECT true FROM users WHERE merge_requests.merge_user_id = users.id)')
.where('merge_user_id IS NOT NULL')
end
end
def up
MergeRequest.with_orphaned_mergers.each_batch(of: 100) do |batch|
batch.update_all(merge_user_id: nil)
end
add_concurrent_index(
:merge_requests,
:merge_user_id,
where: 'merge_user_id IS NOT NULL'
)
add_concurrent_foreign_key(
:merge_requests,
:users,
column: :merge_user_id,
on_delete: :nullify
)
end
def down
remove_foreign_key_without_error(:merge_requests, column: :merge_user_id)
remove_concurrent_index(:merge_requests, :merge_user_id)
end
end
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class MergeRequestsSourceProjectIdForeignKey < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
disable_ddl_transaction!
class MergeRequest < ActiveRecord::Base
include EachBatch
self.table_name = 'merge_requests'
def self.with_orphaned_source_projects
where('NOT EXISTS (SELECT true FROM projects WHERE merge_requests.source_project_id = projects.id)')
.where('source_project_id IS NOT NULL')
end
end
def up
MergeRequest.with_orphaned_source_projects.each_batch(of: 100) do |batch|
batch.update_all(source_project_id: nil)
end
# We need to allow NULL values so we can nullify the column when the source
# project is removed. We _don't_ want to remove the merge request, instead
# the application will keep them but close them.
change_column_null(:merge_requests, :source_project_id, true)
add_concurrent_foreign_key(
:merge_requests,
:projects,
column: :source_project_id,
on_delete: :nullify
)
end
def down
remove_foreign_key_without_error(:merge_requests, column: :source_project_id)
change_column_null(:merge_requests, :source_project_id, false)
end
end
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class MergeRequestsMilestoneIdForeignKey < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
disable_ddl_transaction!
class MergeRequest < ActiveRecord::Base
include EachBatch
self.table_name = 'merge_requests'
def self.with_orphaned_milestones
where('NOT EXISTS (SELECT true FROM milestones WHERE merge_requests.milestone_id = milestones.id)')
.where('milestone_id IS NOT NULL')
end
end
def up
MergeRequest.with_orphaned_milestones.each_batch(of: 100) do |batch|
batch.update_all(milestone_id: nil)
end
add_concurrent_foreign_key(
:merge_requests,
:milestones,
column: :milestone_id,
on_delete: :nullify
)
end
def down
remove_foreign_key_without_error(:merge_requests, column: :milestone_id)
end
end
......@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 20171114104051) do
ActiveRecord::Schema.define(version: 20171114162227) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
......@@ -1040,7 +1040,7 @@ ActiveRecord::Schema.define(version: 20171114104051) do
create_table "merge_requests", force: :cascade do |t|
t.string "target_branch", null: false
t.string "source_branch", null: false
t.integer "source_project_id", null: false
t.integer "source_project_id"
t.integer "author_id"
t.integer "assignee_id"
t.string "title"
......@@ -1080,6 +1080,7 @@ ActiveRecord::Schema.define(version: 20171114104051) do
add_index "merge_requests", ["description"], name: "index_merge_requests_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"}
add_index "merge_requests", ["head_pipeline_id"], name: "index_merge_requests_on_head_pipeline_id", using: :btree
add_index "merge_requests", ["latest_merge_request_diff_id"], name: "index_merge_requests_on_latest_merge_request_diff_id", using: :btree
add_index "merge_requests", ["merge_user_id"], name: "index_merge_requests_on_merge_user_id", where: "(merge_user_id IS NOT NULL)", using: :btree
add_index "merge_requests", ["milestone_id"], name: "index_merge_requests_on_milestone_id", using: :btree
add_index "merge_requests", ["source_branch"], name: "index_merge_requests_on_source_branch", using: :btree
add_index "merge_requests", ["source_project_id", "source_branch"], name: "index_merge_requests_on_source_project_id_and_source_branch", using: :btree
......@@ -1088,6 +1089,7 @@ ActiveRecord::Schema.define(version: 20171114104051) do
add_index "merge_requests", ["target_project_id", "merge_commit_sha", "id"], name: "index_merge_requests_on_tp_id_and_merge_commit_sha_and_id", using: :btree
add_index "merge_requests", ["title"], name: "index_merge_requests_on_title", using: :btree
add_index "merge_requests", ["title"], name: "index_merge_requests_on_title_trigram", using: :gin, opclasses: {"title"=>"gin_trgm_ops"}
add_index "merge_requests", ["updated_by_id"], name: "index_merge_requests_on_updated_by_id", where: "(updated_by_id IS NOT NULL)", using: :btree
create_table "merge_requests_closing_issues", force: :cascade do |t|
t.integer "merge_request_id", null: false
......@@ -1965,7 +1967,13 @@ ActiveRecord::Schema.define(version: 20171114104051) do
add_foreign_key "merge_request_metrics", "merge_requests", on_delete: :cascade
add_foreign_key "merge_requests", "ci_pipelines", column: "head_pipeline_id", name: "fk_fd82eae0b9", on_delete: :nullify
add_foreign_key "merge_requests", "merge_request_diffs", column: "latest_merge_request_diff_id", name: "fk_06067f5644", on_delete: :nullify
add_foreign_key "merge_requests", "milestones", name: "fk_6a5165a692", on_delete: :nullify
add_foreign_key "merge_requests", "projects", column: "source_project_id", name: "fk_3308fe130c", on_delete: :nullify
add_foreign_key "merge_requests", "projects", column: "target_project_id", name: "fk_a6963e8447", on_delete: :cascade
add_foreign_key "merge_requests", "users", column: "assignee_id", name: "fk_6149611a04", on_delete: :nullify
add_foreign_key "merge_requests", "users", column: "author_id", name: "fk_e719a85f8a", on_delete: :nullify
add_foreign_key "merge_requests", "users", column: "merge_user_id", name: "fk_ad525e1f87", on_delete: :nullify
add_foreign_key "merge_requests", "users", column: "updated_by_id", name: "fk_641731faff", on_delete: :nullify
add_foreign_key "merge_requests_closing_issues", "issues", on_delete: :cascade
add_foreign_key "merge_requests_closing_issues", "merge_requests", on_delete: :cascade
add_foreign_key "milestones", "namespaces", column: "group_id", name: "fk_95650a40d4", on_delete: :cascade
......
module Gitlab
module ImportExport
class MergeRequestParser
FORKED_PROJECT_ID = -1
FORKED_PROJECT_ID = nil
def initialize(project, diff_head_sha, merge_request, relation_hash)
@project = project
......
......@@ -155,7 +155,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
end
it 'has no source if source/target differ' do
expect(MergeRequest.find_by_title('MR2').source_project_id).to eq(-1)
expect(MergeRequest.find_by_title('MR2').source_project_id).to be_nil
end
end
......
......@@ -5,7 +5,7 @@ describe Milestones::DestroyService do
let(:project) { create(:project) }
let(:milestone) { create(:milestone, title: 'Milestone v1.0', project: project) }
let!(:issue) { create(:issue, project: project, milestone: milestone) }
let(:merge_request) { create(:merge_request, source_project: project, milestone: milestone) }
let!(:merge_request) { create(:merge_request, source_project: project, milestone: milestone) }
before do
project.team << [user, :master]
......
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