Copilot commented on code in PR #18940:
URL: https://github.com/apache/pinot/pull/18940#discussion_r3547021918
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/PinotDataType.java:
##########
@@ -894,12 +904,22 @@ public String convert(Object value, PinotDataType
sourceType) {
JSON {
@Override
public int toInt(Object value) {
- return Integer.parseInt(value.toString().trim());
+ String trimmedStr = value.toString().trim();
+ try {
+ return Integer.parseInt(trimmedStr);
+ } catch (NumberFormatException e) {
+ return new BigDecimal(trimmedStr).intValue();
+ }
Review Comment:
Same overflow issue as `STRING.toInt`: falling back to
`BigDecimal.intValue()` after a failed `Integer.parseInt` will silently wrap
for out-of-range numeric strings. This can turn invalid inputs into incorrect
values instead of throwing.
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/PinotDataType.java:
##########
@@ -834,12 +834,22 @@ public Long toInternal(Object value) {
STRING {
@Override
public int toInt(Object value) {
- return Integer.parseInt(value.toString().trim());
+ String trimmedStr = value.toString().trim();
+ try {
+ return Integer.parseInt(trimmedStr);
+ } catch (NumberFormatException e) {
+ return new BigDecimal(trimmedStr).intValue();
+ }
Review Comment:
The fallback to `new BigDecimal(trimmedStr).intValue()` changes overflow
behavior: values that previously threw (e.g. "2147483648" or "1e20") will now
silently wrap/truncate, producing wrong results. The fallback should still
throw `NumberFormatException` when the truncated integral value is outside the
`int` range, while still accepting decimal/scientific strings.
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/PinotDataType.java:
##########
@@ -834,12 +834,22 @@ public Long toInternal(Object value) {
STRING {
@Override
public int toInt(Object value) {
- return Integer.parseInt(value.toString().trim());
+ String trimmedStr = value.toString().trim();
+ try {
+ return Integer.parseInt(trimmedStr);
+ } catch (NumberFormatException e) {
+ return new BigDecimal(trimmedStr).intValue();
+ }
}
@Override
public long toLong(Object value) {
- return Long.parseLong(value.toString().trim());
+ String trimmedStr = value.toString().trim();
+ try {
+ return Long.parseLong(trimmedStr);
+ } catch (NumberFormatException e) {
+ return new BigDecimal(trimmedStr).longValue();
+ }
Review Comment:
The `BigDecimal` fallback in `toLong` will also run for out-of-range integer
strings because `Long.parseLong` throws `NumberFormatException` on overflow.
That means inputs like "9223372036854775808" will now return a wrapped `long`
instead of throwing. Please enforce range checks and keep throwing
`NumberFormatException` on overflow after truncation.
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/PinotDataType.java:
##########
@@ -894,12 +904,22 @@ public String convert(Object value, PinotDataType
sourceType) {
JSON {
@Override
public int toInt(Object value) {
- return Integer.parseInt(value.toString().trim());
+ String trimmedStr = value.toString().trim();
+ try {
+ return Integer.parseInt(trimmedStr);
+ } catch (NumberFormatException e) {
+ return new BigDecimal(trimmedStr).intValue();
+ }
}
@Override
public long toLong(Object value) {
- return Long.parseLong(value.toString().trim());
+ String trimmedStr = value.toString().trim();
+ try {
+ return Long.parseLong(trimmedStr);
+ } catch (NumberFormatException e) {
+ return new BigDecimal(trimmedStr).longValue();
+ }
Review Comment:
Same overflow issue as `STRING.toLong`: `Long.parseLong` throws
`NumberFormatException` for overflow, which will route into the
`BigDecimal.longValue()` fallback and produce a wrapped `long`. The fallback
should preserve the pre-existing contract of throwing for out-of-range values.
--
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]
