oracle / node-oracledb

Oracle Database driver for Node.js maintained by Oracle Corp.
http://oracle.github.io/node-oracledb/
Other
2.26k stars 1.08k forks source link

JavaScript & Oracle Number handling: two connections with differents fetchAsString parameters #665

Closed atiertant closed 6 years ago

atiertant commented 7 years ago

how to create two connections: first with fetchAsString on numbers and second without any ?

sagiegurari commented 7 years ago

you can set those up on every connection.execute call

dmcghan commented 7 years ago

@atiertant See the fetchInfo section of the doc.

atiertant commented 7 years ago

@sagiegurari connection.execute is asynchronous, are you sure this is working if there are many request at the same time?

@dmcghan this is for explicit column not for every one like fetchAsString

dmcghan commented 7 years ago

@atiertant That's true, there's currently no generic solution at the execute level. Could you please explain the use case? Why does fetchAsString not work for you?

atiertant commented 7 years ago

@dmcghan first problem was node-oracledb don't bind float correctly, to avoid this problem, we used fetchAsString but for integrate this in knex we need to write unit test, the fetchAsString test is working, but other test fail because they sould not return string... the problem is getConnection() doesn't configure fetchAsString in connection, it use the module global array...

dmcghan commented 7 years ago

@atiertant

first problem was node-oracledb don't bind float correctly

Did you log an issue? Do you have a test case? It sounds like this is the real problem you're having so I think it's best we look at that.

cjbj commented 7 years ago

The attribute hierarchy change request is tracked in https://github.com/oracle/node-oracledb/issues/361

At a guess, any number 'issue' is an expected artifact of conversion to/from different binary representations.

atiertant commented 7 years ago

@cjbj console.log(0.2 + 0.7); // gives 0.8999999999999999 but console.log(0.9); //gives 0.9 so this could be represented in javascript number... where does this binary conversion is done in javascript code?

bjouhier commented 7 years ago

@atiertant See http://floating-point-gui.de/basic/

atiertant commented 7 years ago

@bjouhier java and many other languages have same precision in operations: console.log(0.2 + 0.7); // gives 0.8999999999999999 but console.log((2 + 7) /10 ); // gives 0.9

see https://github.com/MikeMcl/bignumber.js/

there are solutions ...

bjouhier commented 7 years ago

Of course but JavaScript only has one number datatype, and it's an IEEE 754 floating point.

atiertant commented 7 years ago

@bjouhier problem is not in javascript Number datatype (this is a 64bits signed number)

var a = 0.9;
console.log(a); // gives 0.9

only mathematical operations loss precision with floating point but you can perform them with integer and divide result after... console.log((0.2 * 10 + 0.7 * 10) /10 ); // gives 0.9 here + is performed on integers so no problem with precision...

bjouhier commented 7 years ago

@atiertant Sorry to contradict you but the problem is in the JavaScript Number datatype. This datatype is an IEEE 754 floating point. See https://www.ecma-international.org/ecma-262/7.0/index.html section 4.3.21

When you write 0.9 this is not exactly 0.9, this is the floating-point number which is closest to 0.9 (and there is some magic in the number to string conversion to format it as 0.9).

atiertant commented 7 years ago

@bjouhier your right, it's not exactly 0.9 in the value binary representation, that's why we lose precision. but the stringified value should be correct, this one is used by every javascript mathematical libraries and in every displayed value... so this could be fixed using javascript Number datatype.

bjouhier commented 7 years ago

If you want to do precise arithmetics with decimals you should not use JavaScript numbers. Instead you should use a big decimal library like the one you mentioned above. But you should manipulate the values only as instances of the Big class or as strings. Don't convert them back and forth to javascript numbers.

When reading or writing to Oracle, you should pass the values as strings, not numbers.

Oracle numbers can have up to 38 decimals of precision (and, unlike JS numbers, they represent these numbers exactly). A 64 bit integer has 19.27 decimals of precision but a JS number only has 15.95 decimals of precision. So you're heading for trouble it you try to map high precision database values to JS values, even integer values that would fit into a 64 bit int. Try console.log(+"12345678901234567").

