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 of
primitive type.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
