rowland / fb

Firebird Extension Library for Ruby
64 stars 35 forks source link

fixed decimal values converted to floats #36

Closed vizcay closed 9 years ago

vizcay commented 9 years ago

Rowland: Hi, after your last fix about the rounding issue I've looked at little closer at the fb implementation because something got my attention (extracted from fb.c@2039)

static VALUE fb_cursor_fetch(struct FbCursor *fb_cursor)
(...)
case SQL_INT64:
        if (var->sqlscale < 0) {
          ratio = 1;
          for (scnt = 0; scnt > var->sqlscale; scnt--) ratio *= 10;
          dval = (double)*(ISC_INT64*)var->sqldata/ratio;
          val = rb_float_new(dval);
        } else {
          val = LL2NUM(*(ISC_INT64*)var->sqldata);
        }
  break;

FB is handling fixed precision decimals (sqlscale > 0) converting them back to C doubles and ruby floats..! But wait, I said.. at ActiveRecord I'm pretty sure I'm getting back BigDecimals.. that's true, but they are being instantiated from the Floats so the precision lost already happened!

I 've made this simple script to expose the issue:

require 'active_record'
require 'fb'

db = Fb::Database.new(:database => "#{File.expand_path(File.dirname(__FILE__))}/test.fdb", :username => 'sysdba', :password => 'masterkey')
conn = db.connect rescue db.create.connect
conn.execute("create table TESTS (ID int not null primary key, N decimal(18, 2))") if !conn.table_names.include?("TESTS")
conn.execute("delete from TESTS")
conn.execute("insert into TESTS values (?, ?)", 1, BigDecimal.new('91520.65'))
value = conn.query(:hash, "select * from TESTS").first['N']
puts 'fb directly'
puts "91520.65 = #{value}? (class #{value.class})"
puts "but actually.. surprise!! " + "%.13f" % value

CONFIG = <<-EOS
test:
  adapter: fb
  database: test.fdb
  username: sysdba
  password: masterkey
  host: localhost
  create: true
  encoding: UTF-8
EOS

ActiveRecord::Base.configurations = YAML::load(CONFIG)
ActiveRecord::Base.establish_connection('test')
class Test < ActiveRecord::Base
end
puts "\nthrough activerecord-fb-adapter"
value = Test.first.n
puts "91520.65 = #{value}? (class #{value.class})"
puts "\nerror only when displaying..?"
puts "915206500000 = #{(value * BigDecimal('10000000')).to_i} nope..!!"

That output's

fb directly
91520.65 = 91520.65? (class Float)
but actually.. surprise!! 91520.6499999999942

through activerecord-fb-adapter
91520.65 = 91520.64999999999? (class BigDecimal)

error only when displaying..?
915206500000 = 915206499999 nope..

So actually this is what is happening here:

So the question is.. do you think is possible to, instead of instantiate a Float, to return a BigDecimal there without creating havoc on the user base? I may even try to pull the patch myself..

Something like:

rb_funcall(rb_path2class("BigDecimal"), rb_intern("new"), 1, rb_str_new(str_decimal, 6));
rowland commented 9 years ago

fb originated before BigDecimal was available. Using double was non-ideal, but usable in a majority of cases and did not incur extra dependencies. A patch that improves the status quo without breaking backward compatibility or significantly impacting performance would be welcomed.

fb-only scenarios as well as ActiveRecord must be considered.