bziobrowski commented on code in PR #14337:
URL: https://github.com/apache/pinot/pull/14337#discussion_r1832283197
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/JsonExtractScalarTransformFunction.java:
##########
@@ -184,8 +191,7 @@ public long[] transformToLongValuesSV(ValueBlock
valueBlock) {
if (result instanceof Number) {
_longValuesSV[i] = ((Number) result).longValue();
} else {
- // Handle scientific notation
- _longValuesSV[i] = (long) Double.parseDouble(result.toString());
+ _longValuesSV[i] = Long.parseLong(result.toString());
Review Comment:
JSON standard does allow that for numbers but not for integers.
It's a question of whether we want to keep supporting conversion from json
floating point ( scientific notation or a number with fractional part) string
directly to long.
In terms of testing one can simulate it by adding the following to
`JsonExtractScalarTransformFunctionTest.testJsonPathTransformFunctionDataProvider`
:
```
testArguments.add(new Object[]{
String.format("jsonExtractScalar(%s,'$.stringSV','LONG')", input),
DataType.LONG, true
});
```
I'd really avoid allocating object per row if result is going to be long.
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
