Raise ArgumentError for invalid :limit and :precision like as other options

When I've added new `:size` option in #35071, I've found that invalid
`:limit` and `:precision` raises `ActiveRecordError` unlike other
invalid options.

I think that is hard to distinguish argument errors and statement
invalid errors since the `StatementInvalid` is a subclass of the
`ActiveRecordError`.

c9e4c848ee/activerecord/lib/active_record/errors.rb (L103)

```ruby
begin
  # execute any migration
rescue ActiveRecord::StatementInvalid
  # statement invalid
rescue ActiveRecord::ActiveRecordError, ArgumentError
  # `ActiveRecordError` except `StatementInvalid` is maybe an argument error
end
```

I'd say this is the inconsistency worth fixing.

Before:

```ruby
add_column :items, :attr1, :binary,   size: 10      # => ArgumentError
add_column :items, :attr2, :decimal,  scale: 10     # => ArgumentError
add_column :items, :attr3, :integer,  limit: 10     # => ActiveRecordError
add_column :items, :attr4, :datetime, precision: 10 # => ActiveRecordError
```

After:

```ruby
add_column :items, :attr1, :binary,   size: 10      # => ArgumentError
add_column :items, :attr2, :decimal,  scale: 10     # => ArgumentError
add_column :items, :attr3, :integer,  limit: 10     # => ArgumentError
add_column :items, :attr4, :datetime, precision: 10 # => ArgumentError
```
This commit is contained in:
Ryuta Kamizono 2019-02-13 07:13:23 +09:00
parent c9e4c848ee
commit 20da6c7eac
10 changed files with 40 additions and 18 deletions

@ -1,3 +1,25 @@
* Raise `ArgumentError` for invalid `:limit` and `:precision` like as other options.
Before:
```ruby
add_column :items, :attr1, :binary, size: 10 # => ArgumentError
add_column :items, :attr2, :decimal, scale: 10 # => ArgumentError
add_column :items, :attr3, :integer, limit: 10 # => ActiveRecordError
add_column :items, :attr4, :datetime, precision: 10 # => ActiveRecordError
```
After:
```ruby
add_column :items, :attr1, :binary, size: 10 # => ArgumentError
add_column :items, :attr2, :decimal, scale: 10 # => ArgumentError
add_column :items, :attr3, :integer, limit: 10 # => ArgumentError
add_column :items, :attr4, :datetime, precision: 10 # => ArgumentError
```
*Ryuta Kamizono*
* Association loading isn't to be affected by scoping consistently
whether preloaded / eager loaded or not, with the exception of `unscoped`.

@ -1097,7 +1097,7 @@ def type_to_sql(type, limit: nil, precision: nil, scale: nil, **) # :nodoc:
if (0..6) === precision
column_type_sql << "(#{precision})"
else
raise(ActiveRecordError, "No #{native[:name]} type has precision of #{precision}. The allowed range of precision is from 0 to 6")
raise ArgumentError, "No #{native[:name]} type has precision of #{precision}. The allowed range of precision is from 0 to 6"
end
elsif (type != :primary_key) && (limit ||= native.is_a?(Hash) && native[:limit])
column_type_sql << "(#{limit})"

@ -240,7 +240,7 @@ def limit_to_size(limit, type)
when nil, 0x100..0xffff; nil
when 0x10000..0xffffff; "medium"
when 0x1000000..0xffffffff; "long"
else raise ActiveRecordError, "No #{type} type has byte size #{limit}"
else raise ArgumentError, "No #{type} type has byte size #{limit}"
end
end
end
@ -252,7 +252,7 @@ def integer_to_sql(limit)
when 3; "mediumint"
when nil, 4; "int"
when 5..8; "bigint"
else raise ActiveRecordError, "No integer type has byte size #{limit}. Use a decimal with scale 0 instead."
else raise ArgumentError, "No integer type has byte size #{limit}. Use a decimal with scale 0 instead."
end
end
end

