From 67729cecc12a56591160d04ea5d79614b1102dc6 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 13 Mar 2017 20:36:17 +0800 Subject: [PATCH 1/6] Add a test which would rollback db and migrate again Closes #29106 --- .gitlab-ci.yml | 7 +++++++ db/migrate/20160919145149_add_group_id_to_labels.rb | 2 +- ...0083353_add_pipeline_id_to_merge_request_metrics.rb | 2 +- .../20161031171301_add_project_id_to_subscriptions.rb | 1 + ...61207231621_create_environment_name_unique_index.rb | 4 ++-- ...1209153400_add_unique_index_for_environment_slug.rb | 2 +- .../20170124174637_add_foreign_keys_to_timelogs.rb | 10 ++++++++++ ...70216141440_drop_index_for_builds_project_status.rb | 2 +- ...206101007_remove_trackable_columns_from_timelogs.rb | 3 ++- 9 files changed, 26 insertions(+), 7 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index deeb01f9a3c..b1ca61604d5 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -222,6 +222,13 @@ rake db:migrate:reset: script: - bundle exec rake db:migrate:reset +rake db:rollback: + stage: test + <<: *use-db + <<: *dedicated-runner + script: + - bundle exec rake db:rollback db:migrate STEP=120 + rake db:seed_fu: stage: test <<: *use-db diff --git a/db/migrate/20160919145149_add_group_id_to_labels.rb b/db/migrate/20160919145149_add_group_id_to_labels.rb index 828b6afddb1..e20e693f3aa 100644 --- a/db/migrate/20160919145149_add_group_id_to_labels.rb +++ b/db/migrate/20160919145149_add_group_id_to_labels.rb @@ -12,8 +12,8 @@ class AddGroupIdToLabels < ActiveRecord::Migration end def down + remove_foreign_key :labels, column: :group_id remove_index :labels, :group_id if index_exists? :labels, :group_id - remove_foreign_key :labels, :namespaces, column: :group_id remove_column :labels, :group_id end end diff --git a/db/migrate/20161020083353_add_pipeline_id_to_merge_request_metrics.rb b/db/migrate/20161020083353_add_pipeline_id_to_merge_request_metrics.rb index ad3eb4a26f9..35ad22b6c01 100644 --- a/db/migrate/20161020083353_add_pipeline_id_to_merge_request_metrics.rb +++ b/db/migrate/20161020083353_add_pipeline_id_to_merge_request_metrics.rb @@ -32,8 +32,8 @@ class AddPipelineIdToMergeRequestMetrics < ActiveRecord::Migration end def down + remove_foreign_key :merge_request_metrics, column: :pipeline_id remove_index :merge_request_metrics, :pipeline_id if index_exists? :merge_request_metrics, :pipeline_id - remove_foreign_key :merge_request_metrics, :ci_commits, column: :pipeline_id remove_column :merge_request_metrics, :pipeline_id end end diff --git a/db/migrate/20161031171301_add_project_id_to_subscriptions.rb b/db/migrate/20161031171301_add_project_id_to_subscriptions.rb index d5c343dc527..8b1c10a124f 100644 --- a/db/migrate/20161031171301_add_project_id_to_subscriptions.rb +++ b/db/migrate/20161031171301_add_project_id_to_subscriptions.rb @@ -9,6 +9,7 @@ class AddProjectIdToSubscriptions < ActiveRecord::Migration end def down + remove_foreign_key :subscriptions, column: :project_id remove_column :subscriptions, :project_id end end diff --git a/db/migrate/20161207231621_create_environment_name_unique_index.rb b/db/migrate/20161207231621_create_environment_name_unique_index.rb index ac680c8d10f..5ff0f5bae4d 100644 --- a/db/migrate/20161207231621_create_environment_name_unique_index.rb +++ b/db/migrate/20161207231621_create_environment_name_unique_index.rb @@ -12,7 +12,7 @@ class CreateEnvironmentNameUniqueIndex < ActiveRecord::Migration end def down - remove_index :environments, [:project_id, :name], unique: true - add_concurrent_index :environments, [:project_id, :name] + remove_index :environments, [:project_id, :name] + add_concurrent_index :environments, [:project_id, :name], unique: true end end diff --git a/db/migrate/20161209153400_add_unique_index_for_environment_slug.rb b/db/migrate/20161209153400_add_unique_index_for_environment_slug.rb index d7ef1aa83d9..ede0316e860 100644 --- a/db/migrate/20161209153400_add_unique_index_for_environment_slug.rb +++ b/db/migrate/20161209153400_add_unique_index_for_environment_slug.rb @@ -14,6 +14,6 @@ class AddUniqueIndexForEnvironmentSlug < ActiveRecord::Migration end def down - remove_index :environments, [:project_id, :slug], unique: true if index_exists? :environments, [:project_id, :slug] + remove_index :environments, [:project_id, :slug] if index_exists? :environments, [:project_id, :slug] end end diff --git a/db/migrate/20170124174637_add_foreign_keys_to_timelogs.rb b/db/migrate/20170124174637_add_foreign_keys_to_timelogs.rb index 69bfa2d3fc4..676e18cddd3 100644 --- a/db/migrate/20170124174637_add_foreign_keys_to_timelogs.rb +++ b/db/migrate/20170124174637_add_foreign_keys_to_timelogs.rb @@ -49,6 +49,16 @@ class AddForeignKeysToTimelogs < ActiveRecord::Migration Timelog.where('issue_id IS NOT NULL').update_all("trackable_id = issue_id, trackable_type = 'Issue'") Timelog.where('merge_request_id IS NOT NULL').update_all("trackable_id = merge_request_id, trackable_type = 'MergeRequest'") + constraint = + if Gitlab::Database.postgresql? + 'CONSTRAINT' + else + 'FOREIGN KEY' + end + + execute "ALTER TABLE timelogs DROP #{constraint} fk_timelogs_issues_issue_id" + execute "ALTER TABLE timelogs DROP #{constraint} fk_timelogs_merge_requests_merge_request_id" + remove_columns :timelogs, :issue_id, :merge_request_id end end diff --git a/db/migrate/20170216141440_drop_index_for_builds_project_status.rb b/db/migrate/20170216141440_drop_index_for_builds_project_status.rb index 906711b9f3f..a2839f52d89 100644 --- a/db/migrate/20170216141440_drop_index_for_builds_project_status.rb +++ b/db/migrate/20170216141440_drop_index_for_builds_project_status.rb @@ -3,6 +3,6 @@ class DropIndexForBuildsProjectStatus < ActiveRecord::Migration DOWNTIME = false def change - remove_index(:ci_commits, [:gl_project_id, :status]) + remove_index(:ci_commits, column: [:gl_project_id, :status]) end end diff --git a/db/post_migrate/20170206101007_remove_trackable_columns_from_timelogs.rb b/db/post_migrate/20170206101007_remove_trackable_columns_from_timelogs.rb index 89aa753646c..aee0c1b6245 100644 --- a/db/post_migrate/20170206101007_remove_trackable_columns_from_timelogs.rb +++ b/db/post_migrate/20170206101007_remove_trackable_columns_from_timelogs.rb @@ -18,6 +18,7 @@ class RemoveTrackableColumnsFromTimelogs < ActiveRecord::Migration # disable_ddl_transaction! def change - remove_columns :timelogs, :trackable_id, :trackable_type + remove_column :timelogs, :trackable_id, :integer + remove_column :timelogs, :trackable_type, :string end end From 0b20d850cb964da89f8b0df3c9e78a1cc2a86e1b Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 14 Mar 2017 14:36:33 +0800 Subject: [PATCH 2/6] Fix for postgresql --- db/migrate/20161201160452_migrate_project_statistics.rb | 4 ++-- db/migrate/20161212142807_add_lower_path_index_to_routes.rb | 2 +- .../20170130204620_add_index_to_project_authorizations.rb | 4 ++++ db/migrate/20170305203726_add_owner_id_foreign_key.rb | 6 +++++- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/db/migrate/20161201160452_migrate_project_statistics.rb b/db/migrate/20161201160452_migrate_project_statistics.rb index 3ae3f2c159b..8386f7f9d4f 100644 --- a/db/migrate/20161201160452_migrate_project_statistics.rb +++ b/db/migrate/20161201160452_migrate_project_statistics.rb @@ -17,7 +17,7 @@ class MigrateProjectStatistics < ActiveRecord::Migration end def down - add_column_with_default :projects, :repository_size, :float, default: 0.0 - add_column_with_default :projects, :commit_count, :integer, default: 0 + add_column :projects, :repository_size, :float, default: 0.0 + add_column :projects, :commit_count, :integer, default: 0 end end diff --git a/db/migrate/20161212142807_add_lower_path_index_to_routes.rb b/db/migrate/20161212142807_add_lower_path_index_to_routes.rb index 6958500306f..53f4c6bbb18 100644 --- a/db/migrate/20161212142807_add_lower_path_index_to_routes.rb +++ b/db/migrate/20161212142807_add_lower_path_index_to_routes.rb @@ -17,6 +17,6 @@ class AddLowerPathIndexToRoutes < ActiveRecord::Migration def down return unless Gitlab::Database.postgresql? - remove_index :routes, name: :index_on_routes_lower_path + remove_index :routes, name: :index_on_routes_lower_path if index_exists?(:routes, name: :index_on_routes_lower_path) end end diff --git a/db/migrate/20170130204620_add_index_to_project_authorizations.rb b/db/migrate/20170130204620_add_index_to_project_authorizations.rb index e9a0aee4d6a..a8c504f265a 100644 --- a/db/migrate/20170130204620_add_index_to_project_authorizations.rb +++ b/db/migrate/20170130204620_add_index_to_project_authorizations.rb @@ -8,4 +8,8 @@ class AddIndexToProjectAuthorizations < ActiveRecord::Migration def up add_concurrent_index(:project_authorizations, :project_id) end + + def down + remove_index(:project_authorizations, :project_id) + end end diff --git a/db/migrate/20170305203726_add_owner_id_foreign_key.rb b/db/migrate/20170305203726_add_owner_id_foreign_key.rb index 3eece0e2eb5..5fbdc45f1a7 100644 --- a/db/migrate/20170305203726_add_owner_id_foreign_key.rb +++ b/db/migrate/20170305203726_add_owner_id_foreign_key.rb @@ -5,7 +5,11 @@ class AddOwnerIdForeignKey < ActiveRecord::Migration disable_ddl_transaction! - def change + def up add_concurrent_foreign_key :ci_triggers, :users, column: :owner_id, on_delete: :cascade end + + def down + remove_foreign_key :ci_triggers, column: :owner_id + end end From c9fbbb3ae2c7f0eb44b0f973155d68e678149544 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 14 Mar 2017 19:56:37 +0800 Subject: [PATCH 3/6] Disable rubocop for down method --- db/migrate/20161201160452_migrate_project_statistics.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/db/migrate/20161201160452_migrate_project_statistics.rb b/db/migrate/20161201160452_migrate_project_statistics.rb index 8386f7f9d4f..82fbdf02444 100644 --- a/db/migrate/20161201160452_migrate_project_statistics.rb +++ b/db/migrate/20161201160452_migrate_project_statistics.rb @@ -16,6 +16,7 @@ class MigrateProjectStatistics < ActiveRecord::Migration remove_column :projects, :commit_count end + # rubocop: disable Migration/AddColumn def down add_column :projects, :repository_size, :float, default: 0.0 add_column :projects, :commit_count, :integer, default: 0 From f67d8eb1da269150764224cea1807195cdf2ffb5 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 14 Mar 2017 20:03:22 +0800 Subject: [PATCH 4/6] Drop the index only for postgresql, because mysql cannot simply drop the index without dropping the corresponding foreign key, and we certainly don't want to drop the foreign key here. --- .../20170130204620_add_index_to_project_authorizations.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/db/migrate/20170130204620_add_index_to_project_authorizations.rb b/db/migrate/20170130204620_add_index_to_project_authorizations.rb index a8c504f265a..629b49436e3 100644 --- a/db/migrate/20170130204620_add_index_to_project_authorizations.rb +++ b/db/migrate/20170130204620_add_index_to_project_authorizations.rb @@ -10,6 +10,7 @@ class AddIndexToProjectAuthorizations < ActiveRecord::Migration end def down - remove_index(:project_authorizations, :project_id) + remove_index(:project_authorizations, :project_id) if + Gitlab::Database.postgresql? end end From af8cc2e064bb97a8a1801521735d5403b189bfb5 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 14 Mar 2017 20:13:36 +0800 Subject: [PATCH 5/6] Use `remove_foreign_key :timelogs, name: '...'` Feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9908#note_25324225 --- .../20170124174637_add_foreign_keys_to_timelogs.rb | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/db/migrate/20170124174637_add_foreign_keys_to_timelogs.rb b/db/migrate/20170124174637_add_foreign_keys_to_timelogs.rb index 676e18cddd3..a7d4e141a1a 100644 --- a/db/migrate/20170124174637_add_foreign_keys_to_timelogs.rb +++ b/db/migrate/20170124174637_add_foreign_keys_to_timelogs.rb @@ -49,15 +49,8 @@ class AddForeignKeysToTimelogs < ActiveRecord::Migration Timelog.where('issue_id IS NOT NULL').update_all("trackable_id = issue_id, trackable_type = 'Issue'") Timelog.where('merge_request_id IS NOT NULL').update_all("trackable_id = merge_request_id, trackable_type = 'MergeRequest'") - constraint = - if Gitlab::Database.postgresql? - 'CONSTRAINT' - else - 'FOREIGN KEY' - end - - execute "ALTER TABLE timelogs DROP #{constraint} fk_timelogs_issues_issue_id" - execute "ALTER TABLE timelogs DROP #{constraint} fk_timelogs_merge_requests_merge_request_id" + remove_foreign_key :timelogs, name: 'fk_timelogs_issues_issue_id' + remove_foreign_key :timelogs, name: 'fk_timelogs_merge_requests_merge_request_id' remove_columns :timelogs, :issue_id, :merge_request_id end From 83e36064998f77f40c534bad531b6cea19ec198b Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 14 Mar 2017 21:40:58 +0800 Subject: [PATCH 6/6] Split to two commands, feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9908#note_25331127 --- .gitlab-ci.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index b1ca61604d5..406a0f3dcad 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -227,7 +227,8 @@ rake db:rollback: <<: *use-db <<: *dedicated-runner script: - - bundle exec rake db:rollback db:migrate STEP=120 + - bundle exec rake db:rollback STEP=120 + - bundle exec rake db:migrate rake db:seed_fu: stage: test