dmcghan commented 7 years ago

@atiertant The problem is the JavaScript Number type. The stringified value is sometimes rounded. We can't depend on that.

A JavaScript decimal type recently came up in a twitter discussion from one of the tc39 folks: https://twitter.com/code_barbarian/status/850942340873060352

If JavaScript gets a decimal type, then we can look at using that instead.

atiertant commented 7 years ago

@dmcghan if the problem was the JavaScript Number type why must i use a fetchAsString then parsefloat that return a JavaScript Number ???

console.log(Number(0.2 + 0.7) !== Number(0.9)) // gives true
console.log(Number(0.9) !== Number(0.9)) // gives false

maybe the max oracle database precision could not be handled with JavaScript Number type but at now node-oracledb do worst than javascript can...

bjouhier commented 7 years ago

Don't convert to JS number (don't use parseFloat)! Convert from string to Big number (from 3rd party library) and do all the computation with Big numbers.

dmcghan commented 7 years ago

@atiertant fetchAsString with parseFloat is not fixing the problem, it's masking it. If you tried to do arithmetic with the result this would become more obvious.

let x = parseFloat('.1');
let y = parseFloat('.2');

console.log(x); // 0.1
console.log(y); // 0.2
console.log(x + y); // 0.30000000000000004

We can't depend on rounding display values, we need a decimal type.

In the meantime, decimal arithmetic should be done in Oracle and the results brought through as strings to be displayed. As for your library, I'd recommend not trying to fix this on your own.

bjouhier commented 7 years ago

You are fooling yourself into thinking that JavaScript can do a good job with decimal numbers. JS numbers only represent a very small subset of decimal numbers exactly. Even numbers as simple as 0.1 or 0.2 are only approximations (but 0.125 is exact!). If you want to do decimal arithmetics, stay away from JS numbers and use a big decimal library.

The problem has nothing to do with the oracledb driver. It has to do with JavaScript itself.

atiertant commented 7 years ago

@dmcghan i use mathjs for mathematical operation. i need fetchAsString because node-oracledb can't assign number correctly, javascript number type can't do mathematical operation with precision, it's ok (and out of node-oracldb scope).

but javascript can handle stringify number correctly, and using them with mathjs (stringifing them and using bigNumber internaly) but those caming from node-oracledb not, because the stringified value is wrong! parseFloat just create the same number with the correct string value...(i only use 2 digits)

if you used integer value in your conversion algorithm and divide it at end, you should have a correct string value. console.log((0.2 * 10 + 0.7 * 10) /10 ); // gives 0.9

atiertant commented 7 years ago

@bjouhier wouldn't like to do decimal arithmetics, just have a correct value on assignment, this is a fact: number caming from database could be assigned correctly because parseFloat do, sure there are some case where it could not work but 2 digits ...

bjouhier commented 7 years ago

@atiertant Why don't you use fetchAsString and then math.bignumber(str) with the strings that you obtain? The whole point behind fetchAsString is to give you a reliable method to pass decimal values back and forth between Oracle and the decimal library of your choice.

For the fun of it I tried to find out how many numbers in the 0.000000, 0.000001, 0.000002, 0.000003, ... 0.999999 series are exact in JavaScript. It's only 64 out of 1,000,000! With 2 digits, it is only 4 out of 100: 0.00, 0.25, 0.50 and 0.75. All the others numbers are approximations.

bjouhier commented 7 years ago

@atiertant To understand your problem better I have a few questions:

atiertant commented 7 years ago

hope this to be more clear:

var oracledb = require('oracledb');

