[PATCH] Full high-precision decimal support for ActiveRecord, with testcases - MySQL, PGSQL, SQLite[23], DB2, SQL Server fully tested

rails-svn-r4479-add-decimal-type.diff

The above patch is against the latest SVN head at the time of submission (r4479).

robbat2,

Thanks for taking my patch further. I've been too busy to work on it lately. Can you explain what bit of my code was causing precision loss? I wasn't quite sure what you meant by "Also did weird casting to float/scale depending on the size of the number". I don't remember doing that on purpose! (Although I did return Integer objects if the scale was 0 as this would be an integer by definition.)

I'm just glad to see that decimal support might get into Rails now :D

This weekend I'll run the full test quite against one of my work's SQL Server installations and see if there are any issues.

Cheers Ashley

I couldn't find the exact source of precision loss in your code, or my original version - that was part of my reason for rewriting it.

Using scale=0 for Integer/Fixnum wasn't entirely safe for me, as I had some integers that were larger than 64-bits. They would technically be safe as BigNum, but I didn't see any advantage in mixing BigNum in, and it made things a pain for a few of the RDBMS that needed large numbers to be quoted.

The patch includes extraneous changes. Could you whittle it down to decimal support only?

Numeric with zero scale is a common way to get a bigint, so it's probably a good idea to return one.

Ran the tests against SQL Server 2000 last night and got these two errors (above what was failing before I patched the trunk, r4492):

  8) Failure:
test_add_table_with_decimals(MigrationTest) [./migration_test.rb:456]:
<2> expected but was
<#<BigDecimal:2510ba8,'0.3E1',4(8)>>.

  9) Failure:
test_change_column(MigrationTest) [./migration_test.rb:376]:
<nil> expected but was
<#<ActiveRecord::ConnectionAdapters::SQLServerColumn:0x24b99ac
 @default=true,
 @identity=false,
 @is_special=nil,
 @limit=nil,
 @name="approved",
 @null=true,
 @number=false,
 @precision=nil,
 @primary=nil,
 @scale=nil,
 @sql_type="bit(1)",
 @text=false,
 @type=:boolean>>.

I was going to clean up any "extraneous changes" but I couldn't see anything that wasn't just a minor tweak so I've left everything alone.

When I fix the SQL Server issues I could change the code back so it returns Integer objects when scale = 0. What is the concensus on this - should all decimals be BigDecimals or should integer decimals be Integers? There is no precision issue, but using Integers would be faster and more conceptually correct (IMHO).

Regarding robbat2's comment that "a few of the RDBMS need large numbers to be quoted", should that be fixed as a separate ticket?

patch tweaked for SQL Server

The code is now working against SQL Server 2000 - all the tests pass ok (that are relevant to this patch anyway). I've set it to return int when scale = 0. The changes are pretty minor either way so you can decide which is better. (I'd prefer ints, as there is no large-int type in SQL, and there is no loss of accuracy combinig BigDecimals with Integers in ruby)

Pending a patch limited to decimal support - please excise the many unrelated changes.

Ping - anyone pursuing this?

Bitsweat-

I'll have a look at this but I only wrote part of the code in this combined patch. Can you give me some clues to what the unrelated changes are? robbat said he had to make extra changes to get decimal support working so I don't know where the line is drawn

This is really important please integrate this guys! *BUMP*

I've gone through and found changes from ticket #3278 that got added into an early version of the decimal patch I wrote. As far as I can see, the only other changes are syntax (I tided up a few things). I've saved all the other changes so I'll submit them one ticket per fix when this gets included in the trunk.

Anything else need fixing? I'm dying to get this in Rails :)

Patch stripped down to just decimal support

(In [4596]) r4704@asus: jeremy | 2006-06-27 12:00:19 -0700

decimal r4705@asus: jeremy | 2006-06-27 12:20:47 -0700 current_adapter? checks whether any of its arguments is the name of the current adapter class r4834@asus: jeremy | 2006-07-08 13:08:24 -0700 Room to float. r4835@asus: jeremy | 2006-07-08 13:09:18 -0700 Give lock test a few chances. r4836@asus: jeremy | 2006-07-08 13:12:05 -0700 Numeric and decimal columns map to BigDecimal instead of Float. Those with scale 0 map to Integer. Closes #5454.

