Copilot commented on code in PR #18815:
URL: https://github.com/apache/pinot/pull/18815#discussion_r3444998116
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/PinotDataType.java:
##########
@@ -264,7 +264,7 @@ public byte[] toBytes(Object value) {
}
},
- INTEGER {
+ INT {
Review Comment:
Renaming/removing enum constants in `pinot-spi` is a source- and
binary-incompatible change for any downstream code that references
`PinotDataType.INTEGER`/`INTEGER_ARRAY` (including plugins built against older
Pinot). If this is intended, it should be explicitly called out and coordinated
with release/versioning; otherwise consider keeping the old constants as
`@Deprecated` aliases for at least one release cycle.
##########
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java:
##########
@@ -62,10 +62,10 @@ public static Object cast(Object value, String
targetTypeLiteral) {
case "DECIMAL":
targetDataType = BIG_DECIMAL;
break;
- case "INT":
+ case "INTEGER":
case "UTINYINT":
case "USMALLINT":
- targetDataType = INTEGER;
+ targetDataType = INT;
break;
Review Comment:
`cast()` uppercases `targetTypeLiteral` using the default locale
(`toUpperCase()`), which is locale-sensitive and can break type resolution in
locales like Turkish (e.g., "int" -> "İNT"). Since this PR is already touching
the cast type-literal mapping, it’s a good opportunity to make the
normalization locale-independent (use `Locale.ROOT`).
--
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]
