diff options
| -rw-r--r-- | changelogs/unreleased/remove-all-varchar-limits.yml | 8 | ||||
| -rw-r--r-- | db/migrate/20170919220540_remove_varchar_limits.rb | 57 | ||||
| -rw-r--r-- | db/schema.rb | 2 | ||||
| -rw-r--r-- | spec/migrations/active_record/schema_spec.rb | 9 |
4 files changed, 75 insertions, 1 deletions
diff --git a/changelogs/unreleased/remove-all-varchar-limits.yml b/changelogs/unreleased/remove-all-varchar-limits.yml new file mode 100644 index 00000000000..1a0aeafbd64 --- /dev/null +++ b/changelogs/unreleased/remove-all-varchar-limits.yml @@ -0,0 +1,8 @@ +--- +title: Remove varchar length limits leftover in databases that were migrateid from + MySQL in the past. This means altering any tables that have varchar(255) or varchar(510) + to just be plain varchar. This brings these databases closer into sync with the + bundled schema.rb. +merge_request: +author: +type: other diff --git a/db/migrate/20170919220540_remove_varchar_limits.rb b/db/migrate/20170919220540_remove_varchar_limits.rb new file mode 100644 index 00000000000..df352a97be9 --- /dev/null +++ b/db/migrate/20170919220540_remove_varchar_limits.rb @@ -0,0 +1,57 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class RemoveVarcharLimits < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + # Because ALTER TABLE locks the table, even briefly, and we're doing + # this on many tables in an arbitrary order there would be a severe + # risk of a deadlock if we did it all in a single transaction. The + # only way to avoid it would be to ensure we did it in the same + # order that every other transaction accesses these tables which + # would be impossible. + disable_ddl_transaction! + + # We're removing varchar(510) and varchar(255) limits which are a + # relic of the MySQL->Postgres migration in the past... + # c.f. https://github.com/gitlabhq/mysql-postgresql-converter/issues/8 + + # Note that doing ALTER COLUMN to go from varchar(xxx) to plain + # varchar does not need to rewrite the table or even do a full table + # scan to verify the constraint. It does require obtaining a lock on + # the table so on very busy databases it could cause a short + # outage. Doing this in separate transactions should minimize this. + + def up + return unless Gitlab::Database.postgresql? + + connection.tables.each do |table| + next unless gitlab_table?(table) + + varchars = columns(table).select { |col| old_mysql_varchar?(col) } + next if varchars.empty? + + # Do a single change_table to alter column on all the columns in + # a table together to minimize the number of lock acquisitions + say "Removing #{varchars.length} leftover varchar limits from MySQL migration from table #{table}" + change_table table do |t| + varchars.each do |c| + t.change c.name, :string + end + end + end + end + + # No down method since these limits ought never have been present + + def gitlab_table?(table) + return table != 'schema_migrations' + end + + def old_mysql_varchar?(col) + return (col.cast_type.is_a?(ActiveRecord::Type::String) && + (col.cast_type.limit == 510 || col.cast_type.limit == 255)) + end +end diff --git a/db/schema.rb b/db/schema.rb index 3ec430c0078..6f3f5c4a5da 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: 20170918223303) do +ActiveRecord::Schema.define(version: 20170919220540) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" diff --git a/spec/migrations/active_record/schema_spec.rb b/spec/migrations/active_record/schema_spec.rb index e132529d8d8..3a2ecbf26eb 100644 --- a/spec/migrations/active_record/schema_spec.rb +++ b/spec/migrations/active_record/schema_spec.rb @@ -20,4 +20,13 @@ describe ActiveRecord::Schema do it '> schema version should equal the latest migration timestamp stored in schema_migrations table' do expect(latest_migration_timestamp).to eq(ActiveRecord::Migrator.current_version.to_i) end + + it '> schema does not contain any varchar(255) leftovers from MySQL' do + return unless ActiveRecord::Base.configurations[Rails.env]['adapter'] =~ /^postgresql/ + number_of_broken_columns = File.open(Rails.root.join('db', 'schema.rb')) do |file| + file.find_all { |line| line =~ /limit: (255|510)/ } + end.length + + expect(number_of_broken_columns).to eq(0) + end end |