Clean up schema of the "issues" tablecleanup-issues-schema

This adds various foreign key constraints, indexes, missing NOT NULL
constraints, and changes some column types from timestamp to
timestamptz.

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

9 files changed, 247 insertions, 5 deletions

diff --git a/db/migrate/20171106132212_issues_confidential_not_null.rb b/db/migrate/20171106132212_issues_confidential_not_null.rb
new file mode 100644
index 00000000000..c959d2dd938
--- /dev/null
+++ b/db/migrate/20171106132212_issues_confidential_not_null.rb
@@ -0,0 +1,23 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class IssuesConfidentialNotNull < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  class Issue < ActiveRecord::Base
+    self.table_name = 'issues'
+  end
+
+  def up
+    Issue.where('confidential IS NULL').update_all(confidential: false)
+
+    change_column_null :issues, :confidential, false
+  end
+
+  def down
+    # There's no way / point to revert this.
+  end
+end
diff --git a/db/migrate/20171106135924_issues_milestone_id_foreign_key.rb b/db/migrate/20171106135924_issues_milestone_id_foreign_key.rb
new file mode 100644
index 00000000000..e6a780d0964
--- /dev/null
+++ b/db/migrate/20171106135924_issues_milestone_id_foreign_key.rb
@@ -0,0 +1,38 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class IssuesMilestoneIdForeignKey < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  class Issue < ActiveRecord::Base
+    include EachBatch
+
+    self.table_name = 'issues'
+
+    def self.with_orphaned_milestones
+      where('NOT EXISTS (SELECT true FROM milestones WHERE milestones.id = issues.milestone_id)')
+    end
+  end
+
+  def up
+    Issue.with_orphaned_milestones.each_batch(of: 100) do |batch|
+      batch.update_all(milestone_id: nil)
+    end
+
+    add_concurrent_foreign_key(
+      :issues,
+      :milestones,
+      column: :milestone_id,
+      on_delete: :nullify
+    )
+  end
+
+  def down
+    remove_foreign_key_without_error(:issues, column: :milestone_id)
+  end
+end
diff --git a/db/migrate/20171106150657_issues_updated_by_id_foreign_key.rb b/db/migrate/20171106150657_issues_updated_by_id_foreign_key.rb
new file mode 100644
index 00000000000..3b8844d7d9f
--- /dev/null
+++ b/db/migrate/20171106150657_issues_updated_by_id_foreign_key.rb
@@ -0,0 +1,45 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class IssuesUpdatedByIdForeignKey < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  class Issue < ActiveRecord::Base
+    include EachBatch
+
+    self.table_name = 'issues'
+
+    def self.with_orphaned_updaters
+      where('NOT EXISTS (SELECT true FROM users WHERE users.id = issues.updated_by_id)')
+        .where('updated_by_id IS NOT NULL')
+    end
+  end
+
+  def up
+    Issue.with_orphaned_updaters.each_batch(of: 100) do |batch|
+      batch.update_all(updated_by_id: nil)
+    end
+
+    # This index is only used for foreign keys, and those in turn will always
+    # specify a value. As such we can add a WHERE condition to make the index
+    # smaller.
+    add_concurrent_index(:issues, :updated_by_id, where: 'updated_by_id IS NOT NULL')
+
+    add_concurrent_foreign_key(
+      :issues,
+      :users,
+      column: :updated_by_id,
+      on_delete: :nullify
+    )
+  end
+
+  def down
+    remove_foreign_key_without_error(:issues, column: :updated_by_id)
+    remove_concurrent_index(:issues, :updated_by_id)
+  end
+end
diff --git a/db/migrate/20171106151218_issues_moved_to_id_foreign_key.rb b/db/migrate/20171106151218_issues_moved_to_id_foreign_key.rb
new file mode 100644
index 00000000000..8d2ceb8cc18
--- /dev/null
+++ b/db/migrate/20171106151218_issues_moved_to_id_foreign_key.rb
@@ -0,0 +1,44 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class IssuesMovedToIdForeignKey < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  class Issue < ActiveRecord::Base
+    include EachBatch
+
+    self.table_name = 'issues'
+
+    def self.with_orphaned_moved_to_issues
+      where('NOT EXISTS (SELECT true FROM issues WHERE issues.id = issues.moved_to_id)')
+        .where('moved_to_id IS NOT NULL')
+    end
+  end
+
+  def up
+    Issue.with_orphaned_moved_to_issues.each_batch(of: 100) do |batch|
+      batch.update_all(moved_to_id: nil)
+    end
+
+    add_concurrent_foreign_key(
+      :issues,
+      :issues,
+      column: :moved_to_id,
+      on_delete: :nullify
+    )
+
+    # We're using a partial index here so we only index the data we actually
+    # care about.
+    add_concurrent_index(:issues, :moved_to_id, where: 'moved_to_id IS NOT NULL')
+  end
+
+  def down
+    remove_foreign_key_without_error(:issues, column: :moved_to_id)
+    remove_concurrent_index(:issues, :moved_to_id)
+  end
+end
diff --git a/db/migrate/20171106154015_remove_issues_branch_name.rb b/db/migrate/20171106154015_remove_issues_branch_name.rb
new file mode 100644
index 00000000000..3d08225c96d
--- /dev/null
+++ b/db/migrate/20171106154015_remove_issues_branch_name.rb
@@ -0,0 +1,13 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class RemoveIssuesBranchName < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  def change
+    remove_column :issues, :branch_name, :string
+  end
+end
diff --git a/db/migrate/20171106155656_turn_issues_due_date_index_to_partial_index.rb b/db/migrate/20171106155656_turn_issues_due_date_index_to_partial_index.rb
new file mode 100644
index 00000000000..e4bed778695
--- /dev/null
+++ b/db/migrate/20171106155656_turn_issues_due_date_index_to_partial_index.rb
@@ -0,0 +1,37 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class TurnIssuesDueDateIndexToPartialIndex < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  NEW_INDEX_NAME = 'idx_issues_on_project_id_and_due_date_and_id_and_state_partial'
+  OLD_INDEX_NAME = 'index_issues_on_project_id_and_due_date_and_id_and_state'
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index(
+      :issues,
+      [:project_id, :due_date, :id, :state],
+      where: 'due_date IS NOT NULL',
+      name: NEW_INDEX_NAME
+    )
+
+    # We set the column name to nil as otherwise Rails will ignore the custom
+    # index name and remove the wrong index.
+    remove_concurrent_index(:issues, nil, name: OLD_INDEX_NAME)
+  end
+
+  def down
+    add_concurrent_index(
+      :issues,
+      [:project_id, :due_date, :id, :state],
+      name: OLD_INDEX_NAME
+    )
+
+    remove_concurrent_index(:issues, nil, name: NEW_INDEX_NAME)
+  end
+end
diff --git a/db/migrate/20171106171453_add_timezone_to_issues_closed_at.rb b/db/migrate/20171106171453_add_timezone_to_issues_closed_at.rb
new file mode 100644
index 00000000000..ad540b1e509
--- /dev/null
+++ b/db/migrate/20171106171453_add_timezone_to_issues_closed_at.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 AddTimezoneToIssuesClosedAt < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    change_column_type_concurrently(:issues, :closed_at, :datetime_with_timezone)
+  end
+
+  def down
+    cleanup_concurrent_column_type_change(:issues, :closed_at)
+  end
+end
diff --git a/db/post_migrate/20171106180641_cleanup_add_timezone_to_issues_closed_at.rb b/db/post_migrate/20171106180641_cleanup_add_timezone_to_issues_closed_at.rb
new file mode 100644
index 00000000000..88dd8f89ba6
--- /dev/null
+++ b/db/post_migrate/20171106180641_cleanup_add_timezone_to_issues_closed_at.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 CleanupAddTimezoneToIssuesClosedAt < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    cleanup_concurrent_column_type_change(:issues, :closed_at)
+  end
+
+  # rubocop:disable Migration/Datetime
+  def down
+    change_column_type_concurrently(:issues, :closed_at, :datetime)
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index c60cb729b75..37e08d453c8 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: 20171106101200) do
+ActiveRecord::Schema.define(version: 20171106180641) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -817,13 +817,12 @@ ActiveRecord::Schema.define(version: 20171106101200) do
     t.integer "project_id"
     t.datetime "created_at"
     t.datetime "updated_at"
