nastra commented on code in PR #9008:
URL: https://github.com/apache/iceberg/pull/9008#discussion_r1629719484


##########
api/src/main/java/org/apache/iceberg/transforms/Timestamps.java:
##########
@@ -31,54 +33,162 @@
 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 {
+    YEARS(ChronoUnit.YEARS),
+    MONTHS(ChronoUnit.MONTHS),
+    DAYS(ChronoUnit.DAYS),
+    HOURS(ChronoUnit.HOURS),
+    MICROS(ChronoUnit.MICROS),
+    NANOS(ChronoUnit.NANOS),
+    ;
+
+    private final ChronoUnit unit;
+
+    ResultTypeUnit(final ChronoUnit unit) {
+      this.unit = unit;
+    }
+
+    Duration getDuration() {
+      return unit.getDuration();
+    }
+  }
 
   @Immutable
   static class Apply implements SerializableFunction<Long, Integer> {
-    private final ChronoUnit granularity;
+    private final ChronoUnit sourceTypeUnit;
+    private final ResultTypeUnit resultTypeUnit;
 
-    Apply(ChronoUnit granularity) {
-      this.granularity = granularity;
+    Apply(ChronoUnit sourceTypeUnit, ResultTypeUnit resultTypeUnit) {
+      this.sourceTypeUnit = sourceTypeUnit;
+      this.resultTypeUnit = resultTypeUnit;
     }
 
     @Override
-    public Integer apply(Long timestampMicros) {
-      if (timestampMicros == null) {
+    public Integer apply(Long timestampUnits) {
+      if (timestampUnits == null) {
         return null;
       }
 
-      switch (granularity) {
-        case YEARS:
-          return DateTimeUtil.microsToYears(timestampMicros);
-        case MONTHS:
-          return DateTimeUtil.microsToMonths(timestampMicros);
-        case DAYS:
-          return DateTimeUtil.microsToDays(timestampMicros);
-        case HOURS:
-          return DateTimeUtil.microsToHours(timestampMicros);
+      switch (sourceTypeUnit) {
+        case MICROS:
+          switch (resultTypeUnit) {
+            case YEARS:
+              return DateTimeUtil.microsToYears(timestampUnits);
+            case MONTHS:
+              return DateTimeUtil.microsToMonths(timestampUnits);
+            case DAYS:
+              return DateTimeUtil.microsToDays(timestampUnits);
+            case HOURS:
+              return DateTimeUtil.microsToHours(timestampUnits);
+            default:
+              throw new UnsupportedOperationException(
+                  "Unsupported result type unit: " + resultTypeUnit);
+          }
+        case NANOS:
+          // TODO(epg): Overflows for MILLIS, MICROS, and NANOS!  Fixing this 
is quite invasive, as
+          //  Timestamps is assumed to be Transform<Long, Integer> in many, 
many places.
+          return (int) DateTimeUtil.convertNanos(timestampUnits, 
resultTypeUnit.unit);

Review Comment:
   I wonder if this should be using 
`Math.toIntExact(DateTimeUtil.convertNanos(timestampUnits, 
resultTypeUnit.unit))`



-- 
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

Reply via email to