JRuby

Regression: Float#to_d doesn't work on 1.7 (did in 1.6)

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Blocker Blocker
  • Resolution: Fixed
  • Affects Version/s: JRuby 1.7.0.pre1
  • Fix Version/s: JRuby 1.7.0.pre2
  • Component/s: Standard Library
  • Labels:
    None
  • Number of attachments :
    0

Description

jruby --1.9 -v
=> jruby 1.6.7.dev (ruby-1.9.2-p312) (2012-02-06 32028b6) (Java HotSpot(TM) 64-Bit Server VM 1.7.0_02) [linux-amd64-java]
jruby --1.9 -e 'require "bigdecimal"; require "bigdecimal/util"; p 1.1.to_d'
=> #<BigDecimal:31799ca4,'0.11E1',2(4)>}}

jruby --1.9 -v
=> jruby 1.7.0.dev (ruby-1.9.3-p28) (2012-02-06 1c3a405) (Java HotSpot(TM) 64-Bit Server VM 1.7.0_02) [linux-amd64-java]
jruby --1.9 -e 'require "bigdecimal"; require "bigdecimal/util"; p 1.1.to_d'
=> TypeError: can't convert Float into String
     new at org/jruby/ext/bigdecimal/RubyBigDecimal.java:367
    to_d at /home/tim/.rbenv/versions/jruby/lib/ruby/1.9/bigdecimal/util.rb:31
  (root) at -e:1

Issue Links

is related to

Bug - A problem which impairs or prevents the functions of the product. JRUBY-6632 BigDecimal.new fails when passed anything but string (Float, BigDecimal, Fixnum)

  • Major - Major loss of function.
  • Resolved - A resolution has been taken, and it is awaiting verification by reporter. From here issues are either reopened, or are closed.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Hiro Asari added a comment -

This is a funny one. The bug itself is present in JRuby 1.6, but upgrading stdlib from 1.9.2 to 1.9.3 merely exposed it.

Compare Float#to_d in 1.9.2-p290: https://github.com/ruby/ruby/blob/866f369446da5fdbfa315f40571025b8075d31ce/ext/bigdecimal/lib/bigdecimal/util.rb#L18-22 and in 1.9.3-p0: https://github.com/ruby/ruby/blob/7da63cd80fa13606e17b29f3a79dc0b00e809ef0/ext/bigdecimal/lib/bigdecimal/util.rb#L18-33

The crucial difference is the presence of the second argument in the 1.9.3 version.

The code branch is different when we call BigDecimal.new with precision. JRuby code has not changed; even with 1.6.x, we still have this problem:

$ ./bin/jruby --1.9 -v; ./bin/jruby --1.9 -e 'require "bigdecimal"; p BigDecimal.new(1.1,16)'
jruby 1.6.7.dev (ruby-1.9.2-p312) (2012-02-06 9677219) (Java HotSpot(TM) 64-Bit Server VM 1.6.0_29) [darwin-x86_64-java]
TypeError: can't convert Float into String
     new at org/jruby/RubyBigDecimal.java:353
  (root) at -e:1

In general, we are dealing with the string representation of an Object when we get to this code path. However, currently RubyBasicObject.convertToString() is used, which is not good for Floats and Rationals. Thus the problem above manifests for Rationals as well:

$ jruby -v; jruby -e 'require "bigdecimal"; p BigDecimal.new(Rational(1,2),16)'
jruby 1.7.0.dev (ruby-1.9.3-p28) (2012-02-06 1c3a405) (Java HotSpot(TM) 64-Bit Server VM 1.6.0_29) [darwin-x86_64-java]
TypeError: can't convert Rational into String
     new at org/jruby/ext/bigdecimal/RubyBigDecimal.java:367
  (root) at -e:1

Note that switching to asString() is not sufficient here, since Rational#to_s returns something like "1/2", not "0.5".

I also note that the second argument has no effect on the return value.

