Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decimal to binary conversion of numbers #1461

Closed
cjbj opened this issue Feb 24, 2022 · 8 comments
Closed

Decimal to binary conversion of numbers #1461

cjbj opened this issue Feb 24, 2022 · 8 comments
Labels
inactive This was marked by stalebot as inactive question

Comments

@cjbj
Copy link
Member

cjbj commented Feb 24, 2022

We have cases where columns intended to represent monetary values have been defined as Number, and decimal values have been inserted into these fields that include high precision. This causes problems for code that retrieve these later. Asking the driver to treat these as strings does not work for us because the large volume of calling code expects these values to be numbers and to be able to perform additions and subtractions with them. We would have to adjust all of this code to convert the strings to numbers.

I was hoping to find a way to get the oracledb driver to consider the scale of a number when extracting it from the DB, and adjust the number accordingly to eliminate any excess caused by the inherent issues with Javascript number representation. I understand that Javascript has limitations on how precisely it can represent decimal numbers. My desire here is to use the stated scale in the column definition to approximate the numeric value at that specific precision.

For example, if we had inserted a value of 2.30 (two dollars and 30 cents), and that was stored in the DB as 2.3000000000000003, upon extraction we would get 2.3 assuming the column was defined as Number(10,2).

I have tried to instrument the wrapper that we have around oracledb to look at the result set, and for the fields in our schema that are dollar values, perform a conversion:

value = Number(row[dollarFIeld].toFixed(2))

so converting it to a string with a fixed scale of 2, and then converting it back to a number. This works perfectly, but the performance impact is not manageable when our results sets are large.

I see that in SQLDeveloper, it appears to use the definition to adjust what is shown. A value in the DB such as 2.3000000000000003 is shown as 2.3000000000000003 if the column is defined as just Number, but is shown as 2.3 if the column is defined as Number(10,2). When I pull these same values via the Oracle driver, they both show as 2.3000000000000003, so presumably SQLDeveloper is considering the scale but the Oracle driver is not.

My guess is that Javascript, when assigning a floating point number to a variable, or converting a string to a number via the Number function (not constructor) is able to preserve the scale that was indicated by the literal number or string, but when any arithmetic is done, the intended scale is no longer clear so the result of that operation now has a large scale.

For example:

let x = 0.1
let y = 0.2
console.log(x + y)
0.30000000000000004
let z = Number((x+y).toFixed(2));
console.log(z);
0.3

Similar to the example above, the Oracle driver could use the scale in the column definition to call the Number function on columnValue.toFixed(columnScale)

Originally posted by @edfbarker in #793 (comment)

@edfbarker
Copy link

edfbarker commented Feb 26, 2022

A follow up on this.

After some reading, I think I understand the issue, but wanted to confirm.

Support I am trying to insert a monetary value into an Oracle field defined as just Number, which has the maximum for precision and scale (38 and 127 according to the docs).

In node.js code, I set my bind variable to 24.9, which in JavaScript is actually approximated as 24.89999999999999857891452847979962825775146484375000000000000000 (this is 29.9 printed with a precision of 64 via the toPrecision() function).

After inserting this value into Oracle, and pulling it back out via a select, the returned value printed out using toPrecision(64) gives a slightly different value: 24.90000000000000213162820728030055761337280273437500000000000000

According to the JavaScript standard, the default toString() representation of the original number allows it to be displayed as 24.9, since that can be deterministically be traced to the approximation (the original 24.89999999999999857891452847979962825775146484375000000000000000) rather than the nearest numbers greater than and less than that original value. However, the value that Oracle returned, if represented as just 24.9, is actually closer to one of the "nearest neighbor" values, and therefore toString() has to display this value as 24.900000000000002.

What I am not clear about is:

  • What causes Oracle to change the value. The docs say that the max scale is 127 which seems greater than the scale involved in this example.
  • Other than resorting to strings, is there a way to prevent this change in value from occurring?
  • How does the Oracle column definitions precision and scale settings impact the conversion, if at all. Would specifying an explicit scale of 2 allow the original value to be retained?

Running tests to answer the last point, it seems that there is no difference in behavior between a column defined as just Number and a column defined as Number(12,2).

In JavaScript, any number that is defined with two digits of scale will be shown via toString() with no more than 2 digits following the decimal, even though JS is representing that number as something slightly differently. With the value adjustment that occurs after inserting into Oracle and retrieving, the resulting number often times can't be shown with just two digits of precision via toString().