oracledb.getConnection({
  user: "user",
  password: "password",
  connectString: "ip/db"
}, function(err, connection) {
  if (err) {
    console.error(err.message);
    return;
  }
  connection.execute("SELECT 0.235 from DUAL", function(err, result) {
    if (err) {
      console.error(err.message);
      doRelease(connection);
      return;
    }
    console.log('oracledb', result.rows[0].toString()); // oracledb 0.23500000000000001
    var a = parseFloat('0.235');
    console.log('parseFloat', a); // parseFloat 0.235
    console.log('is equal', a.toString() === result.rows[0].toString()); // is equal false
    doRelease(connection);
  });
});

function doRelease(connection) {
  connection.close(function(err) {
    if (err) {
      console.error(err.message);
    }
  });
}

as you can see string value is wrong...but javascript can assign this value

bjouhier commented 7 years ago

I tried some variants and dived a bit into the source of the driver.

Interestingly, SELECT CAST(0.235 as BINARY_DECIMAL) from DUAL gives 0.2349999999999999 instead of 0.23500000000000001.

I did not see any special parsing in the oracledb driver source code. It just transfers the values between OCI and V8 (https://github.com/oracle/node-oracledb/blob/master/src/njs/src/njsConnection.cpp#L4047 for example). The conversion between the oracle number and double must be inside OCI and may differ from what a you get by first converting to string, and then parsing with parseFloat (which would be inefficient).

It is very likely that you will get the same discrepancy if you call OCI from C and retrieve Oracle decimal numbers into C double variables.

Morale: to get reliable results with decimal values, always fetch them as strings.

atiertant commented 7 years ago

so this is an oracle(maybe in OCI) bug is there any chance to see this fixed a day?

Morale: to get reliable results with decimal values, always fetch them as strings.

so, to get reliable results, do not use oracle methods, rewrite your own...

bjouhier commented 7 years ago

so, to get reliable results, do not use oracle methods, rewrite your own...

Not quite. OCI has a proper API to retrieve numbers with full precision: https://docs.oracle.com/cd/B12037_01/appdev.101/b10779/oci11oty.htm#434061. The problem is that JavaScript does not have any built-in type for decimals, just a floating-point number type. A .NET driver, for example, would be able to do exact conversions to .NET decimals.

An alternative would be to expose the OCI Number class to JavaScript. I know this has been suggested but I don't think it has been implemented so far.

atiertant commented 7 years ago

look like an old problem ... https://github.com/oracle/node-oracledb/issues/20 https://github.com/oracle/node-oracledb/issues/68 https://github.com/oracle/node-oracledb/issues/206 https://github.com/oracle/node-oracledb/issues/555 https://github.com/oracle/node-oracledb/issues/638

atiertant commented 7 years ago

@bjouhier

The problem is that JavaScript does not have any built-in type for decimals, just a floating-point number type

so the problem would come from nan who can't assign 0.235 Number string value as there are no exact binary representation for this value. i made some simple tests:

git clone https://github.com/nodejs/nan.git
cd nan/
npm install
npm run rebuild-tests
vi test/js/converters-test.js

replace line 41:

-  t.equal(converters.numberValue(15.3), 15.3);
+  t.equal(converters.numberValue(0.235).toString(), "0.235");

then launch tests

npm test
...
ok test/js/converters-test.js ......................... 29/29

all test were ok !

bjouhier commented 7 years ago

No, nan doesn't have a problem. The following test works too:

t.equal(converters.numberValue(0.235), 0.235);

My guess is that the problem is more in the algorithm that converts Oracle decimals (exact representations) to JS numbers (approximations). To get an idea why this algorithm may not produce what you'd expect, try:

$ node
> 0.2 + 0.03 + 0.005
0.23500000000000001

Producing the IEEE 754 number which prints exactly as "0.235" is not that easy. It requires a very clever algorithm, which has some overhead. My guess is that OCI uses a simpler and faster algorithm. JS numbers are not suitable for exact decimal arithmetics anyway so why take a performance hit?

bjouhier commented 7 years ago

To get an idea of what it takes to implement number parsing, take a look at https://github.com/nodejs/node/blob/v8.x/deps/v8/src/strtod.cc. As you can see around lines 413-418 it can take several attempts to get the right value (the guess variable). This is where OCI probably takes some speed shortcuts. V8 MUST go the extra mile because parseFloat has to conform to the language specs.

atiertant commented 7 years ago

@bjouhier

I did not see any special parsing in the oracledb driver source code. It just transfers the values between OCI and V8 (https://github.com/oracle/node-oracledb/blob/master/src/njs/src/njsConnection.cpp#L4047 for example). The conversion between the oracle number and double must be inside OCI and may differ from what a you get by first converting to string, and then parsing with parseFloat (which would be inefficient).

so node doesn't handle conversion ... stop saying it's a node problem

My guess is that OCI uses a simpler and faster algorithm. JS numbers are not suitable for exact decimal arithmetics anyway so why take a performance hit?

do you think fetchAsString + parseFloat is faster ? why would i like the exact value? just because 0.23500000000000001 instead of 0.235 is unusable...(what would you like to do with this number? faster to do what?) in my stack, i don't acess node-oracledb it self to query database and acess data. i got an orm that call an adapter that use knex that use node-oracledb and then give me back numbers... as i already said we use mathjs for arithmetics but this could only work if string value is correct.

bjouhier commented 7 years ago

as i already said we use mathjs for arithmetics but this could only work if string value is correct

Then, read the docs, retrieve values as strings with fetchAsString and pass them to mathjs as strings. Don't convert them to JS numbers in the middle. I start to feel that you are trolling me.

0.23500000000000001 is not unusable. It all depends what you do. If you compute an accounting balance, it can be a problem but if you do statistics (compute averages, standard deviations, etc.) it is good enough.

bjouhier commented 7 years ago

@atiertant And if you insist on going through JS numbers the following function may help you:

> function round3(x) { return Math.round(x * 1000) / 1000; }
> round3(0.23500000000000001);
0.235
atiertant commented 7 years ago

@bjouhier i won't rewrite all my stack just because node-oracledb can only provide approximative numbers... (with oracle, it would be the 10th rewrite...this is a troll ;)) so one more time this would not be a bug but a feature (parsing wrong but parsing faster) hope this is a joke @cjbj @dmcghan could you confirm ?

bjouhier commented 7 years ago

@atiertant Then, read the docs, retrieve values as strings with fetchAsString and pass them to mathjs as strings. What's the problem?

atiertant commented 7 years ago

@bjouhier read the issue

bjouhier commented 7 years ago

@atiertant I understand your issue. You don't like the fact that reading decimals as numbers sometimes gives values that are a bit off. But this is a well known issue; this is covered by the documentation and there is an option (fetchAsString) which is specifically designed to solve this problem. Why don't you use it?

I tried to help you and dig to give you an explanation where this could come from (I'm curious too). I cannot do much more.

atiertant commented 7 years ago

@bjouhier the only thing i don't like is saying "The problem is the JavaScript Number type" in this case, this is wrong! so oracle could reconize it's a bug in their code, they fix or not but stop saying this is not possible in javascript... would be curious too to know what is contain in (double )bind->value

there is an option (fetchAsString) which is specifically designed to solve this problem.

this doesn't solve the problem, this is a trick to avoid the problem.(but haven't got choice)

that said ! for now, i need fetchAsString to get exact number but an other problem is that fetchAsString is a module global parameter so can't have two connections with different parameters.

bjouhier commented 7 years ago

so oracle could reconize it's a bug in their code

That's Oracle's call, not mine. I don't work for Oracle.

bjouhier commented 7 years ago

From the doc:

Individual query columns in execute() or queryStream() calls can override the fetchAsString global setting by using fetchInfo.

Maybe this can help you handle multiple connections. This is how we handle this issue.

Otherwise, I'm pretty sure that the*(double*)bind->value contains the bad value. To check it you can just add a printf and recompile the driver.

anthony-tuininga commented 7 years ago

As stated many times already, the problem goes to the fact that decimal numbers can only be approximated in floating point. Arguing about which approximation is "correct" seems pointless since neither of them are accurate. Both are approximations! The only advantage that the JavaScript approximation has over the Oracle approximation is that it is consistent with itself. If, however, you attempt to perform any arithmetic with those floating point numbers the inaccuracy will become obvious soon enough -- again, as has been shown above.

The real solution is to use decimal numbers in JavaScript as well -- and there are a number of third party libraries that support them. But in order to use those you have to use fetchAsString and then iterate over the rows returned to transform those strings into the decimal numbers -- which incurs a great deal of overhead and is disruptive to the code. One possible solution to this problem that I have considered is the introduction of a "hook" that could be specified at various levels (globally, per connection, per bind variable). This hook would be a callable that would accept one argument (the value that would ordinarily be returned by node-oracledb) and would return the value that should be returned instead. This would allow the use of any decimal number library (or even parseFloat() for those who prefer it!) without having to massage the output from the node-oracledb library. It could of course be used for other purposes! Does this sound like a good idea? Like it might resolve your issue? Thoughts welcome!

dmcghan commented 7 years ago

The only advantage that the JavaScript approximation has over the Oracle approximation is that it is consistent with itself.

I would add one more advantage, which is @atiertant's main use case and a common one for Node.js in general: REST APIs that output JSON. In this case, you're not planning on doing arithmetic, you're just sending the data along for another system to leverage. Sending the number out as a string is often undesirable (though is still needed for really large numbers).

One possible solution to this problem that I have considered is the introduction of a "hook" that could be specified at various levels (globally, per connection, per bind variable).

To better illustrate this concept (though the final API could be different) here's an example:

oracledb.fetchMappings = [
  {
    from: oracledb.NUMBER,
    to: oracledb.STRING,
    converter: parseFloat
  }
]
bjouhier commented 7 years ago

Arguing about which approximation is "correct" seems pointless since neither of them are accurate. Both are approximations!

Right. JavaScript uses the IEEE 754-2008 “round to nearest, ties to even” rounding mode. Libraries written in other languages (like OCI) may use a different rounding rule when they convert decimals to floating-point. This cannot be qualified as an OCI bug because OCI may be called from different languages which may have different rules (or no rule at all, leaving this up to the compiler!).

The oracledb driver could bypass the decimal-to-double conversion done by OCI, to align strictly on the JavaScript rule (easy, just call parseFloat(fetch_as_string_value)). But this is not free (in CPU cycles) and it is a bit of a mirage (as values are approximations anyway). IMO, not worth it.

@anthony-tuininga The current API does the job for us but I can see the benefit of having a hook. What seems to be lacking most is a way to configure the mapping at the connection or pool level.

anthony-tuininga commented 7 years ago

@anthony-tuininga The current API does the job for us but I can see the benefit of having a hook. What seems to be lacking most is a way to configure the mapping at the connection or pool level.

Thanks for the feedback. I agree that being able to configure the mapping at a connection or pool level would be a good thing to have. What sort of behaviour would you like to see? One possibility would be to extend fetchInfo and permit the column name to be a data type as well, as in this:

fetchInfo: { oracledb.NUMBER : { type : oracledb.STRING } }

And if the hook was implemented it could look like this:

fetchInfo: { oracledb.NUMBER : { type : oracledb.STRING, converter = BigNumber } }

This format could then be used globally, at the connection level or at the execute level. At the connection level, if the attribute is not set by the user, the global value would be used. Does that seem reasonable to you?

bjouhier commented 7 years ago

@anthony-tuininga API looks reasonable except for the mix of keys (column names and datatype constants) under fetchInfo. I'd rather introduce a different key for datatypes, something like fetchTypes.

anthony-tuininga commented 7 years ago

@bjouhier Thanks. We'll keep that in mind.

bjouhier commented 7 years ago

@anthony-tuininga One potential issue with hooks might be performance: every call to the hook will have to go through the C++/JavaScript boundary. This may be more costly than returning all the values as strings and transforming the result's rows in JS afterwards.

anthony-tuininga commented 7 years ago

@bjouhier Good point. We'll need to test that. Thanks.