diff --git a/db/migrate/20170622135451_remove_duplicated_variable.rb b/db/migrate/20170622135451_remove_duplicated_variable.rb new file mode 100644 index 0000000000000000000000000000000000000000..bd3aa3f53236f0614d60d549c46b40066549733b --- /dev/null +++ b/db/migrate/20170622135451_remove_duplicated_variable.rb @@ -0,0 +1,45 @@ +class RemoveDuplicatedVariable < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + if Gitlab::Database.postgresql? + execute <<~SQL + DELETE FROM ci_variables var USING (#{duplicated_ids}) dup + #{join_conditions} + SQL + else + execute <<~SQL + DELETE var FROM ci_variables var INNER JOIN (#{duplicated_ids}) dup + #{join_conditions} + SQL + end + end + + def down + # noop + end + + def duplicated_ids + <<~SQL + SELECT MAX(id) AS id, #{key}, project_id + FROM ci_variables GROUP BY #{key}, project_id + SQL + end + + def join_conditions + <<~SQL + WHERE var.key = dup.key + AND var.project_id = dup.project_id + AND var.id <> dup.id + SQL + end + + def key + # key needs to be quoted in MySQL + quote_column_name('key') + end +end diff --git a/db/migrate/20170612150426_add_environment_scope_to_ci_variables.rb b/db/migrate/20170622135628_add_environment_scope_to_ci_variables.rb similarity index 100% rename from db/migrate/20170612150426_add_environment_scope_to_ci_variables.rb rename to db/migrate/20170622135628_add_environment_scope_to_ci_variables.rb diff --git a/db/migrate/20170622135728_add_unique_constraint_to_ci_variables.rb b/db/migrate/20170622135728_add_unique_constraint_to_ci_variables.rb new file mode 100644 index 0000000000000000000000000000000000000000..f953cd6641440802130d63b0bea43682f6f9718c --- /dev/null +++ b/db/migrate/20170622135728_add_unique_constraint_to_ci_variables.rb @@ -0,0 +1,23 @@ +class AddUniqueConstraintToCiVariables < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + unless index_exists?(:ci_variables, columns) + add_concurrent_index(:ci_variables, columns, unique: true) + end + end + + def down + if index_exists?(:ci_variables, columns) + remove_concurrent_index(:ci_variables, columns) + end + end + + def columns + @columns ||= [:project_id, :key, :environment_scope] + end +end diff --git a/db/schema.rb b/db/schema.rb index 006122bc7c73d582296039bf84c26e0cbe7b0981..fa66d515a0dc869fdd97e0024eb632abf620d11f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170621102400) do +ActiveRecord::Schema.define(version: 20170622135728) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -377,7 +377,7 @@ ActiveRecord::Schema.define(version: 20170621102400) do t.string "environment_scope", default: "*", null: false end - add_index "ci_variables", ["project_id"], name: "index_ci_variables_on_project_id", using: :btree + add_index "ci_variables", ["project_id", "key", "environment_scope"], name: "index_ci_variables_on_project_id_and_key_and_environment_scope", unique: true, using: :btree create_table "container_repositories", force: :cascade do |t| t.integer "project_id", null: false diff --git a/spec/migrations/remove_duplicated_variable_spec.rb b/spec/migrations/remove_duplicated_variable_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..9a521a7d980fb9751e31739f01dfdb04119c1a4f --- /dev/null +++ b/spec/migrations/remove_duplicated_variable_spec.rb @@ -0,0 +1,25 @@ +require 'spec_helper' +require Rails.root.join('db', 'migrate', '20170622135451_remove_duplicated_variable.rb') + +describe RemoveDuplicatedVariable, :migration do + let(:variables) { table(:ci_variables) } + let(:projects) { table(:projects) } + + before do + projects.create!(id: 1) + variables.create!(id: 1, key: 'key1', project_id: 1) + variables.create!(id: 2, key: 'key2', project_id: 1) + variables.create!(id: 3, key: 'keyX', project_id: 1) + variables.create!(id: 4, key: 'keyX', project_id: 1) + variables.create!(id: 5, key: 'keyY', project_id: 1) + variables.create!(id: 6, key: 'keyX', project_id: 1) + variables.create!(id: 7, key: 'key7', project_id: 1) + variables.create!(id: 8, key: 'keyY', project_id: 1) + end + + it 'correctly remove duplicated records with smaller id' do + migrate! + + expect(variables.pluck(:id)).to contain_exactly(1, 2, 6, 7, 8) + end +end