-    t.string "branch_name"
     t.text "description"
     t.integer "milestone_id"
     t.string "state"
     t.integer "iid"
     t.integer "updated_by_id"
-    t.boolean "confidential", default: false
+    t.boolean "confidential", default: false, null: false
     t.datetime "deleted_at"
     t.date "due_date"
     t.integer "moved_to_id"
@@ -832,11 +831,11 @@ ActiveRecord::Schema.define(version: 20171106101200) do
     t.text "description_html"
     t.integer "time_estimate"
     t.integer "relative_position"
-    t.datetime "closed_at"
     t.integer "cached_markdown_version"
     t.datetime "last_edited_at"
     t.integer "last_edited_by_id"
     t.boolean "discussion_locked"
+    t.datetime_with_timezone "closed_at"
   end
 
   add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree
@@ -845,13 +844,15 @@ ActiveRecord::Schema.define(version: 20171106101200) do
   add_index "issues", ["deleted_at"], name: "index_issues_on_deleted_at", using: :btree
   add_index "issues", ["description"], name: "index_issues_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"}
   add_index "issues", ["milestone_id"], name: "index_issues_on_milestone_id", using: :btree
+  add_index "issues", ["moved_to_id"], name: "index_issues_on_moved_to_id", where: "(moved_to_id IS NOT NULL)", using: :btree
   add_index "issues", ["project_id", "created_at", "id", "state"], name: "index_issues_on_project_id_and_created_at_and_id_and_state", using: :btree
