2022 regression: negative DECIMAL, NUMERIC and DECFLOAT numbers in result sets are truncated to negative integers in fetch_array, fetch_assoc, fetch_object #172
Comments
|
I have the same problem |
|
Hi Olivier,
Here is the patch we made in our private fork while waiting for an upstream
fix.
diff --git a/IBM_DB_Adapter/ibm_db/ext/ibm_db.c
b/IBM_DB_Adapter/ibm_db/ext/ibm_db.c
index 232b879..6640680 100644
--- a/IBM_DB_Adapter/ibm_db/ext/ibm_db.c
+++ b/IBM_DB_Adapter/ibm_db/ext/ibm_db.c
@@ -9578,7 +9578,10 @@ static VALUE
_ruby_ibm_db_bind_fetch_helper(ibm_db_fetch_helper_args *data)
return Qnil;
}
- if ((atof(row_data->str_val) - atol(row_data->str_val)) > 0)
+ /* if ((atof(row_data->str_val) - atol(row_data->str_val)) > 0)
*/
+ /* The logic above does not work to detect *negative*
decimals/floats. */
+ /* Also, these should always be returned as BigDecimal, never as
Integer. */
+ /* See: #172 */
{
strcpy(tmpStr, "BigDecimal(\'");
strcat(tmpStr, row_data->str_val);
@@ -9596,17 +9599,17 @@ static VALUE
_ruby_ibm_db_bind_fetch_helper(ibm_db_fetch_helper_args *data)
ruby_xfree(tmpStr);
tmpStr = NULL;
}
- else
- {
- if ( op & FETCH_ASSOC ) {
- rb_hash_aset(return_value, colName, LONG2NUM(atol((char
*)row_data->str_val)));
- }
- if ( op == FETCH_INDEX ) {
- rb_ary_store(return_value, i, LONG2NUM(atol((char
*)row_data->str_val)));
- } else if ( op == FETCH_BOTH ) {
- rb_hash_aset(return_value, INT2NUM(i), LONG2NUM(atol((char
*)row_data->str_val)));
- }
- }
+ /* else */
+ /* { */
+ /* if ( op & FETCH_ASSOC ) { */
+ /* rb_hash_aset(return_value, colName, LONG2NUM(atol((char
*)row_data->str_val))); */
+ /* } */
+ /* if ( op == FETCH_INDEX ) { */
+ /* rb_ary_store(return_value, i, LONG2NUM(atol((char
*)row_data->str_val))); */
+ /* } else if ( op == FETCH_BOTH ) { */
+ /* rb_hash_aset(return_value, INT2NUM(i),
LONG2NUM(atol((char *)row_data->str_val))); */
+ /* } */
+ /* } */
break;
case SQL_SMALLINT:
…----------------------
In case the diff is hard to read, here is the code that replaces the code
that was there. Of course you don't have to keep the comments.
/* if ((atof(row_data->str_val) - atol(row_data->str_val)) >
0) */ /* The logic above does not work to detect *negative*
decimals/floats. */ /* Also, these should always be returned
as BigDecimal, never as Integer. */ /* See:
#172 */ {
strcpy(tmpStr, "BigDecimal(\'"); strcat(tmpStr,
row_data->str_val); strcat(tmpStr, "\')"); if (
op & FETCH_ASSOC ) { rb_hash_aset(return_value, colName,
rb_eval_string(tmpStr)); } if ( op ==
FETCH_INDEX ) { rb_ary_store(return_value, i,
rb_eval_string(tmpStr) ); } else if ( op == FETCH_BOTH ) {
rb_hash_aset( return_value, INT2NUM(i), rb_eval_string(
tmpStr ) ); } ruby_xfree(tmpStr);
tmpStr = NULL; } /* else */ /* { */
/* if ( op & FETCH_ASSOC ) { */ /*
rb_hash_aset(return_value, colName, LONG2NUM(atol((char
*)row_data->str_val))); */ /* } */ /* if ( op ==
FETCH_INDEX ) { */ /* rb_ary_store(return_value, i,
LONG2NUM(atol((char *)row_data->str_val))); */ /* } else if
( op == FETCH_BOTH ) { */ /* rb_hash_aset(return_value,
INT2NUM(i), LONG2NUM(atol((char *)row_data->str_val))); */ /*
} */ /* } */
-c
On Fri, Aug 9, 2024 at 12:03 AM Olivier COUSIN ***@***.***> wrote:
I have the same problem
—
Reply to this email directly, view it on GitHub
<#172 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AANEUM3K6G2W4O7DUHTFADDZQRST3AVCNFSM6AAAAABK5O7AVCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENZXGI3TSNZVGU>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
All negative DECIMAL, NUMERIC and DECFLOAT numbers in result sets are truncated to negative integers in fetch_array, fetch_assoc, fetch_object. The problem does not happen in fetch_row/result, where they are returned as string types including negative sign, and could be parsed as decimals if desired.
To reproduce:
The bug is here in line 9582:
ruby-ibmdb/IBM_DB_Driver/ibm_db.c
Line 9582 in 8e33411
The data coming in from the DB2 driver is evidently a string representation of the value (!). The code is trying to determine if it has a decimal component, or not. If it does, it is going to create a BigDecimal. Otherwise, it just going to use a C "long" integer which will become a Ruby Integer. In that line, it is choosing a really odd way of determining if there is a decimal component: as follows: letting the C compiler parse the string as a float, then as a long, and then subtracting. If the result is "greater than zero" (e.g. 23.5 - 23 = 0.5) then of course there must be a floating point component, and so it uses BigDecimal. But that logic only works for positive numbers. If the number is -23.50, then you have -23.50 - -23, which gives you -.05, which is less than zero, so the whole-number (else) path is followed, giving us the integer interpretation (truncating everything after the decimal point). So this logic will never work for negative decimal or float values.
I would also dispute whether this logic should ever be returning integer values for these data types which have all been declared to have a decimal component. It seems it should always be returning BigDecimal, but that is another question. In other words, 12.00 stored in my database as decimal(10,2) is not the same meaning as 12 (integer).
The bug comes from this commit in 2022:
58e48b1
I am surprised to be the first person to find this bug. Maybe nobody ever uses negative decimal / currency numbers? In my client’s application, they are common.
P.S. the bug does not happen when using fetch_row/result, where these values are returned as string types including negative sign. I am not using that function myself, but this could also be considered wrong behavior. fetch_row/result should probably use the same internal logic to convert data values, as these other functions. Maybe it's too late to change that at this point.
Hope this helps,
-c
The text was updated successfully, but these errors were encountered: