Merge branch 'integer_migration_style' into 'master'

Integer migration style

See merge request !6334

doc/development/migration_style_guide.md

@@ -111,6 +111,28 @@ class MyMigration < ActiveRecord::Migration
 end
 ```
 
+
+## Integer column type
+
+By default, an integer column can hold up to a 4-byte (32-bit) number. That is
+a max value of 2,147,483,647. Be aware of this when creating a column that will
+hold file sizes in byte units. If you are tracking file size in bytes this
+restricts the maximum file size to just over 2GB.
+
+To allow an integer column to hold up to an 8-byte (64-bit) number, explicitly
+set the limit to 8-bytes. This will allow the column to hold a value up to
+9,223,372,036,854,775,807.
+
+Rails migration example:
+
+```
+add_column_with_default(:projects, :foo, :integer, default: 10, limit: 8)
+
+# or
+
+add_column(:projects, :foo, :integer, default: 10, limit: 8)
+```
+
 ## Testing
 
 Make sure that your migration works with MySQL and PostgreSQL with data. An empty database does not guarantee that your migration is correct.

lib/gitlab/database/migration_helpers.rb

@@ -129,12 +129,14 @@ module Gitlab
       # column - The name of the column to add.
       # type - The column type (e.g. `:integer`).
       # default - The default value for the column.
+      # limit - Sets a column limit. For example, for :integer, the default is
+      #         4-bytes. Set `limit: 8` to allow 8-byte integers.
       # allow_null - When set to `true` the column will allow NULL values, the
       #              default is to not allow NULL values.
       #
       # This method can also take a block which is passed directly to the
       # `update_column_in_batches` method.
-      def add_column_with_default(table, column, type, default:, allow_null: false, &block)
+      def add_column_with_default(table, column, type, default:, limit: nil, allow_null: false, &block)
         if transaction_open?
           raise 'add_column_with_default can not be run inside a transaction, ' \
             'you can disable transactions by calling disable_ddl_transaction! ' \
@@ -144,7 +146,11 @@ module Gitlab
         disable_statement_timeout
 
         transaction do
-          add_column(table, column, type, default: nil)
+          if limit
+            add_column(table, column, type, default: nil, limit: limit)
+          else
+            add_column(table, column, type, default: nil)
+          end
 
           # Changing the default before the update ensures any newly inserted
           # rows already use the proper default value.

spec/lib/gitlab/database/migration_helpers_spec.rb

@@ -91,63 +91,80 @@ describe Gitlab::Database::MigrationHelpers, lib: true do
 
   describe '#add_column_with_default' do
     context 'outside of a transaction' do
-      before do
-        expect(model).to receive(:transaction_open?).and_return(false)
+      context 'when a column limit is not set' do
+        before do
+          expect(model).to receive(:transaction_open?).and_return(false)
 
-        expect(model).to receive(:transaction).and_yield
+          expect(model).to receive(:transaction).and_yield
 
-        expect(model).to receive(:add_column).
-          with(:projects, :foo, :integer, default: nil)
+          expect(model).to receive(:add_column).
+            with(:projects, :foo, :integer, default: nil)
 
-        expect(model).to receive(:change_column_default).
-          with(:projects, :foo, 10)
-      end
+          expect(model).to receive(:change_column_default).
+            with(:projects, :foo, 10)
+        end
 
-      it 'adds the column while allowing NULL values' do
-        expect(model).to receive(:update_column_in_batches).
-          with(:projects, :foo, 10)
+        it 'adds the column while allowing NULL values' do
+          expect(model).to receive(:update_column_in_batches).
+            with(:projects, :foo, 10)
 
-        expect(model).not_to receive(:change_column_null)
+          expect(model).not_to receive(:change_column_null)
 
-        model.add_column_with_default(:projects, :foo, :integer,
-                                      default: 10,
-                                      allow_null: true)
-      end
+          model.add_column_with_default(:projects, :foo, :integer,
+                                        default: 10,
+                                        allow_null: true)
+        end
 
-      it 'adds the column while not allowing NULL values' do
-        expect(model).to receive(:update_column_in_batches).
-          with(:projects, :foo, 10)
+        it 'adds the column while not allowing NULL values' do
+          expect(model).to receive(:update_column_in_batches).
+            with(:projects, :foo, 10)
 
-        expect(model).to receive(:change_column_null).
-          with(:projects, :foo, false)
+          expect(model).to receive(:change_column_null).
+            with(:projects, :foo, false)
 
-        model.add_column_with_default(:projects, :foo, :integer, default: 10)
-      end
+          model.add_column_with_default(:projects, :foo, :integer, default: 10)
+        end
 
-      it 'removes the added column whenever updating the rows fails' do
-        expect(model).to receive(:update_column_in_batches).
-          with(:projects, :foo, 10).
-          and_raise(RuntimeError)
+        it 'removes the added column whenever updating the rows fails' do
+          expect(model).to receive(:update_column_in_batches).
+            with(:projects, :foo, 10).
+            and_raise(RuntimeError)
 
-        expect(model).to receive(:remove_column).
-          with(:projects, :foo)
+          expect(model).to receive(:remove_column).
+            with(:projects, :foo)
 
-        expect do
-          model.add_column_with_default(:projects, :foo, :integer, default: 10)
-        end.to raise_error(RuntimeError)
+          expect do
+            model.add_column_with_default(:projects, :foo, :integer, default: 10)
+          end.to raise_error(RuntimeError)
+        end
+
+        it 'removes the added column whenever changing a column NULL constraint fails' do
+          expect(model).to receive(:change_column_null).
+            with(:projects, :foo, false).
+            and_raise(RuntimeError)
+
+          expect(model).to receive(:remove_column).
+            with(:projects, :foo)
+
+          expect do
+            model.add_column_with_default(:projects, :foo, :integer, default: 10)
+          end.to raise_error(RuntimeError)
+        end
       end
 
-      it 'removes the added column whenever changing a column NULL constraint fails' do
-        expect(model).to receive(:change_column_null).
-          with(:projects, :foo, false).
-          and_raise(RuntimeError)
+      context 'when a column limit is set' do
+        it 'adds the column with a limit' do
+          allow(model).to receive(:transaction_open?).and_return(false)
+          allow(model).to receive(:transaction).and_yield
+          allow(model).to receive(:update_column_in_batches).with(:projects, :foo, 10)
+          allow(model).to receive(:change_column_null).with(:projects, :foo, false)
+          allow(model).to receive(:change_column_default).with(:projects, :foo, 10)
 
-        expect(model).to receive(:remove_column).
-          with(:projects, :foo)
+          expect(model).to receive(:add_column).
+            with(:projects, :foo, :integer, default: nil, limit: 8)
 
-        expect do
-          model.add_column_with_default(:projects, :foo, :integer, default: 10)
-        end.to raise_error(RuntimeError)
+          model.add_column_with_default(:projects, :foo, :integer, default: 10, limit: 8)
+        end
       end
     end