ActionWebService is now broken if you are using db columns that get cast to BigDecimal. I thought it would be an easy fix to just cast them to floats in ActionWebService, but the server would just return nil values when I did that. So either I missed something, or possibly it's causing some issues with the ruby soap extension?

Please open a new ticket for the AWS issue since this one's resolved.

For the record, there is still an issue here. I believe Rails goes too far when it converts a type DECIMAL with no scale to an INTEGER (line 180 of abstract_adapter.rb in Rails 1.2.3)

Why does it do that? If I specify a type :decimal, then I want a decimal, not an integer.

Sorry, correction... schema_definitions's simplified_type method is where the conversion from decimal to integer takes place.

In SQL, decimal is the "large" equivalent of float, but there is no large equivalent of integer. Ruby provides a Bignum class for this. Since a decimal with scale 0 IS an integer, that's what I made it return. It has the same mathematical properties either way; it just doesn't unnecessarily incur the overhead of a BigDecimal.

Yes, you can argue that a decimal with a scale of zero is an integer. Unfortunately, most databases will treat an integer as signed 32 bit. That is too small sometimes, but in Rails there is no way to get a formal bigint. So what's the alternative? We are forced to use a decimal, because that's the only way to get a numeric type that can hold a very large number. The type integer just doesn't cut it.

The conversion of a decimal to an integer is a premature optimization, and definitely non-obvious. I consider this a bug.

Also, I'd like to add that the large equivalent of an integer, in SQL, can be NUMERIC.

As cited in http://www.contrib.andrew.cmu.edu/~shadow/sql/sql1992.txt, section 4.1:

SQL defines distinct data types named by the following <key word>s: CHARACTER, CHARACTER VARYING, BIT, BIT VARYING, NUMERIC, DECIMAL, INTEGER, SMALLINT, FLOAT, REAL, DOUBLE PRECISION, DATE, TIME, TIMESTAMP, and INTERVAL.

Section 6.1 shows that you can specify the scale and precision of a NUMERIC, whereas you cannot with an INTEGER.

Therefore, I can specify a large equivalent of an integer by using NUMERIC.

I'm a little confused by your reply, and I'm not sure why you quoted the SQL92 spec. I was already aware that you can store large integers in an SQL database! That is why I mappened them to integers in Ruby.

Yes, you can argue that a decimal with a scale of zero is an integer. Unfortunately, most databases will treat an integer as signed 32 bit. - Yes, if the column type is INTEGER, not if it is DECIMAL/NUMERIC.

in Rails there is no way to get a formal bigint - There is, it's the Bignum class, and Ruby uses it as soon as a number is too large to fit inside a Fixnum, which is either a 32- or 64-bit int, depending on your system architecture.

You could change this mapping behaviour, but I don't see the point - it really seems too minor to bother with. Do you have a calculation in Ruby that is incorrect due to the use of integers? Surely that is the best definition of a bug here. If you do, then I will happily admit I made a mistake, but I was not aware of any limitations in the BigDecimal/Bignum maths in Ruby at the time.

1. It seems that treating scale 0 as Integers is not working, at least on Postgres:

t.column "ct", :integer, :limit => 8, :precision => 8, :scale => 0, :null => false # Shows as: ct numeric(8,0) NOT NULL

still gets mapped to BigDecimal, not Integer. (Rails 1.2.2/Postgres 8.2)

2. to_yaml doesn't work if there are BigDecimals I would suggest using #to_f when called by YAML:

class BigDecimal ; def to_yaml ; to_f.to_yaml ; end ; end

This seems to work fine, as arbitrary precision decimals are primitive YAML types.

-- Also: +1 on the original patcher's decision to treat scale 0 as Integer. Ruby will automatically handle Bignum's when you need it - why try to outsmart it? And the database will treat is as NUMERIC, so no worry there either.

Do you have a patch + tests for each?

Do you have a patch + tests for each?

Who is that comment addressing? The original poster included patches and tests. I was posting 2 bugs, and a suggested way to fix one of them. I certainly don't have a patch or test - it was more of a bug report with an idea on how to fix.

Addressing you :) Please attach patches for the issues you raised. The original patch still needs work as well.

Replying to bitsweat:

Addressing you :) Please attach patches for the issues you raised. The original patch still needs work as well.

Done. I put the patch and test in a sep. ticker #8746 , since it's tiny and stands on its own.

What else needs to be done for this ticket?

#8746 addresses the missing BigDecimal#to_yaml. What about a test that scale/precision behave correctly?