[PATCH] Models for tables with numeric columns quietly insert zeroes on invalid user input

This is a feature request in my eyes. There are lots of ways around it, but to have it automatically handled would be nicer.

Let me explain why I think this is more serious than you might feel.

For you, this isn't an issue: I haven't seen any numeric fields on any of the sites you've created, and this has never bitten you.

However, I'm hoping to see Rails push Ruby into the mainstream. One of the things that is a passport for Rails is the scaffolding, and automatic form generation. So folks start using Rails, and the first thing they try is to create a scaffold. They bring up the app, type some stuff into fields, press enter, and discover that the values stored in the database are not what they typed. They go "hmm, let's try some other framework, this one is clearly buggy" and we lose a punter.

This isn't an obscure bug: I ran into it in the very first example I put together for the book.

Your call, but I had to vote at least once to have this treated more seriously.

Cheers

Dave

I'm with Dave on this, after discussing it with David HH, i'm returning this to a bug.

We'll fix it before 1.0

With last patch nil is inserted instead of zero, thus allowing validations to be performed on column name instead of column_before_type_cast. Note that validation is not added automatically.

We could do following:

1) if column can be NULL, eval for all cloumns with numeric type

validates_numericality_of :column, :only_integer => true, :allow_nil => true

2) else eval

validates_numericality_of :column, :only_integer => true, :allow_nil => false

Currently can_be_null property of columns is not retrieved. I know that this would be possible with mysql, dont know about other databases and I cant try them out.

Second attempt

Typecasts to value instead of nil, still no automatic vaidation (yet).

If we do automatic validation of numeric columns, we just as well add others things like:

* validates_presence_of attribute
* validates_length_of attribute, :maximum=>30
* validates_uniqueness_of attribute

To this date we are not inferring validations from database schema at all, instead we rely on the user to add them. Probably there is a reason for it, that is not clear to me. Any ideas?

Numeric type conversion in low level adapters now return original unconverted value instead of zero or nil. Better to have an erroneous value than a silent conversion to something valid.

Rewrote validates_numericality_of, to check type of column value instead of relying regular expressions being applied to column_before_type_cast.

Inferred can_be_null column property from database. Note that this was only done for myqql. Other adapters should be modified also. I need a helping hand with this ...

Base#attributes_from_column_definition now uses Base#columns instead of going though connection adapter. I'm not sure if this was by design but fields where queried when there should have not been.

Implemented new autovalidation feature for integers and floats, by adding validates_numericality_of inside diverted method Base#column

Added table definition for mysql 'floattests'. Other adapters are missing this as I had no access each kind of database, and it did not want to play on changes that I cant verify at least syntactically. ValidationsTest.test_throw_away_typing thrown away (pun intended). ValidationsTest.test_validates_numericality_of removed some float test values as they failed autovalidation for integers. Perhaps this test case should be deleted also?

Introduced some new test cases: test_autovalidates_numericality_of_int_with_string_fail test_autovalidates_numericality_of_int_with_string_pass test_autovalidates_numericality_of_float_with_string_fail test_autovalidates_numericality_of_float_with_string_pass

Extended my previous patch do to autovalidation for validate_length_of. Validations can now be disabled globaly and for individual attributes. By default auto validation is on. This meant that some testcases broke, and other changes where needed also (like validate_length_of on Value clases).

See previous description for other changes.

From what Dave originally said on this ticket, it sounds like the problem is mostly when using scaffolding. Perhaps the scaffolding could also auto-generate some validation code?

Nope, it happens regardless of scaffolding or not. Problem is how typecasting is performed at connection adapter level. See relevant snippet from abstract_adapter.rb

 def type_cast(value)
        if value.nil? then return nil end
        case type
          when :string    then value
          when :text      then value
          when :integer   then value.to_i rescue value ? 1 : 0
# ... snip ...

If column was inferred to have type integer and you have a string value it will be casted to zero.

irb(main):002:0> "wibble".to_i
=> 0

I suggest performing the cast only if it is possible, otherwise original string value should be kept so that validation would be possible. Database will not let us save a string for an integer anyway. My second patch numeric_columns_quietly_insert_zeroes2.diff does exactly this.

Forth patch does autovalidation also so that non attentive users will get the benefit too.

Any progress on a fix + tests?

This sounds somewhat like the issue described in #6156 and fixed in [5901], [5902], [5903], and [5905]. Not sure if that's the case.

Issue still exists. I tried validating a numeric column using:

validates_format_of :num_variable, :with => /\A[0-9]+\Z/

Entering a string such as "wibble" does not throw an error. Instead 0 is entered into the column.

Currently using Rails 1.2.3.

#3142