The ask in the issue is that there be some mechanism in the driver to recognize that we only care about a certain amount of scale (in this case 2), and and in the result rows array set numeric values according to their precision. It could be said that this distorts the original value, but the original value was already distorted by the conversion from Node to Oracle and back.

Of course, this new feature would require a setting to enable it, since it would be new behavior.

@cjbj cjbj changed the title We have cases where columns intended to represent monetary values have been defined as Number, and decimal values have been inserted into these fields that include high precision. This causes problems for code that retrieve these later. Asking the driver to treat these as strings does not work for us because the large volume of calling code expects these values to be numbers and to be able to perform additions and subtractions with them. We would have to adjust all of this code to convert the strings to numbers. Decimal to binary conversion of numbers Mar 5, 2022
@sla100
Copy link

sla100 commented Mar 29, 2022

While waiting for OCA, let me give you some examples.

import oracledb from 'oracledb';
import {deepStrictEqual} from 'node:assert/strict';

const conn = await oracledb.getConnection({
  user: 'admin',
  password: 'admin',
  connectString: 'localhost',
});

await conn.execute("ALTER SESSION SET NLS_TERRITORY = 'SPAIN'" );  // Yes, not all people live in the US 
const {rows} = await conn.execute(`
  SELECT 38.73 AS TEMPERATURE, -- SQL literals only use periods 
         1999.99 AS PRICE
   FROM DUAL
  `, {}, {
  fetchInfo: {
    PRICE:{ type: oracledb.STRING }, // Let's consider that the price is shown according to the user's location 
  },
  outFormat: oracledb.OUT_FORMAT_OBJECT,
});

deepStrictEqual( rows[0], {
  TEMPERATURE: 38.730000000000004,  // Yes, double conversion NUMBER -> double -> number has some limitations
  PRICE: '1999,99', // Yes - half of the world uses a comma as decimal separator
});

When the NLS parameters are set dynamically, the fetchInfo=STRING functionality is not sufficient.

We need to go depper...

@sla100
Copy link

sla100 commented Mar 29, 2022

Let's make a slight change to the file src/njsVariable.c:

bool njsVariable_initForQuery(njsVariable *vars, uint32_t numVars,
        dpiStmt *handle, njsBaton *baton)
{
            // the remaining types are valid but no special processing needs to
            // be done
            case DPI_ORACLE_TYPE_NUMBER:
                  // -----------------------------  MODIFIED ---------------------------------------
                  // We change the default download method to byte 
                  vars[i].nativeTypeNum = DPI_NATIVE_TYPE_BYTES;
                  vars[i].maxSize = 40; // sign + 38 digits + dot
                  // -----------------------------  MODIFIED ---------------------------------------
            case DPI_ORACLE_TYPE_NATIVE_INT:
            case DPI_ORACLE_TYPE_NATIVE_FLOAT:
            case DPI_ORACLE_TYPE_NATIVE_DOUBLE:
            case DPI_ORACLE_TYPE_ROWID:
            case DPI_ORACLE_TYPE_STMT:
            case DPI_ORACLE_TYPE_JSON:
                break;
} 

Result:

deepStrictEqual( rows[0], {
  TEMPERATURE: '38.73',  // Always "like literal" period separator
  PRICE: '1999,99', // User separator
});

@cjbj
Copy link
Member Author

cjbj commented Mar 29, 2022

@sla100 I found the right place and can see there is an outstanding OCA request for node-oracledb; I've asked for action. At the worst I'll end up having to do some training on the system so I can approve it myself.

@pvenkatraman
Copy link

The suggested change will fetch all numeric values as Strings which is not desirable. For most of the numeric value node-oracledb driver works good. Yes, in this case, add fetchInfo and convert the string to number dynamically.

@sla100
Copy link

sla100 commented Apr 8, 2022

Yes. It has to be parameterized. I just wanted to show the idea. Odpi-c can get the data without lossy conversion.

@stale
Copy link

stale bot commented Jul 10, 2022

This issue has been automatically marked as inactive because it has not been updated recently. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the inactive This was marked by stalebot as inactive label Jul 10, 2022
@stale
Copy link

stale bot commented Jul 31, 2022

This issue has been automatically closed because it has not been updated for a month.

@stale stale bot closed this as completed Jul 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inactive This was marked by stalebot as inactive question
Projects
None yet
Development

No branches or pull requests

4 participants