Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Always treat integer :limit as byte length. [#420 state:resolved]

  • Loading branch information...
commit baddea95e183b3bef290ec433f917ee8cd806934 1 parent 3610997
@tarmo tarmo authored jeremy committed
View
2  activerecord/CHANGELOG
@@ -1,5 +1,7 @@
*Edge*
+* Always treat integer :limit as byte length. #420 [Tarmo Tänav]
+
* Partial updates don't update lock_version if nothing changed. #426 [Daniel Morrison]
* Fix column collision with named_scope and :joins. #46 [Duncan Beevers, Mark Catley]
View
21 activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -99,7 +99,8 @@ def simplified_type(field_type)
end
def extract_limit(sql_type)
- if sql_type =~ /blob|text/i
+ case sql_type
+ when /blob|text/i
case sql_type
when /tiny/i
255
@@ -110,6 +111,10 @@ def extract_limit(sql_type)
else
super # we could return 65535 here, but we leave it undecorated by default
end
+ when /^int/i; 4
+ when /^bigint/i; 8
+ when /^smallint/i; 2
+ when /^mediumint/i; 3
else
super
end
@@ -168,7 +173,7 @@ class MysqlAdapter < AbstractAdapter
:primary_key => "int(11) DEFAULT NULL auto_increment PRIMARY KEY".freeze,
:string => { :name => "varchar", :limit => 255 },
:text => { :name => "text" },
- :integer => { :name => "int"},
+ :integer => { :name => "int", :limit => 4 },
:float => { :name => "float" },
:decimal => { :name => "decimal" },
:datetime => { :name => "datetime" },
@@ -467,14 +472,10 @@ def type_to_sql(type, limit = nil, precision = nil, scale = nil)
return super unless type.to_s == 'integer'
case limit
- when 0..3
- "smallint(#{limit})"
- when 4..8
- "int(#{limit})"
- when 9..20
- "bigint(#{limit})"
- else
- 'int(11)'
+ when 1..2; 'smallint'
+ when 3; 'mediumint'
+ when 4, nil; 'int(11)'
+ when 5..8; 'bigint'
end
end
View
16 activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -47,6 +47,12 @@ def initialize(name, default, sql_type = nil, null = true)
end
private
+ def extract_limit(sql_type)
+ return 8 if sql_type =~ /^bigint/i
+ return 2 if sql_type =~ /^smallint/i
+ super
+ end
+
# Extracts the scale from PostgreSQL-specific data types.
def extract_scale(sql_type)
# Money type has a fixed scale of 2.
@@ -785,12 +791,10 @@ def remove_index(table_name, options = {})
def type_to_sql(type, limit = nil, precision = nil, scale = nil)
return super unless type.to_s == 'integer'
- if limit.nil? || limit == 4
- 'integer'
- elsif limit < 4
- 'smallint'
- else
- 'bigint'
+ case limit
+ when 1..2; 'smallint'
+ when 3..4, nil; 'integer'
+ when 5..8; 'bigint'
end
end
View
5 activerecord/test/cases/migration_test.rb
@@ -173,6 +173,11 @@ def test_create_table_with_limits
assert_equal 'smallint', one.sql_type
assert_equal 'integer', four.sql_type
assert_equal 'bigint', eight.sql_type
+ elsif current_adapter?(:MysqlAdapter)
+ assert_match /^int\(\d+\)/, default.sql_type
+ assert_match /^smallint\(\d+\)/, one.sql_type
+ assert_match /^int\(\d+\)/, four.sql_type
+ assert_match /^bigint\(\d+\)/, eight.sql_type
elsif current_adapter?(:OracleAdapter)
assert_equal 'NUMBER(38)', default.sql_type
assert_equal 'NUMBER(1)', one.sql_type
View
46 activerecord/test/cases/schema_dumper_test.rb
@@ -72,6 +72,52 @@ def test_schema_dump_includes_not_null_columns
assert_match %r{:null => false}, output
end
+ def test_schema_dump_includes_limit_constraint_for_integer_columns
+ stream = StringIO.new
+
+ ActiveRecord::SchemaDumper.ignore_tables = [/^(?!integer_limits)/]
+ ActiveRecord::SchemaDumper.dump(ActiveRecord::Base.connection, stream)
+ output = stream.string
+
+ if current_adapter?(:PostgreSQLAdapter)
+ assert_match %r{c_int_1.*:limit => 2}, output
+ assert_match %r{c_int_2.*:limit => 2}, output
+
+ # int 3 is 4 bytes in postgresql
+ assert_match %r{c_int_3.*}, output
+ assert_no_match %r{c_int_3.*:limit}, output
+
+ assert_match %r{c_int_4.*}, output
+ assert_no_match %r{c_int_4.*:limit}, output
+ elsif current_adapter?(:MysqlAdapter)
+ assert_match %r{c_int_1.*:limit => 2}, 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.*}, output
+ assert_no_match %r{c_int_4.*:limit}, output
+ elsif current_adapter?(:SQLiteAdapter)
+ 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?(:SQLiteAdapter)
+ assert_match %r{c_int_5.*:limit => 5}, output
+ assert_match %r{c_int_6.*:limit => 6}, output
+ assert_match %r{c_int_7.*:limit => 7}, output
+ assert_match %r{c_int_8.*:limit => 8}, output
+ else
+ assert_match %r{c_int_5.*:limit => 8}, output
+ assert_match %r{c_int_6.*:limit => 8}, output
+ assert_match %r{c_int_7.*:limit => 8}, output
+ assert_match %r{c_int_8.*:limit => 8}, output
+ end
+ end
+
def test_schema_dump_with_string_ignored_table
stream = StringIO.new
View
7 activerecord/test/schema/schema.rb
@@ -407,6 +407,13 @@ def create_table(*args, &block)
t.column :key, :string
end
+ create_table :integer_limits, :force => true do |t|
+ t.integer :"c_int_without_limit"
+ (1..8).each do |i|
+ t.integer :"c_int_#{i}", :limit => i
+ end
+ end
+
except 'SQLite' do
# fk_test_has_fk should be before fk_test_has_pk
create_table :fk_test_has_fk, :force => true do |t|
Please sign in to comment.
Something went wrong with that request. Please try again.