summaryrefslogtreecommitdiff
path: root/db
diff options
context:
space:
mode:
authorSean McGivern <sean@gitlab.com>2017-11-15 17:22:18 +0000
committerSean McGivern <sean@gitlab.com>2017-11-16 15:18:22 +0000
commit2cd98a638fe2700f4cf1bed1160717b269ce7856 (patch)
tree851511f9c838faca5c77bb0f6204c058cf9363f4 /db
parenta4072db0198896242886d22c644ed91c1016aa8d (diff)
downloadgitlab-ce-use-merge-requests-diff-id-column.tar.gz
Use latest_merge_request_diff associationuse-merge-requests-diff-id-column
Compared to the merge_request_diff association: 1. It's simpler to query. The query uses a foreign key to the merge_request_diffs table, so no ordering is necessary. 2. It's faster for preloading. The merge_request_diff association has to load every diff for the MRs in the set, then discard all but the most recent for each. This association means that Rails can just query for N diffs from N MRs. 3. It's more complicated to update. This is a bidirectional foreign key, so we need to update two tables when adding a diff record. This also means we need to handle this as a special case when importing a GitLab project. There is some juggling with this association in the merge request model: * `MergeRequest#latest_merge_request_diff` is _always_ the latest diff. * `MergeRequest#merge_request_diff` reuses `MergeRequest#latest_merge_request_diff` unless: * Arguments are passed. These are typically to force-reload the association. * It doesn't exist. That means we might be trying to implicitly create a diff. This only seems to happen in specs. * The association is already loaded. This is important for the reasons explained in the comment, which I'll reiterate here: if we a) load a non-latest diff, then b) get its `merge_request`, then c) get that MR's `merge_request_diff`, we should get the diff we loaded in c), even though that's not the latest diff. Basically, `MergeRequest#merge_request_diff` is the latest diff in most cases, but not quite all.
Diffstat (limited to 'db')
-rw-r--r--db/migrate/20171115164540_populate_merge_requests_latest_merge_request_diff_id_take_two.rb29
-rw-r--r--db/schema.rb2
2 files changed, 30 insertions, 1 deletions
diff --git a/db/migrate/20171115164540_populate_merge_requests_latest_merge_request_diff_id_take_two.rb b/db/migrate/20171115164540_populate_merge_requests_latest_merge_request_diff_id_take_two.rb
new file mode 100644
index 00000000000..6bbb8d3b8d1
--- /dev/null
+++ b/db/migrate/20171115164540_populate_merge_requests_latest_merge_request_diff_id_take_two.rb
@@ -0,0 +1,29 @@
+# This is identical to PopulateMergeRequestsLatestMergeRequestDiffId, which
+# already has specs.
+class PopulateMergeRequestsLatestMergeRequestDiffIdTakeTwo < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+ BATCH_SIZE = 1_000
+
+ class MergeRequest < ActiveRecord::Base
+ self.table_name = 'merge_requests'
+
+ include ::EachBatch
+ end
+
+ disable_ddl_transaction!
+
+ def up
+ update = '
+ latest_merge_request_diff_id = (
+ SELECT MAX(id)
+ FROM merge_request_diffs
+ WHERE merge_requests.id = merge_request_diffs.merge_request_id
+ )'.squish
+
+ MergeRequest.where(latest_merge_request_diff_id: nil).each_batch(of: BATCH_SIZE) do |relation|
+ relation.update_all(update)
+ end
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 37e08d453c8..e8caf37e99f 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: 20171106180641) do
+ActiveRecord::Schema.define(version: 20171115164540) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"