-  add_index "issues", ["project_id", "due_date", "id", "state"], name: "index_issues_on_project_id_and_due_date_and_id_and_state", using: :btree
+  add_index "issues", ["project_id", "due_date", "id", "state"], name: "idx_issues_on_project_id_and_due_date_and_id_and_state_partial", where: "(due_date IS NOT NULL)", using: :btree
   add_index "issues", ["project_id", "iid"], name: "index_issues_on_project_id_and_iid", unique: true, using: :btree
   add_index "issues", ["project_id", "updated_at", "id", "state"], name: "index_issues_on_project_id_and_updated_at_and_id_and_state", using: :btree
   add_index "issues", ["relative_position"], name: "index_issues_on_relative_position", using: :btree
   add_index "issues", ["state"], name: "index_issues_on_state", using: :btree
   add_index "issues", ["title"], name: "index_issues_on_title_trigram", using: :gin, opclasses: {"title"=>"gin_trgm_ops"}
+  add_index "issues", ["updated_by_id"], name: "index_issues_on_updated_by_id", where: "(updated_by_id IS NOT NULL)", using: :btree
 
   create_table "keys", force: :cascade do |t|
     t.integer "user_id"
@@ -1937,8 +1938,11 @@ ActiveRecord::Schema.define(version: 20171106101200) do
   add_foreign_key "issue_assignees", "issues", name: "fk_b7d881734a", on_delete: :cascade
   add_foreign_key "issue_assignees", "users", name: "fk_5e0c8d9154", on_delete: :cascade
   add_foreign_key "issue_metrics", "issues", on_delete: :cascade
+  add_foreign_key "issues", "issues", column: "moved_to_id", name: "fk_a194299be1", on_delete: :nullify
+  add_foreign_key "issues", "milestones", name: "fk_96b1dd429c", on_delete: :nullify
   add_foreign_key "issues", "projects", name: "fk_899c8f3231", on_delete: :cascade
   add_foreign_key "issues", "users", column: "author_id", name: "fk_05f1e72feb", on_delete: :nullify
+  add_foreign_key "issues", "users", column: "updated_by_id", name: "fk_ffed080f01", on_delete: :nullify
   add_foreign_key "label_priorities", "labels", on_delete: :cascade
   add_foreign_key "label_priorities", "projects", on_delete: :cascade
   add_foreign_key "labels", "namespaces", column: "group_id", on_delete: :cascade