Include default column limits in schema.rb

Allows :limit defaults to be changed without pulling the rug out from
under old migrations that omitted :limit because it matched the default
at the time.

3 files changed, 23 insertions, 8 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 9e05ad1163..7d4869e113 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,8 @@
+*   Include default column limits in schema.rb. Allows defaults to be changed
+    in the future without affecting old migrations that assumed old defaults.
+
+    *Jeremy Kemper*
+
 *   MySQL: schema.rb now includes TEXT and BLOB column limits.
 
     *Jeremy Kemper*
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb
index 9bd0401e40..b05a4f8440 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb
@@ -19,12 +19,16 @@ module ActiveRecord
         spec = {}
         spec[:name]      = column.name.inspect
         spec[:type]      = column.type.to_s
-        spec[:limit]     = column.limit.inspect if column.limit != types[column.type][:limit]
+        spec[:null]      = 'false' unless column.null
+
+        limit = column.limit || types[column.type][:limit]
+        spec[:limit]     = limit.inspect if limit
         spec[:precision] = column.precision.inspect if column.precision
         spec[:scale]     = column.scale.inspect if column.scale
-        spec[:null]      = 'false' unless column.null
-        spec[:default]   = schema_default(column) if column.has_default?
-        spec.delete(:default) if spec[:default].nil?
+
+        default = schema_default(column) if column.has_default?
+        spec[:default]   = default unless default.nil?
+
         spec
       end
 
diff --git a/activerecord/test/cases/schema_dumper_test.rb b/activerecord/test/cases/schema_dumper_test.rb
index c8fc297a7a..72fb296825 100644
--- a/activerecord/test/cases/schema_dumper_test.rb
+++ b/activerecord/test/cases/schema_dumper_test.rb
@@ -100,7 +100,11 @@ class SchemaDumperTest < ActiveRecord::TestCase
   def test_schema_dump_includes_limit_constraint_for_integer_columns
     output = dump_all_table_schema([/^(?!integer_limits)/])
 
+    assert_match %r{c_int_without_limit}, output
+
     if current_adapter?(:PostgreSQLAdapter)
+      assert_no_match %r{c_int_without_limit.*limit:}, output
+
       assert_match %r{c_int_1.*limit: 2}, output
       assert_match %r{c_int_2.*limit: 2}, output
 
@@ -111,6 +115,8 @@ class SchemaDumperTest < ActiveRecord::TestCase
       assert_match %r{c_int_4.*}, output
       assert_no_match %r{c_int_4.*limit:}, output
     elsif current_adapter?(:MysqlAdapter, :Mysql2Adapter)
+      assert_match %r{c_int_without_limit.*limit: 4}, output
+
       assert_match %r{c_int_1.*limit: 1}, output
       assert_match %r{c_int_2.*limit: 2}, output
       assert_match %r{c_int_3.*limit: 3}, output
@@ -118,13 +124,13 @@ class SchemaDumperTest < ActiveRecord::TestCase
       assert_match %r{c_int_4.*}, output
       assert_no_match %r{c_int_4.*:limit}, output
     elsif current_adapter?(:SQLite3Adapter)
+      assert_no_match %r{c_int_without_limit.*limit:}, output
+
       assert_match %r{c_int_1.*limit: 1}, output
       assert_match %r{c_int_2.*limit: 2}, output
       assert_match %r{c_int_3.*limit: 3}, output
       assert_match %r{c_int_4.*limit: 4}, output
     end
-    assert_match %r{c_int_without_limit.*}, output
-    assert_no_match %r{c_int_without_limit.*limit:}, output
 
     if current_adapter?(:SQLite3Adapter)
       assert_match %r{c_int_5.*limit: 5}, output
@@ -354,7 +360,7 @@ class SchemaDumperTest < ActiveRecord::TestCase
     match = output.match(%r{create_table "goofy_string_id"(.*)do.*\n(.*)\n})
     assert_not_nil(match, "goofy_string_id table not found")
     assert_match %r(id: false), match[1], "no table id not preserved"
-    assert_match %r{t.string[[:space:]]+"id",[[:space:]]+null: false$}, match[2], "non-primary key id column not preserved"
+    assert_match %r{t.string\s+"id",.*?null: false$}, match[2], "non-primary key id column not preserved"
   end
 
   def test_schema_dump_keeps_id_false_when_id_is_false_and_unique_not_null_column_added
@@ -433,7 +439,7 @@ class SchemaDumperDefaultsTest < ActiveRecord::TestCase
   def test_schema_dump_defaults_with_universally_supported_types
     output = dump_table_schema('defaults')
 
-    assert_match %r{t\.string\s+"string_with_default",\s+default: "Hello!"}, output
+    assert_match %r{t\.string\s+"string_with_default",.*?default: "Hello!"}, output
     assert_match %r{t\.date\s+"date_with_default",\s+default: '2014-06-05'}, output
     assert_match %r{t\.datetime\s+"datetime_with_default",\s+default: '2014-06-05 07:17:04'}, output
     assert_match %r{t\.time\s+"time_with_default",\s+default: '2000-01-01 07:17:04'}, output