search

rowland / fb

Firebird Extension Library for Ruby
64 stars 35 forks source link

better handling of SQL_SHORT, SQL_LONG & SQL_INT64 datatypes #37

Closed vizcay closed 9 years ago

vizcay commented 9 years ago

Rowland:

I've refactored all the code that reads and writes SQL_SHORT, SQL_LONG & SQL_INT64 (scaled and not) and removed all the in between double conversions. Now BigDecimal is used when needed.

Also I've made all the changes trying to be 100% compatible with the current interface for setting values but this demanded odd elections at some points. I think that it will be a good idea to discuss what is the expected behaviour when setting values according to columns types, make a guide with it, refactor the code and do a semantic version bump to reflect the "incompatible change" (nothing very dramatic, 98% of users won't notice, specially if you stay safe when passing values to fb). Also there are a few commented assertions that should pass but I think the problem is related to ruby at the 64bit integer boundaries, I'll investigate further but nothing too relevant.

The tests now are a bit more slow because I've added a new file with 50 tests / 338 assertions all related to this.

Let me know what you think,

this commit closes #36

rowland commented 9 years ago

I'll review the code and test for compatibility with some existing codebases that will hopefully serve as a representative sample of other code in the wild.

vizcay commented 9 years ago

Hi Rowland, any progress reviewing this?

rowland commented 9 years ago

This PR breaks Ruby 1.8.7 compatibility. I'm trying to decide whether to care about pre-1.9 anymore.

vizcay commented 9 years ago

If it helps in any way: https://www.ruby-lang.org/en/news/2013/06/30/we-retire-1-8-7/

Bigdecimal is present in the stdlib 1.8.7, but after a fast google search it looks like they have changed a few things related to precision and representation between there an 1.9.3.. I bet there is a way to replace the flawed implementation with the new one (a backport) if someone really needs it.

rowland commented 9 years ago

Ruby 1.8.7 users can still use fb-0.7.4, I suppose. I'll declare the next version 0.8.0 for a clear line.