@ -548,21 +548,21 @@ def type_to_sql(type, limit: nil, precision: nil, scale: nil, array: nil, **) #
# The hard limit is 1GB, because of a 32-bit size field, and TOAST.
case limit
when nil, 0..0x3fffffff; super(type)
else raise ActiveRecordError, "No binary type has byte size #{limit}. The limit on binary can be at most 1GB - 1byte."
else raise ArgumentError, "No binary type has byte size #{limit}. The limit on binary can be at most 1GB - 1byte."
end
when "text"
# PostgreSQL doesn't support limits on text columns.
# The hard limit is 1GB, according to section 8.3 in the manual.
case limit
when nil, 0..0x3fffffff; super(type)
else raise ActiveRecordError, "No text type has byte size #{limit}. The limit on text can be at most 1GB - 1byte."
else raise ArgumentError, "No text type has byte size #{limit}. The limit on text can be at most 1GB - 1byte."
end
when "integer"
case limit
when 1, 2; "smallint"
when nil, 3, 4; "integer"
when 5..8; "bigint"
else raise(ActiveRecordError, "No integer type has byte size #{limit}. Use a numeric with scale 0 instead.")
else raise ArgumentError, "No integer type has byte size #{limit}. Use a numeric with scale 0 instead."
end
else
super

@ -35,7 +35,7 @@ def test_column
def test_binary_columns_are_limitless_the_upper_limit_is_one_GB
assert_equal "bytea", @connection.type_to_sql(:binary, limit: 100_000)
assert_raise ActiveRecord::ActiveRecordError do
assert_raise ArgumentError do
@connection.type_to_sql(:binary, limit: 4294967295)
end
end

@ -64,7 +64,7 @@ def test_update_oid
def test_text_columns_are_limitless_the_upper_limit_is_one_GB
assert_equal "text", @connection.type_to_sql(:text, limit: 100_000)
assert_raise ActiveRecord::ActiveRecordError do
assert_raise ArgumentError do
@connection.type_to_sql(:text, limit: 4294967295)
end
end

@ -82,7 +82,7 @@ def test_passing_precision_to_datetime_does_not_set_limit
end
def test_invalid_datetime_precision_raises_error
assert_raises ActiveRecord::ActiveRecordError do
assert_raises ArgumentError do
@connection.create_table(:foos, force: true) do |t|
t.timestamps precision: 7
end

@ -176,9 +176,9 @@ def test_native_types
if current_adapter?(:Mysql2Adapter, :PostgreSQLAdapter)
def test_out_of_range_limit_should_raise
assert_raise(ActiveRecordError) { add_column :test_models, :integer_too_big, :integer, limit: 10 }
assert_raise(ActiveRecordError) { add_column :test_models, :text_too_big, :text, limit: 0xfffffffff }
assert_raise(ActiveRecordError) { add_column :test_models, :binary_too_big, :binary, limit: 0xfffffffff }
assert_raise(ArgumentError) { add_column :test_models, :integer_too_big, :integer, limit: 10 }
assert_raise(ArgumentError) { add_column :test_models, :text_too_big, :text, limit: 0xfffffffff }
assert_raise(ArgumentError) { add_column :test_models, :binary_too_big, :binary, limit: 0xfffffffff }
end
end
end

@ -583,7 +583,7 @@ def test_decimal_scale_without_precision_should_raise
if current_adapter?(:Mysql2Adapter, :PostgreSQLAdapter)
def test_out_of_range_integer_limit_should_raise
e = assert_raise(ActiveRecord::ActiveRecordError, "integer limit didn't raise") do
e = assert_raise(ArgumentError) do
Person.connection.create_table :test_integer_limits, force: true do |t|
t.column :bigone, :integer, limit: 10
end
@ -595,7 +595,7 @@ def test_out_of_range_integer_limit_should_raise
end
def test_out_of_range_text_limit_should_raise
e = assert_raise(ActiveRecord::ActiveRecordError, "text limit didn't raise") do
e = assert_raise(ArgumentError) do
Person.connection.create_table :test_text_limits, force: true do |t|
t.text :bigtext, limit: 0xfffffffff
end
@ -607,15 +607,15 @@ def test_out_of_range_text_limit_should_raise
end
def test_out_of_range_binary_limit_should_raise
e = assert_raise(ActiveRecord::ActiveRecordError) do
Person.connection.create_table :test_text_limits, force: true do |t|
e = assert_raise(ArgumentError) do
Person.connection.create_table :test_binary_limits, force: true do |t|
t.binary :bigbinary, limit: 0xfffffffff
end
end
assert_includes e.message, "No binary type has byte size #{0xfffffffff}"
ensure
Person.connection.drop_table :test_text_limits, if_exists: true
Person.connection.drop_table :test_binary_limits, if_exists: true
end
end

@ -75,7 +75,7 @@ def test_passing_precision_to_time_does_not_set_limit
end
def test_invalid_time_precision_raises_error
assert_raises ActiveRecord::ActiveRecordError do
assert_raises ArgumentError do
@connection.create_table(:foos, force: true) do |t|
t.time :start, precision: 7
t.time :finish, precision: 7