Fix defaults for MR states and merge statuses

This ensures that merge_requests.state and merge_requests.merge_status
both have a proper default value and NOT NULL constraint on database
level. We also make sure to update any bogus rows first, without blowing
up the database.

Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/40534

changelogs/unreleased/default-values-for-mr-states.yml

@@ -0,0 +1,5 @@
+---
+title: Fix defaults for MR states and merge statuses
+merge_request:
+author:
+type: fixed

db/migrate/20171124125042_add_default_values_to_merge_request_states.rb

@@ -0,0 +1,19 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddDefaultValuesToMergeRequestStates < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  def up
+    change_column_default :merge_requests, :state, :opened
+    change_column_default :merge_requests, :merge_status, :unchecked
+  end
+
+  def down
+    change_column_default :merge_requests, :state, nil
+    change_column_default :merge_requests, :merge_status, nil
+  end
+end

db/migrate/20171124125748_populate_missing_merge_request_statuses.rb

@@ -0,0 +1,50 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class PopulateMissingMergeRequestStatuses < 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'
+  end
+
+  def up
+    say 'Populating missing merge_requests.state values'
+
+    # GitLab.com has no rows where "state" is NULL, and technically this should
+    # never happen. However it doesn't hurt to be 100% certain.
+    MergeRequest.where(state: nil).each_batch do |batch|
+      batch.update_all(state: 'opened')
+    end
+
+    say 'Populating missing merge_requests.merge_status values. ' \
+      'This will take a few minutes...'
+
+    # GitLab.com has 66 880 rows where "merge_status" is NULL, dating back all
+    # the way to 2011.
+    MergeRequest.where(merge_status: nil).each_batch(of: 10_000) do |batch|
+      batch.update_all(merge_status: 'unchecked')
+
+      # We want to give PostgreSQL some time to vacuum any dead tuples. In
+      # production we see it takes roughly 1 minute for a vacuuming run to clear
+      # out 10-20k dead tuples, so we'll wait for 90 seconds between every
+      # batch.
+      sleep(90) if sleep?
+    end
+  end
+
+  def down
+    # Reverting this makes no sense.
+  end
+
+  def sleep?
+    Rails.env.staging? || Rails.env.production?
+  end
+end

db/migrate/20171124132536_make_merge_request_statuses_not_null.rb

@@ -0,0 +1,14 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class MakeMergeRequestStatusesNotNull < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  def change
+    change_column_null :merge_requests, :state, false
+    change_column_null :merge_requests, :merge_status, false
+  end
+end

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: 20171121144800) do
+ActiveRecord::Schema.define(version: 20171124132536) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -1049,8 +1049,8 @@ ActiveRecord::Schema.define(version: 20171121144800) do
     t.datetime "created_at"
     t.datetime "updated_at"
     t.integer "milestone_id"
-    t.string "state"
+    t.string "state", default: "opened", null: false
-    t.string "merge_status"
+    t.string "merge_status", default: "unchecked", null: false
     t.integer "target_project_id", null: false
     t.integer "iid"
     t.text "description"