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

          Activity

          Hide
          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
          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
          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
          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
          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
          Hiro Asari added a comment -
          Show
          Hiro Asari added a comment - Hmm. That broke some tests. http://travis-ci.org/#!/jruby/jruby/builds/1978399
          Hide
          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
            • Votes:
              1 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: