epgif commented on code in PR #9008:
URL: https://github.com/apache/iceberg/pull/9008#discussion_r1698492880
##########
api/src/main/java/org/apache/iceberg/transforms/Timestamps.java:
##########
@@ -31,54 +33,159 @@
import org.apache.iceberg.util.DateTimeUtil;
import org.apache.iceberg.util.SerializableFunction;
-enum Timestamps implements Transform<Long, Integer> {
- YEAR(ChronoUnit.YEARS, "year"),
- MONTH(ChronoUnit.MONTHS, "month"),
- DAY(ChronoUnit.DAYS, "day"),
- HOUR(ChronoUnit.HOURS, "hour");
+class Timestamps implements Transform<Long, Integer> {
+
+ static final Timestamps YEAR_FROM_MICROS =
+ new Timestamps(ChronoUnit.MICROS, ResultTypeUnit.YEARS, "year");
+ static final Timestamps MONTH_FROM_MICROS =
+ new Timestamps(ChronoUnit.MICROS, ResultTypeUnit.MONTHS, "month");
+ static final Timestamps DAY_FROM_MICROS =
+ new Timestamps(ChronoUnit.MICROS, ResultTypeUnit.DAYS, "day");
+ static final Timestamps HOUR_FROM_MICROS =
+ new Timestamps(ChronoUnit.MICROS, ResultTypeUnit.HOURS, "hour");
+ static final Timestamps YEAR_FROM_NANOS =
+ new Timestamps(ChronoUnit.NANOS, ResultTypeUnit.YEARS, "year");
+ static final Timestamps MONTH_FROM_NANOS =
+ new Timestamps(ChronoUnit.NANOS, ResultTypeUnit.MONTHS, "month");
+ static final Timestamps DAY_FROM_NANOS =
+ new Timestamps(ChronoUnit.NANOS, ResultTypeUnit.DAYS, "day");
+ static final Timestamps HOUR_FROM_NANOS =
+ new Timestamps(ChronoUnit.NANOS, ResultTypeUnit.HOURS, "hour");
+
+ static Timestamps get(Types.TimestampType type, String resultTypeUnit) {
+ switch (resultTypeUnit.toLowerCase(Locale.ENGLISH)) {
+ case "year":
+ return get(type, ChronoUnit.YEARS);
+ case "month":
+ return get(type, ChronoUnit.MONTHS);
+ case "day":
+ return get(type, ChronoUnit.DAYS);
+ case "hour":
+ return get(type, ChronoUnit.HOURS);
+ default:
+ throw new IllegalArgumentException(
+ "Unsupported source/result type units: " + type + " -> " +
resultTypeUnit);
+ }
+ }
+
+ static Timestamps get(Types.TimestampNanoType type, String resultTypeUnit) {
+ switch (resultTypeUnit.toLowerCase(Locale.ENGLISH)) {
+ case "year":
+ return get(type, ChronoUnit.YEARS);
+ case "month":
+ return get(type, ChronoUnit.MONTHS);
+ case "day":
+ return get(type, ChronoUnit.DAYS);
+ case "hour":
+ return get(type, ChronoUnit.HOURS);
+ default:
+ throw new IllegalArgumentException(
+ "Unsupported source/result type units: " + type + " -> " +
resultTypeUnit);
+ }
+ }
+
+ static Timestamps get(Types.TimestampType type, ChronoUnit resultTypeUnit) {
+ switch (resultTypeUnit) {
+ case YEARS:
+ return YEAR_FROM_MICROS;
+ case MONTHS:
+ return MONTH_FROM_MICROS;
+ case DAYS:
+ return DAY_FROM_MICROS;
+ case HOURS:
+ return HOUR_FROM_MICROS;
+ default:
+ throw new IllegalArgumentException(
+ "Unsupported source/result type units: " + type + " -> " +
resultTypeUnit);
+ }
+ }
+
+ static Timestamps get(Types.TimestampNanoType type, ChronoUnit
resultTypeUnit) {
+ switch (resultTypeUnit) {
+ case YEARS:
+ return YEAR_FROM_NANOS;
+ case MONTHS:
+ return MONTH_FROM_NANOS;
+ case DAYS:
+ return DAY_FROM_NANOS;
+ case HOURS:
+ return HOUR_FROM_NANOS;
+ default:
+ throw new IllegalArgumentException(
+ "Unsupported source/result type units: " + type + " -> " +
resultTypeUnit);
+ }
+ }
+
+ enum ResultTypeUnit {
Review Comment:
This came from your suggestion at
https://github.com/apache/iceberg/pull/9008#discussion_r1421812913
Summarizing my understanding of your comment:
- ChronoUnit's range is wider than our range here, forcing callers to handle
impossible values
- No ChronoUnit in Iceberg API (now moot as later changes led to this losing
its `public` status)
I think the first reason still applies and, as you said, "the cost of
introducing our own enum is small."
Happy to switch back to ChronoUnit if you think that's better, but please
let's not wait months for that decision.
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
