Rails core - Mar 2008 - ActiveRecord bug? ActiveRecord::ConnectionAdapters::Column.type_cast for float columns

Hi all,

I just noticed that in ActiveRecord::ConnectionAdapters::Column.type_cast[1]
(and type_cast_code[2]) the type casting for float columns simply does
value.to_f, whereas all the others have a rescue or call out to a method,
the default implementations of which have rescues or enough logic in that I
don''t think they''d fail.

Imagine the following:

create_table :vehicle do |t|


Now, doing some archaeology on trac, I''ve found that the rescue for
integer
columns was added way back in changeset 928 [3] based on ticket 820 [4].
Putting a rescue block on the type cast would be nice and consistent, and
simple, so I can create a patch for this easily.  However, I haven''t
created
a patch for this super-tiny thing as I don''t really know if
it''s
intentionally missing or not.  I just want some feedback on if this is
expected behaviour or not before I patch it up (and deal with the no-doubt
cascade of failing tests from changing the behaviour).

If it''s expected behaviour I''ll just patch my version of rails
for this app.



[1]
http://dev.rubyonrails.org/browser/trunk/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb#L57
[2]
http://dev.rubyonrails.org/browser/trunk/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb#L75
[3] http://dev.rubyonrails.org/changeset/928
[4] http://dev.rubyonrails.org/ticket/820

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

Crap, I wasn''t finished writing this. Curse gmail! (and not my fingers,
no,
not their fault at all).

On 19/03/2008, Murray Steele <murray.steele@gmail.com>
wrote:
>
> Hi all,
>
> I just noticed that in
ActiveRecord::ConnectionAdapters::Column.type_cast[1]
> (and type_cast_code[2]) the type casting for float columns simply does
> value.to_f, whereas all the others have a rescue or call out to a method,
> the default implementations of which have rescues or enough logic in that I
> don''t think they''d fail.
>
> Imagine the following:

create_table :vehicle do |t|
  t.integer :wheels
  t.float :top_speed
end

v = Vehicle.new
v.wheels = []
v.wheels #=> 0

v.top_speed = []
v.top_speed #=> NoMethodError: undefined method `to_f'' for []:Array

It seems that we should be able to set wheels and top_speed to be the same
wonky value if we want and have AR cope with that (we''ll use
validations to
complain to the user later).

Now, doing some archaeology on trac, I''ve found that the rescue for
integer
> columns was added way back in changeset 928 [3] based on ticket 820 [4].
> Putting a rescue block on the type cast would be nice and consistent, and
> simple, so I can create a patch for this easily.  However, I
haven''t created
> a patch for this super-tiny thing as I don''t really know if
it''s
> intentionally missing or not.  I just want some feedback on if this is
> expected behaviour or not before I patch it up (and deal with the no-doubt
> cascade of failing tests from changing the behaviour).
>
> If it''s expected behaviour I''ll just patch my version of
rails for this
> app.
>
>
>
> [1]
>
http://dev.rubyonrails.org/browser/trunk/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb#L57
> [2]
>
http://dev.rubyonrails.org/browser/trunk/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb#L75
> [3] http://dev.rubyonrails.org/changeset/928
> [4] http://dev.rubyonrails.org/ticket/820
>
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

For those who care, a patch to resolve this:
http://dev.rubyonrails.org/ticket/11398

On 19/03/2008, Murray Steele <murray.steele@gmail.com>
wrote:
>
> Crap, I wasn''t finished writing this. Curse gmail! (and not my
fingers,
> no, not their fault at all).
>
> On 19/03/2008, Murray Steele <murray.steele@gmail.com> wrote:
> >
> > Hi all,
> >
> > I just noticed that in ActiveRecord::ConnectionAdapters::
> > Column.type_cast[1] (and type_cast_code[2]) the type casting for float
> > columns simply does value.to_f, whereas all the others have a rescue
or
> > call out to a method, the default implementations of which have
rescues or
> > enough logic in that I don''t think they''d fail.
> >
> > Imagine the following:
>
>
> create_table :vehicle do |t|
>   t.integer :wheels
>   t.float :top_speed
> end
>
> v = Vehicle.new
> v.wheels = []
> v.wheels #=> 0
>
> v.top_speed = []
> v.top_speed #=> NoMethodError: undefined method `to_f'' for
[]:Array
>
> It seems that we should be able to set wheels and top_speed to be the same
> wonky value if we want and have AR cope with that (we''ll use
validations to
> complain to the user later).
>
> Now, doing some archaeology on trac, I''ve found that the rescue
for
> > integer columns was added way back in changeset 928 [3] based on
ticket 820
> > [4].  Putting a rescue block on the type cast would be nice and
consistent,
> > and simple, so I can create a patch for this easily.  However, I
haven''t
> > created a patch for this super-tiny thing as I don''t really
know if it''s
> > intentionally missing or not.  I just want some feedback on if this is
> > expected behaviour or not before I patch it up (and deal with the
no-doubt
> > cascade of failing tests from changing the behaviour).
> >
> > If it''s expected behaviour I''ll just patch my
version of rails for this
> > app.
> >
> >
> >
> > [1]
> >
http://dev.rubyonrails.org/browser/trunk/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb#L57
> > [2]
> >
http://dev.rubyonrails.org/browser/trunk/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb#L75
> > [3] http://dev.rubyonrails.org/changeset/928
> > [4] http://dev.rubyonrails.org/ticket/820
> >
>
>
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---