Skip to content

Commit d183184

Browse files
committed
Change how we handle limits for columns and how we store @limit for making schema_dumper_test work.
We now store @limit as 2 for smallint and 8 for bigint (so schema dumper will output :integer, :limit => 2 or 8 and we'll get smallint or bigint columns created accordingly. We also say that if limit is <3 it's a smallint and >4 it's a bigint, before you had to be limit ==4 to be an integer and limit ==3 would be a smallint, which is not quite correct. (Limit of 3 is bigger than a smallint).
1 parent 38aa198 commit d183184

File tree

2 files changed

+63
-5
lines changed

2 files changed

+63
-5
lines changed

lib/active_record/connection_adapters/sqlserver_adapter.rb

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,13 @@ def initialize(info)
6464
@identity = info[:is_identity]
6565
@is_special = ["text", "ntext", "image"].include?(info[:type])
6666
# TODO: check ok to remove @scale = scale_value
67+
@limit = nil unless limitable?(type)
68+
end
69+
70+
def limitable?(type)
6771
# SQL Server only supports limits on *char and float types
68-
@limit = nil unless @type == :float or @type == :string
72+
# although for schema dumping purposes it's useful to know that (big|small)int are 2|8 respectively.
73+
@type == :float || @type == :string || (@type == :integer && type =~ /^(big|small)int/)
6974
end
7075

7176
def simplified_type(field_type)
@@ -223,13 +228,15 @@ def supports_migrations? #:nodoc:
223228

224229
def type_to_sql(type, limit = nil, precision = nil, scale = nil) #:nodoc:
225230
return super unless type.to_s == 'integer'
226-
227-
if limit.nil? || limit == 4
231+
232+
if limit.nil?
228233
'integer'
229-
elsif limit < 4
234+
elsif limit > 4
235+
'bigint'
236+
elsif limit < 3
230237
'smallint'
231238
else
232-
'bigint'
239+
'integer'
233240
end
234241
end
235242

test/cases/adapter_test_sqlserver.rb

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,57 @@ def assert_all_statements_used_are_closed(&block)
9494
end
9595
end
9696

97+
class TypeToSqlForIntegersTest < ActiveRecord::TestCase
98+
99+
def setup
100+
@connection = ActiveRecord::Base.connection
101+
end
102+
103+
# TODO - ugh these tests are pretty literal...
104+
def test_should_create_integers_when_no_limit_supplied
105+
assert_equal 'integer', @connection.type_to_sql(:integer)
106+
end
107+
108+
def test_should_create_integers_when_limit_is_4
109+
assert_equal 'integer', @connection.type_to_sql(:integer, 4)
110+
end
111+
112+
def test_should_create_integers_when_limit_is_3
113+
assert_equal 'integer', @connection.type_to_sql(:integer, 3)
114+
end
115+
116+
def test_should_create_smallints_when_limit_is_less_than_3
117+
assert_equal 'smallint', @connection.type_to_sql(:integer, 2)
118+
assert_equal 'smallint', @connection.type_to_sql(:integer, 1)
119+
end
120+
121+
def test_should_create_bigints_when_limit_is_greateer_than_4
122+
assert_equal 'bigint', @connection.type_to_sql(:integer, 5)
123+
assert_equal 'bigint', @connection.type_to_sql(:integer, 6)
124+
assert_equal 'bigint', @connection.type_to_sql(:integer, 7)
125+
assert_equal 'bigint', @connection.type_to_sql(:integer, 8)
126+
end
127+
128+
end
129+
130+
# NOTE: The existing schema_dumper_test doesn't test the limits of <4 limit things
131+
# for adapaters that aren't mysql, sqlite or postgres. We should.
132+
class SchemaDumperForSqlServerTest < ActiveRecord::TestCase
133+
def test_schema_dump_includes_limit_constraint_for_integer_columns
134+
stream = StringIO.new
135+
136+
ActiveRecord::SchemaDumper.ignore_tables = [/^(?!integer_limits)/]
137+
ActiveRecord::SchemaDumper.dump(ActiveRecord::Base.connection, stream)
138+
output = stream.string
139+
assert_match %r{c_int_1.*:limit => 2}, output
140+
assert_match %r{c_int_2.*:limit => 2}, output
141+
assert_match %r{c_int_3.*}, output
142+
assert_match %r{c_int_4.*}, output
143+
assert_no_match %r{c_int_3.*:limit}, output
144+
assert_no_match %r{c_int_4.*:limit}, output
145+
end
146+
end
147+
97148
class StringDefaultsTest < ActiveRecord::TestCase
98149
class StringDefaults < ActiveRecord::Base; end;
99150

0 commit comments

Comments
 (0)