Show
Hiro Asari added a comment - This is a funny one. The bug itself is present in JRuby 1.6, but upgrading stdlib from 1.9.2 to 1.9.3 merely exposed it. Compare Float#to_d in 1.9.2-p290: https://github.com/ruby/ruby/blob/866f369446da5fdbfa315f40571025b8075d31ce/ext/bigdecimal/lib/bigdecimal/util.rb#L18-22 and in 1.9.3-p0: https://github.com/ruby/ruby/blob/7da63cd80fa13606e17b29f3a79dc0b00e809ef0/ext/bigdecimal/lib/bigdecimal/util.rb#L18-33 The crucial difference is the presence of the second argument in the 1.9.3 version. The code branch is different when we call BigDecimal.new with precision. JRuby code has not changed; even with 1.6.x, we still have this problem: $ ./bin/jruby --1.9 -v; ./bin/jruby --1.9 -e 'require "bigdecimal"; p BigDecimal.new(1.1,16)' jruby 1.6.7.dev (ruby-1.9.2-p312) (2012-02-06 9677219) (Java HotSpot(TM) 64-Bit Server VM 1.6.0_29) [darwin-x86_64-java] TypeError: can't convert Float into String new at org/jruby/RubyBigDecimal.java:353 (root) at -e:1 In general, we are dealing with the string representation of an Object when we get to this code path. However, currently RubyBasicObject.convertToString() is used, which is not good for Floats and Rationals. Thus the problem above manifests for Rationals as well: $ jruby -v; jruby -e 'require "bigdecimal"; p BigDecimal.new(Rational(1,2),16)' jruby 1.7.0.dev (ruby-1.9.3-p28) (2012-02-06 1c3a405) (Java HotSpot(TM) 64-Bit Server VM 1.6.0_29) [darwin-x86_64-java] TypeError: can't convert Rational into String new at org/jruby/ext/bigdecimal/RubyBigDecimal.java:367 (root) at -e:1 Note that switching to asString() is not sufficient here, since Rational#to_s returns something like "1/2", not "0.5". I also note that the second argument has no effect on the return value.
Hide
Permalink
Jason Lunn added a comment -

Discovered this same issue without explicitly requiring when using the activerecord-jdbcsqlite3-adapter used by default with Rails 3.
If you have a migration that creates a high precision decimal for the purpose of saving latitude and longitude, for example:

class CreateLocations < ActiveRecord::Migration
  def self.up
    create_table :locations do |t|
      t.decimal :latitude, :precision => 15, :scale => 10
    end
  end
end

Then the mere act of assigning a value (39.75830078125 for example) will give you a nice stack trace:

TypeError: can't convert Float into String
	from org/jruby/ext/bigdecimal/RubyBigDecimal.java:443:in `new'
	from /Library/Frameworks/JRuby.framework/Versions/1.7.0.preview1/lib/ruby/1.9/bigdecimal/util.rb:31:in `to_d'
	from /Library/Frameworks/JRuby.framework/Gems/1.9/gems/activerecord-3.0.11/lib/active_record/connection_adapters/abstract/schema_definitions.rb:166:in `value_to_decimal'
	from /Library/Frameworks/JRuby.framework/Gems/1.9/gems/activerecord-jdbc-adapter-1.2.2/lib/arjdbc/sqlite3/adapter.rb:28:in `type_cast'
	from /Library/Frameworks/JRuby.framework/Gems/1.9/gems/activerecord-3.0.11/lib/active_record/attribute_methods/dirty.rb:83:in `field_changed?'
	from /Library/Frameworks/JRuby.framework/Gems/1.9/gems/activerecord-3.0.11/lib/active_record/attribute_methods/dirty.rb:57:in `write_attribute'
	from /Library/Frameworks/JRuby.framework/Gems/1.9/gems/activerecord-3.0.11/lib/active_record/attribute_methods/write.rb:14:in `latitude='

....
Show
Jason Lunn added a comment - Discovered this same issue without explicitly requiring when using the activerecord-jdbcsqlite3-adapter used by default with Rails 3. If you have a migration that creates a high precision decimal for the purpose of saving latitude and longitude, for example: class CreateLocations < ActiveRecord::Migration def self.up create_table :locations do |t| t.decimal :latitude, :precision => 15, :scale => 10 end end end Then the mere act of assigning a value (39.75830078125 for example) will give you a nice stack trace: TypeError: can't convert Float into String from org/jruby/ext/bigdecimal/RubyBigDecimal.java:443:in `new' from /Library/Frameworks/JRuby.framework/Versions/1.7.0.preview1/lib/ruby/1.9/bigdecimal/util.rb:31:in `to_d' from /Library/Frameworks/JRuby.framework/Gems/1.9/gems/activerecord-3.0.11/lib/active_record/connection_adapters/abstract/schema_definitions.rb:166:in `value_to_decimal' from /Library/Frameworks/JRuby.framework/Gems/1.9/gems/activerecord-jdbc-adapter-1.2.2/lib/arjdbc/sqlite3/adapter.rb:28:in `type_cast' from /Library/Frameworks/JRuby.framework/Gems/1.9/gems/activerecord-3.0.11/lib/active_record/attribute_methods/dirty.rb:83:in `field_changed?' from /Library/Frameworks/JRuby.framework/Gems/1.9/gems/activerecord-3.0.11/lib/active_record/attribute_methods/dirty.rb:57:in `write_attribute' from /Library/Frameworks/JRuby.framework/Gems/1.9/gems/activerecord-3.0.11/lib/active_record/attribute_methods/write.rb:14:in `latitude=' ....
Hide
Permalink
Jason Lunn added a comment -

