Copilot commented on code in PR #17543:
URL: https://github.com/apache/pinot/pull/17543#discussion_r2710551708
##########
pinot-common/src/main/java/org/apache/pinot/common/function/TimeZoneKey.java:
##########
@@ -80,7 +72,7 @@ public synchronized Object put(Object key, Object value) {
short maxZoneKey = 0;
for (Entry<Object, Object> entry : data.entrySet()) {
- short zoneKey = Short.valueOf(((String) entry.getKey()).trim());
+ short zoneKey = Short.parseShort(((String) entry.getKey()).trim());
Review Comment:
The change from `Short.valueOf()` to `Short.parseShort()` is correct for
avoiding unnecessary boxing, but lacks error handling. If the
zone-index.properties file contains non-numeric keys, this will throw a
NumberFormatException. Consider wrapping this in a try-catch block that
provides a more informative error message indicating which key failed to parse.
```suggestion
String rawKey = ((String) entry.getKey()).trim();
short zoneKey;
try {
zoneKey = Short.parseShort(rawKey);
} catch (NumberFormatException e) {
throw new AssertionError("Zone file contains a non-numeric key: '"
+ rawKey + "'", e);
}
```
##########
pinot-common/src/main/resources/zone-index.properties:
##########
@@ -2196,7 +2198,7 @@
2171 US/Michigan
2172 US/Mountain
2173 US/Pacific
-# 2174 US/Pacific-New # not in joda-time
+# 2174 US/Pacific-New removed
http://mm.icann.org/pipermail/tz-announce/2020-October/000059.html
Review Comment:
The comment references a removal announcement from October 2020, but the URL
provided is an HTTP link. Consider using HTTPS for the ICANN link for better
security: https://mm.icann.org/pipermail/tz-announce/2020-October/000059.html
```suggestion
# 2174 US/Pacific-New removed
https://mm.icann.org/pipermail/tz-announce/2020-October/000059.html
```
--
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]
