bamaer commented on PR #9303:
URL: https://github.com/apache/hop/pull/9303#issuecomment-6678593199

   Thanks @hansva, keeping the decimals is the right fix. I built the script 
plugin from this branch and ran `ScriptTest` (34/34 pass). I also probed the 
GraalJS, Groovy and Jython engines.
   
   **Blocker**
   
   - `BigDecimal.valueOf(double)` gives a negative scale for whole numbers from 
1e7 upwards, so `toString()` switches to scientific notation. GraalJS returns a 
`Double` even for whole numbers. For example, `x = 2.5*2e7` used to give 
`50000000` and now gives `5.0E+7`, and `x = 1e10` now gives `1.0E+10`. That is 
what `getDataXml` writes, and what JDBC drivers that inline parameters send. 
Core already handles this in `ValueMetaBase.convertDoubleToBigNumber` (#7744, 
issue #6111).
     `Script.java:551` and `:569`. Suggested fix: a small helper that rescales 
negative scales to 0.
     ```java
     private static BigDecimal doubleToBigNumber(double value) {
       BigDecimal bigDecimal = BigDecimal.valueOf(value);
       return bigDecimal.scale() < 0 ? bigDecimal.setScale(0) : bigDecimal;
     }
     ```
     With this helper and a test that `5.0E7` gives `"50000000"`, `ScriptTest` 
passes (35/35).
   
   **Should fix**
   
   - A `NaN` or `Infinity` result now stops the transform with 
`NumberFormatException: Character N is neither a decimal digit number...`. 
Before, it silently gave `0` (or `Long.MAX_VALUE` for Infinity). 
`parseFloat('')` or `0/0` in a script is enough to hit it. Stopping is better 
than a wrong 0, but the error should name the field, or the value should become 
`null`. Either way, choose one on purpose and add a test for it.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to