Tweaking util.rb to change 

BigDecimal(self, precision || Float::DIG+1)

to 

BigDecimal(self.to_s, precision || Float::DIG+1)

resolves the symptom.

As to Hiro Asari's observation that nothing is being done with the second argument... it seems to be a straight forward change to RubyBigDecimal.java to alter the invocation of the Java BigDecimal constructor from the 

BigDecimal(String)

signature to 

BigDecimal(String, MathContext)

instead. MathContext will take an integer precision value as an argument to its constructor. As long as HALF_UP rounding mode is what is actually desired... can anyone comment on the intended rounding mode?

Show
Jason Lunn added a comment - Tweaking util.rb to change BigDecimal(self, precision || Float ::DIG+1) to BigDecimal(self.to_s, precision || Float ::DIG+1) resolves the symptom. As to Hiro Asari 's observation that nothing is being done with the second argument... it seems to be a straight forward change to RubyBigDecimal.java to alter the invocation of the Java BigDecimal constructor from the BigDecimal( String ) signature to BigDecimal( String , MathContext) instead. MathContext will take an integer precision value as an argument to its constructor. As long as HALF_UP rounding mode is what is actually desired... can anyone comment on the intended rounding mode?
Hide
Permalink
Charles Oliver Nutter added a comment -

Marking a blocker for 1.7pre2.

Show
Charles Oliver Nutter added a comment - Marking a blocker for 1.7pre2.
Hide
Permalink
Hiro Asari added a comment -

A possible fix is https://github.com/jruby/jruby/commit/2c12e33f65900c973fe8d49ad1019ef6c92b252d (JRUBY-6428 branch).

The cases mentioned here as well as in JRUBY-6632 should pass.

As for the "second argument is ignored bit", I am talking about our neglecting args[1] in RubyBigDecimal.newInstance(); we are still ignoring significant digits specified by the second argument.

Show
Hiro Asari added a comment - A possible fix is https://github.com/jruby/jruby/commit/2c12e33f65900c973fe8d49ad1019ef6c92b252d ( JRUBY-6428 branch). The cases mentioned here as well as in JRUBY-6632 should pass. As for the "second argument is ignored bit", I am talking about our neglecting args [1] in RubyBigDecimal.newInstance() ; we are still ignoring significant digits specified by the second argument.
Hide
Permalink
Hiro Asari added a comment -

Hmm. That broke some tests. http://travis-ci.org/#!/jruby/jruby/builds/1978399

Show
Hiro Asari added a comment - Hmm. That broke some tests. http://travis-ci.org/#!/jruby/jruby/builds/1978399
Hide
Permalink
Charles Oliver Nutter added a comment - 
commit 8ff1b9ef4f776f1812de0fb8e5c9162a38e9afe5
Author: Charles Oliver Nutter <headius@headius.com>
Date:   Wed Aug 1 16:43:24 2012 -0500

    Fix JRUBY-6428
    
    Regression: Float#to_d doesn't work on 1.7 (did in 1.6)
    
    I added specialized logic for instanceof RubyFloat and 1.9 mode,
    which appears to fix the issue without introducing regressions.
    The test patched here fails under 1.9.3 as well.

:100644 100644 6f39305... 502851e... M	src/org/jruby/ext/bigdecimal/RubyBigDecimal.java
:100644 100644 2d34bea... c114447... M	test/test_big_decimal.rb
Show
Charles Oliver Nutter added a comment - commit 8ff1b9ef4f776f1812de0fb8e5c9162a38e9afe5 Author: Charles Oliver Nutter <headius@headius.com> Date: Wed Aug 1 16:43:24 2012 -0500 Fix JRUBY-6428 Regression: Float#to_d doesn't work on 1.7 (did in 1.6) I added specialized logic for instanceof RubyFloat and 1.9 mode, which appears to fix the issue without introducing regressions. The test patched here fails under 1.9.3 as well. :100644 100644 6f39305... 502851e... M src/org/jruby/ext/bigdecimal/RubyBigDecimal.java :100644 100644 2d34bea... c114447... M test/test_big_decimal.rb

People

  • Assignee:
    Charles Oliver Nutter
    Reporter:
    Tim Felgentreff
Vote (1)
Watch (4)

Dates

  • Created:
    Updated:
    Resolved: